Re: [1/3] riched20: Implement handling of WM_KEYDOWN VK_RETURN in a dialog mode.

[Dylan Smith]

On Tue, Sep 1, 2009 at 4:18 AM, Sergey Khodych  wrote:
> @@ -2145,6 +2145,31 @@ ME_KeyDown(ME_TextEditor *editor, WORD nKey)
>       ME_SendRequestResize(editor, FALSE);
>       return TRUE;
>     case VK_RETURN:
> - if (editor->bDialogMode)
> - {
> - DWORD dwStyle;
> -
> - if (ctrl_is_down)
> - return TRUE;
> -
> - dwStyle = GetWindowLongW(editor->hWnd, GWL_STYLE);
> - if (!(dwStyle & ES_WANTRETURN))
> - {
> - DWORD dw;
> - dw = SendMessageW(editor->hwndParent, DM_GETDEFID, 0, 0);
> - if (HIWORD(dw) == DC_HASDEFID)
> - {
> - HWND hwDefCtrl = GetDlgItem(editor->hwndParent, LOWORD(dw));
> - if (hwDefCtrl)
> - {
> - SendMessageW(editor->hwndParent, WM_NEXTDLGCTL, (WPARAM)hwDefCtrl, (LPARAM)TRUE);
> - PostMessageW(hwDefCtrl, WM_KEYDOWN, VK_RETURN, 0);
> - }
> - }
> - return TRUE;
> - }
> - }
> -
>
> if (editor->styleFlags & ES_MULTILINE)
>       {
>         ME_Cursor cursor = editor->pCursors[0];

1. editor->hWnd may be NULL for windowless richedit controls, but you
code doesn't seem to take that into consideration.
2. The style flags that you get from GetWindowLongW should probably
come from the cached values in editor->styleFlags.
3. Is there any need for storing the hwndParent in editor.  Couldn't
the value be obtained by using GetWindowLongW using GWL_HWNDPARENT
after making sure that editor->hWnd is non-NULL (i.e. windowed mode).

[1/3] riched20: Implement handling of WM_KEYDOWN VK_RETURN in a dialog mode.

[Dylan Smith]

> - Is there any need for storing the hwndParent in editor.  Couldn't
>
> the value be obtained by using GetWindowLongW using GWL_HWNDPARENT
> after making sure that editor->hWnd is non-NULL (i.e. windowed mode).

Scratch that point, I see that it is tested for using SetParent in the
tests after creating the richedit control.

Other things I noticed:
> - r = SendMessage(hwRichEdit, WM_GETDLGCODE, (WPARAM)NULL, (LPARAM)NULL);

Avoid superfluous casts.  In this case it is preferred to use 0
instead of casting NULL.  If the cast isn't needed, then leave it out
(See: http://wiki.winehq.org/SuperfluousCasts).
> +struct dialog_mode_messages
> +{
> - int wm_getdefid, wm_close, wm_nextdlgctl;
>
> +};
> -
>
> +static struct dialog_mode_messages dm_messages;
> -
>
> +static void zero_dm_messages(void)
> +{
> - dm_messages.wm_close = 0;
> - dm_messages.wm_getdefid = 0;
> - dm_messages.wm_nextdlgctl =0;
>
> +}

Couldn't memset(dm_messages, 0, sizeof(dm_messages)) be used instead
of the special zero_dm_messages function.

[1/3] riched20: Implement handling of WM_KEYDOWN VK_RETURN in a dialog mode.

[Sergey Khodych]

> - editor->hWnd may be NULL for windowless richedit controls, but you
>
> code doesn't seem to take that into consideration.
> 2. The style flags that you get from GetWindowLongW should probably
> come from the cached values in editor->styleFlags.

The main reason why GetWindowLongW is used there is that 
editor->styleFlags actually isn't window style and doesn't contain 
ES_WANTRETURN. What's the proper way to get actually window style in 
this case?

[1/3] riched20: Implement handling of WM_KEYDOWN VK_RETURN in a dialog mode.

[Dylan Smith]

On Wed, Sep 2, 2009 at 1:34 PM, Sergey Khodych wrote:
> > - editor->hWnd may be NULL for windowless richedit controls, but you
> >
> > code doesn't seem to take that into consideration.
> > 2. The style flags that you get from GetWindowLongW should probably
> > come from the cached values in editor->styleFlags.
>
> The main reason why GetWindowLongW is used there is that editor->styleFlags
> actually isn't window style and doesn't contain ES_WANTRETURN. What's the
> proper way to get actually window style in this case?

editor->styleFlags should cache any window styles that are needed on
window creation, but it seems ES_WANTRETURN was missed because it
wasn't used and there was not ITextHost property to associate it with.
 This can be handled where the extended styles are obtained as shown
in the following diff.  I tested and made sure the ES_WANTRETURN style
does get cached by native richedit by trying to change it with
SetWindowLong, so this is necessary.

diff --git a/dlls/riched20/txthost.c b/dlls/riched20/txthost.c
index ddf3d76..d3a7330 100644
--- a/dlls/riched20/txthost.c
+++ b/dlls/riched20/txthost.c
@@ -58,6 +58,7 @@ ITextHost *ME_CreateTextHost(HWND hwnd, BOOL
bEmulateVersion10)
         texthost->bEmulateVersion10 = bEmulateVersion10;

editor = ME_MakeEditor((ITextHost*)texthost, bEmulateVersion10);
+        editor->styleFlags |= GetWindowLongW(hwnd, GWL_STYLE) & ES_WANTRETURN;
         editor->exStyleFlags = GetWindowLongW(hwnd, GWL_EXSTYLE);
         editor->hWnd = hwnd; /* FIXME: Remove editor's dependence on hWnd */
         SetWindowLongPtrW(hwnd, 0, (LONG_PTR)editor);