Re: [PATCH] winex11.drv, winemac.drv: Only send WM_CANCELMODE if a menu is active.

[Zebediah Figura]

On 11/6/19 11:44 AM, Rémi Bernon wrote:
> From: Zebediah Figura
>
> The ShowWindow(SW_SHOWMINIMIZED):overlapped test shows that the message
> should not be sent in general.
>
> The additional test -which doesn't pass yet on wine for other reasons-
> shows that we should still send the message if a menu is active.
> Signed-off-by: Rémi Bernon
>
> This supersedes https://source.winehq.org/patches/data/172893. The other
> user32/tests patches in the series should still apply.
>
> Notes:
>      While testing I could see some cases when WM_CANCELMODE is not
>      sent on some Windows versions although a menu is open -and it
>      then stays open- but it also depends on the cursor location and
>      the previous keyboard events that have been sent, so it's not
>      reliable at all. Moving the cursor on top of a window beforehand
>      seems to give consistent results.
>
> dlls/user32/tests/msg.c   | 71 ++++++++++++++++++++++++++++++++++++++-
>   dlls/winemac.drv/window.c |  7 +++-
>   dlls/winex11.drv/event.c  |  8 ++++-
>   3 files changed, 83 insertions(+), 3 deletions(-)

Interesting deduction. I remember testing even with a modal menu and 
finding that WM_CANCELMODE was not sent (and that the menu then stayed 
open), but I guess I was running into one of those corner cases.

