[PATCH v5 0/1] MR4633: win32u: 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.

--
  v5: win32u: Fix key state not updating before WH_KEYBOARD hook

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

[PATCH v5 1/1] win32u: 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 | 46 +++++++++++++++++++++++++++++++++++++++
 dlls/win32u/message.c     |  5 +++--
 2 files changed, 49 insertions(+), 2 deletions(-)

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

+static BOOL keyboard_hook_called;
+static BOOL expect_pressed_key;
+static LRESULT CALLBACK keyboard_hook( int code, WPARAM wparam, LPARAM lparam )
+{
+    SHORT key_state;
+
+    keyboard_hook_called = TRUE;
+    key_state = GetKeyState(wparam);
+    if (expect_pressed_key)
+        ok((key_state & 0xFF80) != 0, "Expected key state to be DOWN. Key state: %hx\n", key_state);
+    else
+        ok((key_state & 0xFF80) == 0, "Expected key state to be UP. Key state: %hx\n", 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());
+
+
+    keybd_event(VK_TAB, 0, 0, 0);
+    WaitMessage();
+
+    expect_pressed_key = FALSE;
+    keyboard_hook_called = FALSE;
+    PeekMessageA(&message, 0, WM_KEYDOWN, WM_KEYDOWN, PM_NOREMOVE);
+    ok(keyboard_hook_called == TRUE, "Keyboard hook was not called.\n");
+
+    expect_pressed_key = TRUE;
+    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 +5442,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 v5 1/1] win32u: 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=141015

Your paranoid android.

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

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

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

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