[PATCH] user32: Allow WM_CAPTURECHANGE indicating SetCapture() parameter.

[Christopher Thielen]

Attached is my analysis of Wine's usage of WM_CAPTURECHANGED as 
requested by Alexandre.

Reading the code has revealed the same logic change I originally 
proposed -- allowing a WM_CAPTURECHANGED message to be sent to a window 
losing focus if it was also the window gaining focus -- is needed in the 
Mac driver as well. The Mac driver logic looks copied out of 
dlls/user32/input.c.

I would appreciate if someone could double-check my analysis in a few 
areas based on expertise:

dlls/comctl32/toolbar.c
dlls/comctl32/trackbar.c
dlls/user32/button.c
programs/regedit/childwnd.c

If no one has any concerns after a few days, I'll resubmit my patch with 
the additional change mimicked for the Mac driver.

On 11/25/2015 01:51 PM, Chris Thielen wrote:
> Thanks Alexandre, that seemswise.
>
> I'll get a review ready after 1.8.
>
> On Monday, November 23, 2015, Alexandre Julliard <julliard at winehq.org
> <mailto:julliard at winehq.org>> wrote:
>
>     Christopher Thielen <cthielen at gmail.com <javascript:;>> writes:
>
>      > Fixes https://bugs.winehq.org/show_bug.cgi?id=13683
>      >
>      > A window may be notified with WM_CAPTURECHANGED about itself
>      > gaining mouse capture if it calls SetCapture() twice.
>
>     You'd need to review all places where we handle that message, I don't
>     think we've taken that behavior into account.
>
>     Because of this risk, I'm afraid it will have to wait until after code
>     freeze.
>
>     --
>     Alexandre Julliard
>     julliard at winehq.org <javascript:;>
>
-------------- next part --------------
Analysis of WM_CAPTURECHANGED usage in Wine to predict if an incoming correctedness patch will cause any side-effects.

The patch creates additional WM_CAPTURECHANGED messages in the case where a window losing focus is the same as the window gaining focus. This more correctly mimics Windows' behavior. Wine did not previously do this, causing a long-standing though isolated bug.

dlls/user.exe16/message.c:    case WM_CAPTURECHANGED:

OK. Code is simply specifying WM_CAPTURECHANGED to ensure it is ignored.

dlls/winemac.drv/window.c:            SendMessageW(previous, WM_CAPTURECHANGED, 0, (LPARAM)hwnd);

Appears to be in need of the same patch as I'm suggesting. Any idea why this code appears duplicated? Something about OS X having a different window server than *nix?

dlls/comctl32/tests/button.c:    { WM_CAPTURECHANGED, sent|wparam|defwinproc, 0 },

Should be fine. WM_CAPTURECHANGED is simply here as part of a test message sequence.

dlls/comctl32/tests/monthcal.c:    { WM_CAPTURECHANGED, sent|wparam|lparam|defwinproc, 0, 0},

Same as above.

dlls/comctl32/toolbar.c:	case WM_CAPTURECHANGED:

Probably fine? Appears to be a ToolbarProc translating window messages into toolbar-based equivalents.

dlls/comctl32/trackbar.c:    case WM_CAPTURECHANGED:

Same as above (for trackbars).

dlls/user32/button.c:    case WM_CAPTURECHANGED:
dlls/user32/button.c:        TRACE("WM_CAPTURECHANGED %p\n", hWnd);

Both lines part of the same button procedure. WM_CAPTURECHANGED causes get_button_state() to be called here and, if the button is pressed, causes the state to gain BUTTON_NSTATES.

Somebody who knows user32/button.c may want to comment.

dlls/user32/tests/msg.c:    { WM_CAPTURECHANGED, sent },
dlls/user32/tests/msg.c:    { WM_CAPTURECHANGED, sent },
dlls/user32/tests/msg.c:    { WM_CAPTURECHANGED, sent },
dlls/user32/tests/msg.c:    { WM_CAPTURECHANGED, sent },
dlls/user32/tests/msg.c:    { WM_CAPTURECHANGED, sent },
dlls/user32/tests/msg.c:    { WM_CAPTURECHANGED, sent|wparam|defwinproc, 0 },
dlls/user32/tests/msg.c:    { WM_CAPTURECHANGED, sent|wparam|defwinproc, 0 },
dlls/user32/tests/msg.c:    { WM_CAPTURECHANGED, sent|wparam|defwinproc, 0 },
dlls/user32/tests/msg.c:    { WM_CAPTURECHANGED, sent|defwinproc },
dlls/user32/tests/msg.c:    { WM_CAPTURECHANGED, sent|defwinproc },
dlls/user32/tests/msg.c:    { WM_CAPTURECHANGED, sent|defwinproc },
dlls/user32/tests/msg.c:    { WM_CAPTURECHANGED, sent|defwinproc|wparam|lparam, 0, 0 },
dlls/user32/tests/msg.c:	case WM_CAPTURECHANGED:
dlls/user32/tests/msg.c:    { WM_CAPTURECHANGED, sent|wparam|lparam, 0, WND_POPUP_ID }, /* popup */
dlls/user32/tests/msg.c:    { WM_CAPTURECHANGED, sent|defwinproc },
dlls/user32/tests/msg.c:    { WM_CAPTURECHANGED, sent|defwinproc },
dlls/user32/tests/msg.c:    { WM_CAPTURECHANGED, sent|wparam|lparam|defwinproc, 0, 0 },
dlls/user32/tests/msg.c:    case WM_CAPTURECHANGED:

OK. All part of pre-defined message patterns with the exception of the two cases. user32 tests still pass with patch.

dlls/user32/tests/edit.c:static BOOL got_wm_capturechanged = FALSE;
dlls/user32/tests/edit.c:        case WM_CAPTURECHANGED:
dlls/user32/tests/edit.c:                got_wm_capturechanged = TRUE;
dlls/user32/tests/edit.c:    ok(got_wm_capturechanged, "main window capture did not change\n");

OK? Test continues to pass.

dlls/user32/tests/win.c:        case WM_CAPTURECHANGED:

OK? Test continues to pass.

dlls/user32/spy.c:    "WM_CAPTURECHANGED",

OK. Just a definition.

dlls/user32/input.c:            SendMessageW( previous, WM_CAPTURECHANGED, 0, (LPARAM)hwnd );

OK. This is what my original patch addressed.

include/winuser.rh:#define WM_CAPTURECHANGED      0x0215

OK. Just a definition.

programs/regedit/childwnd.c:    case WM_CAPTURECHANGED:

Unsure. Looks innocuous to me but somebody who knows regedit better could attest.

programs/taskmgr/graph.c:    case WM_CAPTURECHANGED:

OK. Simply filtering it out.

programs/taskmgr/graphctl.c:    case WM_CAPTURECHANGED:

Same as above.

tools/winapi/winapi_module_user.pm:    WM_CAPTURECHANGED => {

OK. Just a definition.