I feel like this probably shouldn't have my name on it as author, given 
the quantity of added code over my original patch, but I don't care too 
strongly.
> diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c
> index 477661ab623..59721056acd 100644
> --- a/dlls/user32/tests/msg.c
> +++ b/dlls/user32/tests/msg.c
> @@ -1927,6 +1927,41 @@ static const struct message WmSHOWNATopInvisible[] = {
>       { 0 }
>   };
>
> +static const struct message WmTrackPopupMenuMinimizeWindow[] = {
> - { HCBT_CREATEWND, hook },
> - { WM_ENTERMENULOOP, sent|wparam|lparam, TRUE, 0 },
> - { WM_INITMENU, sent|lparam, 0, 0 },
> - { WM_INITMENUPOPUP, sent|lparam, 0, 0 },
> - { 0x0093, sent|optional },
> - { 0x0094, sent|optional },
> - { 0x0094, sent|optional },
> - { WM_ENTERIDLE, sent|wparam, 2 },
> - { HCBT_MINMAX, hook },
> - { HCBT_SETFOCUS, hook },
> - { WM_KILLFOCUS, sent|wparam, 0 },
> - { WM_GETTEXT, sent|optional },
> - { WM_WINDOWPOSCHANGING, sent },
> - { WM_GETMINMAXINFO, sent|defwinproc },
> - { WM_NCCALCSIZE, sent|wparam|optional, 1 },
> - { WM_WINDOWPOSCHANGED, sent },
> - { WM_MOVE, sent|defwinproc },
> - { WM_SIZE, sent|defwinproc },
> - { WM_GETTEXT, sent|optional },
> - { WM_NCCALCSIZE, sent|wparam|optional, 1 },
> - { WM_CANCELMODE, sent },
> - { WM_CAPTURECHANGED, sent|defwinproc },
> - { HCBT_DESTROYWND, hook },
> - { WM_UNINITMENUPOPUP, sent|defwinproc|lparam, 0, 0 },
> - { WM_MENUSELECT, sent|defwinproc|wparam|lparam, 0xffff0000, 0 },
> - { WM_EXITMENULOOP, sent|defwinproc|wparam|lparam, 1, 0 },
> - { WM_NCACTIVATE, sent },
> - { WM_GETTEXT, sent|defwinproc|optional },
> - { WM_GETTEXT, sent|defwinproc|optional },
> - { WM_ACTIVATE, sent },
> - { WM_ACTIVATEAPP, sent|wparam, 0 },
> - { 0 }
>
> +};
> - static const struct message WmTrackPopupMenu[] = {
>     { HCBT_CREATEWND, hook },
>     { WM_ENTERMENULOOP, sent|wparam|lparam, TRUE, 0 },
>
> @@ -5163,7 +5198,7 @@ static void test_messages(void)
>   ShowWindow(hwnd, SW_MINIMIZE);
>   flush_events();
> - ok_sequence(WmShowMinOverlappedSeq, "ShowWindow(SW_SHOWMINIMIZED):overlapped", TRUE);
> -
>
> ok_sequence(WmShowMinOverlappedSeq, "ShowWindow(SW_SHOWMINIMIZED):overlapped", FALSE);
>  flush_sequence();
>
> if (GetWindowLongW( hwnd, GWL_STYLE ) & WS_MINIMIZE)
>
> @@ -17279,6 +17314,25 @@ static void test_layered_window(void)
>
> static HMENU hpopupmenu;
>
> +static LRESULT WINAPI minimize_popup_proc(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam)
> +{
> - LRESULT ret;
> -
> - if (ignore_message( message )) return 0;
> - ret = MsgCheckProc( FALSE, hwnd, message, wParam, lParam );
> -
> - switch (message) {
> - case WM_ENTERIDLE:
> -
>    ShowWindow(hwnd, SW_MINIMIZE);
> -
>    break;
> - case WM_TIMER:
> -
>    EndMenu();
> -
>    break;
> - }
> -
> - return ret;
>
> +}
> - static LRESULT WINAPI cancel_popup_proc(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam)
> {
>     if (ignore_message( message )) return 0;
>
> @@ -17361,6 +17415,21 @@ static void test_TrackPopupMenu(void)
>       ok_sequence(WmTrackPopupMenuAbort, "WmTrackPopupMenuAbort", TRUE);
>       ok(ret == TRUE, "TrackPopupMenu failed\n");
> -
>
> SetWindowLongPtrA( hwnd, GWLP_WNDPROC, (LONG_PTR)minimize_popup_proc);
> -
> -
>
> /* set cursor over the window, otherwise the WM_CANCELMODE message may not always be sent */
> -
>
> SetCursorPos( 0, 0 );
> -
>
> ShowWindow( hwnd, SW_SHOW );
> -
> -
>
> flush_events();
> -
>
> flush_sequence();
> -
>
> SetTimer( hwnd, TIMER_ID, 500, NULL );
> -
>
> ret = TrackPopupMenu( hpopupmenu, 0, 100,100, 0, hwnd, NULL );
> -
>
> ok_sequence( WmTrackPopupMenuMinimizeWindow, "TrackPopupMenuMinimizeWindow", TRUE );
> -
>
> ok( ret == 1, "TrackPopupMenu failed with error %i\n", GetLastError() );
> -
>
> KillTimer( hwnd, TIMER_ID );
> -
>
> ShowWindow( hwnd, SW_RESTORE );
> -
> SetWindowLongPtrA( hwnd, GWLP_WNDPROC, (LONG_PTR)cancel_popup_proc);
>
> SetCapture(hwnd);
>
> diff --git a/dlls/winemac.drv/window.c b/dlls/winemac.drv/window.c
> index 605c0c87f16..8091f5249fe 100644
> --- a/dlls/winemac.drv/window.c
> +++ b/dlls/winemac.drv/window.c
> @@ -2368,13 +2368,18 @@ void macdrv_window_got_focus(HWND hwnd, const macdrv_event *event)
>    */
>   void macdrv_window_lost_focus(HWND hwnd, const macdrv_event *event)
>   {
> -
>
> GUITHREADINFO threadinfo;
>  if (!hwnd) return;
>
> TRACE("win %p/%p fg %p\n", hwnd, event->window, GetForegroundWindow());
>
> if (hwnd == GetForegroundWindow())
>  {
> -
>    SendMessageW(hwnd, WM_CANCELMODE, 0, 0);
> -
>    threadinfo.cbSize = sizeof(threadinfo);
> -
>    GetGUIThreadInfo(0, &threadinfo);
> -
>    if (threadinfo.flags & (GUI_INMENUMODE|GUI_INMOVESIZE|GUI_POPUPMENUMODE|GUI_SYSTEMMENUMODE))
> -
>        SendMessageW(hwnd, WM_CANCELMODE, 0, 0);
> -
>     if (hwnd == GetForegroundWindow())
>         SetForegroundWindow(GetDesktopWindow());
> }
>
> diff --git a/dlls/winex11.drv/event.c b/dlls/winex11.drv/event.c
> index dd8837c11da..c18b97f5360 100644
> --- a/dlls/winex11.drv/event.c
> +++ b/dlls/winex11.drv/event.c
> @@ -819,6 +819,7 @@ static void focus_out( Display *display , HWND hwnd )
>    {
>       HWND hwnd_tmp;
>       Window focus_win;
> - GUITHREADINFO threadinfo;
>  int revert;
>  XIC xic;
>
> @@ -833,7 +834,12 @@ static void focus_out( Display *display , HWND hwnd )
>           return;
>       }
>       if (hwnd != GetForegroundWindow()) return;
> - SendMessageW( hwnd, WM_CANCELMODE, 0, 0 );
> -
> -
>
> threadinfo.cbSize = sizeof(threadinfo);
> -
>
> GetGUIThreadInfo(0, &threadinfo);
> -
> -
>
> if (threadinfo.flags & (GUI_INMENUMODE|GUI_INMOVESIZE|GUI_POPUPMENUMODE|GUI_SYSTEMMENUMODE))
> -
>    SendMessageW( hwnd, WM_CANCELMODE, 0, 0 );
>
> /* don't reset the foreground window, if the window which is
>    getting the focus is a Wine window */
>
> --
> 2.24.0.rc2