[PATCH v4 0/1] MR4633: winu32: Fix key state not updating before WH_KEYBOARD hook

[Hans Lehnert (＠hanslehnert)]

If a message is removed from the queue with PeekMessage/GetMessage,
the key state should get updated before calling the WH_KEYBOARD hooks.

--
  v4: winu32: Fix key state not updating before WH_KEYBOARD hook

https://gitlab.winehq.org/wine/wine/-/merge_requests/4633

[PATCH v4 1/1] winu32: Fix key state not updating before WH_KEYBOARD hook

[Hans Lehnert]

From: Hans Lehnert

If a message is removed from the queue with PeekMessage/GetMessage,
the key state should get updated before calling the WH_KEYBOARD hooks.
---
 dlls/user32/tests/input.c | 43 +++++++++++++++++++++++++++++++++++++++
 dlls/win32u/message.c     |  5 +++--
 2 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/dlls/user32/tests/input.c b/dlls/user32/tests/input.c
index 6a28bd24301..e63855cc7bd 100644
--- a/dlls/user32/tests/input.c
+++ b/dlls/user32/tests/input.c
@@ -1583,6 +1583,48 @@ done:
     SetCursorPos(pt_org.x, pt_org.y);
 }

+static BOOL keyboard_hook_called;
+static SHORT expected_key_state;
+static LRESULT CALLBACK keyboard_hook( int code, WPARAM wparam, LPARAM lparam )
+{
+    SHORT key_state;
+
+    keyboard_hook_called = TRUE;
+    key_state = GetKeyState(wparam);
+    ok(expected_key_state == key_state, "Wrong key state. Expected: %hx. Actual %hx\n", expected_key_state, key_state);
+
+    return CallNextHookEx( 0, code, wparam, lparam );
+}
+
+static void test_keyboard_hook(void)
+{
+    HWND hwnd;
+    HHOOK hook;
+    MSG message;
+
+    hwnd = CreateWindowA("static", "Title", WS_OVERLAPPEDWINDOW | WS_VISIBLE,
+                        10, 10, 200, 200, NULL, NULL, NULL, NULL);
+
+    hook = SetWindowsHookExA(WH_KEYBOARD, keyboard_hook, GetModuleHandleA(0), GetCurrentThreadId());
+
+    expected_key_state = GetKeyState(VK_TAB) & 0x01;
+
+    keybd_event(VK_TAB, 0, 0, 0);
+    WaitMessage();
+
+    keyboard_hook_called = FALSE;
+    PeekMessageA(&message, 0, WM_KEYDOWN, WM_KEYDOWN, PM_NOREMOVE);
+    ok(keyboard_hook_called == TRUE, "Keyboard hook was not called.\n");
+
+    expected_key_state ^= 0xFF81;
+    keyboard_hook_called = FALSE;
+    PeekMessageA(&message, 0, WM_KEYDOWN, WM_KEYDOWN, PM_REMOVE);
+    ok(keyboard_hook_called == TRUE, "Keyboard hook was not called.\n");
+
+    UnhookWindowsHookEx(hook);
+    DestroyWindow(hwnd);
+}
+
 static void test_GetMouseMovePointsEx( char **argv )
 {
 #define BUFLIM  64
@@ -5397,6 +5439,7 @@ START_TEST(input)
     test_Input_mouse();
     test_keynames();
     test_mouse_ll_hook();
+    test_keyboard_hook();
     test_key_map();
     test_ToUnicode();
     test_ToAscii();
diff --git a/dlls/win32u/message.c b/dlls/win32u/message.c
index 91f836a60bd..c673034d15f 100644
--- a/dlls/win32u/message.c
+++ b/dlls/win32u/message.c
@@ -2382,6 +2382,7 @@ static BOOL process_keyboard_message( MSG *msg, UINT hw_id, HWND hwnd_filter,
             if (msg->wParam == VK_APPS && !is_menu_active())
                 NtUserPostMessage( msg->hwnd, WM_CONTEXTMENU, (WPARAM)msg->hwnd, -1 );
         }
+        accept_hardware_message( hw_id );
     }

