Re: [PATCH] riched20: Process WM_GETMINMAXINFO which arrives before WM_NCCREATE.

[Dmitry Timoshkov]

Austin Lund  wrote:
> --- a/dlls/riched20/editor.c
> +++ b/dlls/riched20/editor.c
> @@ -4406,7 +4406,7 @@ static LRESULT RichEditWndProc_common(HWND hWnd, UINT msg, WPARAM wParam,
>        texthost = ME_CreateTextHost(hWnd, pcs, FALSE);
>        return texthost != NULL;
>      }
> - else if (msg != WM_NCDESTROY)
> - else if ((msg != WM_NCDESTROY) && (msg != WM_GETMINMAXINFO))
> {
>   ERR("called with invalid hWnd %p - application bug?\n", hWnd);
>   return 0;

This code is completely broken. An app can decide to not even pass these
messages to a subclassed richedit control.

[PATCH] riched20: Process WM_GETMINMAXINFO which arrives before WM_NCCREATE.

[Austin Lund]

On 6 July 2010 17:39, Dmitry Timoshkov  wrote:
> Austin Lund  wrote:
> > --- a/dlls/riched20/editor.c
> > +++ b/dlls/riched20/editor.c
> > @@ -4406,7 +4406,7 @@ static LRESULT RichEditWndProc_common(HWND hWnd, UINT msg, WPARAM wParam,
> >        texthost = ME_CreateTextHost(hWnd, pcs, FALSE);
> >        return texthost != NULL;
> >      }
> > - else if (msg != WM_NCDESTROY)
> > - else if ((msg != WM_NCDESTROY) && (msg != WM_GETMINMAXINFO))
> >
> > {
> >        ERR("called with invalid hWnd %p - application bug?\n", hWnd);
> >        return 0;
>
> This code is completely broken. An app can decide to not even pass these
> messages to a subclassed richedit control.

I'm not sure how that's relevant because if the messages aren't passed
then as long as a WM_NCCREATE is passed then the editor variable is
initialised by this.

I'm reading this comment carefully for a suggestion as to how to "fix"
things, but it seems totally obfuscated.

[PATCH] riched20: Process WM_GETMINMAXINFO which arrives before WM_NCCREATE.

[Dmitry Timoshkov]

Austin Lund  wrote:
> > This code is completely broken. An app can decide to not even pass these
> > messages to a subclassed richedit control.
>
> I'm not sure how that's relevant because if the messages aren't passed
> then as long as a WM_NCCREATE is passed then the editor variable is
> initialised by this.

What happens if an application doesn't pass WM_NCCREATE?
> I'm reading this comment carefully for a suggestion as to how to "fix"
> things, but it seems totally obfuscated.

Editor should be unconditionally initialized if it's NULL, and WM_NCDESTROY
should be made an explicit case in the 'switch' statement.

[PATCH] riched20: Process WM_GETMINMAXINFO which arrives before WM_NCCREATE.

[Alexandre Julliard]

Dmitry Timoshkov  writes:
> Austin Lund  wrote:
> > I'm not sure how that's relevant because if the messages aren't passed
> > then as long as a WM_NCCREATE is passed then the editor variable is
> > initialised by this.
>
> What happens if an application doesn't pass WM_NCCREATE?

It would be legitimate for a window to not be properly initialized in
that case. Richedit may or may not have a workaround for this, but it's
not a requirement of the API.
> > I'm reading this comment carefully for a suggestion as to how to "fix"
> > things, but it seems totally obfuscated.
>
> Editor should be unconditionally initialized if it's NULL, and WM_NCDESTROY
> should be made an explicit case in the 'switch' statement.

Of course with appropriate test cases demonstrating that this is
actually the correct behavior...

[PATCH] riched20: Process WM_GETMINMAXINFO which arrives before WM_NCCREATE.

[Austin Lund]

> > > I'm reading this comment carefully for a suggestion as to how to "fix"
> > > things, but it seems totally obfuscated.
> >
> > Editor should be unconditionally initialized if it's NULL, and WM_NCDESTROY
> > should be made an explicit case in the 'switch' statement.
>
> Of course with appropriate test cases demonstrating that this is
> actually the correct behavior...

This unconditional initialization isn't what windows does.  Windows
only creates something at GetWindowLongPtr(hwnd,0) when WM_NCCREATE is
called.  I'm not sure how to start to write a test for that, but I
have an app which uses hooks to show this (see below).

Also if the editor is initialized on any window message, is there an
easy way to know which return value is an error and which isn't?  It
seems that WM_GETMINMAXINFO's error return value is the opposite to
what WM_NCCREATE's is.

-**-

#include <windows.h>
#include <richedit.h>
#include <stdio.h>

LRESULT WINAPI WndProc(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam) {

return DefWindowProc(hwnd, message, wParam, lParam);
}

LRESULT CALLBACK CallWndProc(int nCode, WPARAM wParam, LPARAM lParam) {
  CWPSTRUCT *cwpstruct;
  cwpstruct = (CWPSTRUCT *)lParam;
  printf("Called WndProc.  hwnd: %x, msg: %x, wParam %x, lParam %x\n",
cwpstruct->hwnd, cwpstruct->message, cwpstruct->wParam,
cwpstruct->lParam);
  printf("Test extra info: %x\n", GetWindowLongPtr(cwpstruct->hwnd,0));
  if (cwpstruct->message == WM_GETMINMAXINFO) {
    MINMAXINFO *minmax = (MINMAXINFO *)cwpstruct->lParam;
    printf("WM_GETMINMAXINFO\n");
    printf("ptMaxSize %d, %d\n", minmax->ptMaxSize.x, minmax->ptMaxSize.y);
  }
  return CallNextHookEx(NULL, nCode, wParam, lParam);
}

LRESULT CALLBACK CallWndProcRet(int nCode, WPARAM wParam, LPARAM lParam) {
  CWPRETSTRUCT *cwpretstruct;
  cwpretstruct = (CWPRETSTRUCT *)lParam;
  printf("Returned WndProc %x.  hwnd: %x, msg: %x, wParam %x, lParam
%x\n", cwpretstruct->lResult, cwpretstruct->hwnd,
cwpretstruct->message, cwpretstruct->wParam, cwpretstruct->lParam);
  printf("Test extra info (RET): %x\n", GetWindowLongPtr(cwpretstruct->hwnd,0));
  return CallNextHookEx(NULL, nCode, wParam, lParam);
}

int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR
lpszCmdParam, int nCmdShow) {
  HWND hwnd;
  MSG msg;

LoadLibrary("riched20.dll");

SetWindowsHookEx(WH_CALLWNDPROC, &CallWndProc ,NULL ,GetCurrentThreadId());
  SetWindowsHookEx(WH_CALLWNDPROCRET, &CallWndProcRet ,NULL
,GetCurrentThreadId());

hwnd = CreateWindow(RICHEDIT_CLASS, "Richedit Sample", WS_VISIBLE,
CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT, NULL,
NULL, hInstance, NULL);

printf("hwnd %x\n", hwnd);
  if (hwnd == NULL) {
    printf("Error: %d\n", GetLastError());
    return 1;
  }

while(GetMessage(&msg, NULL, 0, 0)) {
    printf("Message: hwnd: %x, msg: %x\n",msg.hwnd, msg.message);
    TranslateMessage(&msg);
    DispatchMessage(&msg);
  }

return msg.wParam;
  return 0;

}

-**-

Sample output from WinXP:

Called WndProc.  hwnd: 20166, msg: 24, wParam 0, lParam 22f8c0
Test extra info: 0
WM_GETMINMAXINFO
ptMaxSize 1030, 774
Returned WndProc 0.  hwnd: 20166, msg: 24, wParam 0, lParam 22f8c0
Test extra info (RET): 0
Called WndProc.  hwnd: 20166, msg: 81, wParam 0, lParam 22f8a4
Test extra info: 0
Returned WndProc 1.  hwnd: 20166, msg: 81, wParam 0, lParam 22f8a4
Test extra info (RET): 2443b0
Called WndProc.  hwnd: 20166, msg: 83, wParam 0, lParam 22f8e0
Test extra info: 2443b0
Returned WndProc 0.  hwnd: 20166, msg: 83, wParam 0, lParam 22f8e0
Test extra info (RET): 2443b0
Called WndProc.  hwnd: 20166, msg: 1, wParam 0, lParam 22f8a4
Test extra info: 2443b0
Called WndProc.  hwnd: 20166, msg: 7c, wParam fffffff0, lParam 22ecc0
Test extra info: 2443b0
Returned WndProc 0.  hwnd: 20166, msg: 7c, wParam fffffff0, lParam 22ecc0
Test extra info (RET): 2443b0
Called WndProc.  hwnd: 20166, msg: 7d, wParam fffffff0, lParam 22ecc0
Test extra info: 2443b0
Called WndProc.  hwnd: 20166, msg: d, wParam 208, lParam 22e068
Test extra info: 2443b0
Returned WndProc f.  hwnd: 20166, msg: d, wParam 208, lParam 22e068
Test extra info (RET): 2443b0
Returned WndProc 0.  hwnd: 20166, msg: 7d, wParam fffffff0, lParam 22ecc0
Test extra info (RET): 2443b0
...

--

[PATCH] riched20: Process WM_GETMINMAXINFO which arrives before WM_NCCREATE.

[Dmitry Timoshkov]

Austin Lund  wrote:
> This unconditional initialization isn't what windows does.  Windows
> only creates something at GetWindowLongPtr(hwnd,0) when WM_NCCREATE is
> called.  I'm not sure how to start to write a test for that, but I
> have an app which uses hooks to show this (see below).

Then doing simple

if (!editor)
{
    if (msg == WM_NCCREATE)
    {
        /* initialize things */
        return;
    }
    else
        return DefWindowProc();
}

should be acceptable.