if (call_hooks( WH_KEYBOARD, remove ? HC_ACTION : HC_NOREMOVE,
@@ -2389,10 +2390,10 @@ static BOOL process_keyboard_message( MSG *msg, UINT hw_id, HWND hwnd_filter,
     {
         /* skip this message */
         call_hooks( WH_CBT, HCBT_KEYSKIPPED, LOWORD(msg->wParam), msg->lParam, 0 );
-        accept_hardware_message( hw_id );
+        if (!remove)
+            accept_hardware_message( hw_id );
         return FALSE;
     }
-    if (remove) accept_hardware_message( hw_id );
     msg->pt = point_phys_to_win_dpi( msg->hwnd, msg->pt );

if (remove && msg->message == WM_KEYDOWN)

[PATCH v4 1/1] winu32: Fix key state not updating before WH_KEYBOARD hook

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=140957

Your paranoid android.

=== w10pro64 (32 bit report) ===

user32:
input.c:3631: Test failed: 04090409 / 04090409: WaitForSingleObject returned 0x102

=== w1064v1809 (64 bit report) ===

user32:
input.c:1617: Test failed: Keyboard hook was not called.
input.c:1622: Test failed: Keyboard hook was not called.

=== w10pro64_ja (64 bit report) ===

user32:
input.c:1622: Test failed: Keyboard hook was not called.

=== w10pro64_zh_CN (64 bit report) ===

user32:
input.c:1622: Test failed: Keyboard hook was not called.

=== debian11 (32 bit report) ===

user32:
input.c:1594: Test failed: Wrong key state. Expected: 0. Actual 1
input.c:1594: Test failed: Wrong key state. Expected: ff81. Actual ff80

=== debian11 (32 bit ar:MA report) ===

user32:
input.c:1594: Test failed: Wrong key state. Expected: 0. Actual 1
input.c:1594: Test failed: Wrong key state. Expected: ff81. Actual ff80

=== debian11 (32 bit de report) ===

user32:
input.c:1594: Test failed: Wrong key state. Expected: 0. Actual 1
input.c:1594: Test failed: Wrong key state. Expected: ff81. Actual ff80

=== debian11 (32 bit fr report) ===

user32:
input.c:1594: Test failed: Wrong key state. Expected: 0. Actual 1
input.c:1594: Test failed: Wrong key state. Expected: ff81. Actual ff80

=== debian11 (32 bit he:IL report) ===

user32:
input.c:1594: Test failed: Wrong key state. Expected: 0. Actual 1
input.c:1594: Test failed: Wrong key state. Expected: ff81. Actual ff80

=== debian11 (32 bit hi:IN report) ===

user32:
input.c:1594: Test failed: Wrong key state. Expected: 0. Actual 1
input.c:1594: Test failed: Wrong key state. Expected: ff81. Actual ff80

=== debian11 (32 bit ja:JP report) ===

user32:
input.c:1594: Test failed: Wrong key state. Expected: 0. Actual 1
input.c:1594: Test failed: Wrong key state. Expected: ff81. Actual ff80

=== debian11 (32 bit zh:CN report) ===

user32:
input.c:1594: Test failed: Wrong key state. Expected: 0. Actual 1
input.c:1594: Test failed: Wrong key state. Expected: ff81. Actual ff80

=== debian11b (32 bit WoW report) ===

user32:
input.c:1594: Test failed: Wrong key state. Expected: 0. Actual 1
input.c:1594: Test failed: Wrong key state. Expected: ff81. Actual ff80

=== debian11b (64 bit WoW report) ===

user32:
input.c:1594: Test failed: Wrong key state. Expected: 0. Actual 1
input.c:1594: Test failed: Wrong key state. Expected: ff81. Actual ff80