[PATCH v2 0/2] MR5702: server: Fix handling of KEYEVENTF_UNICODE inputs with a non-zero vkey.
The first commit is an adaptation of https://gitlab.winehq.org/rbernon/wine/-/commit/8af64a92bc6c286e3072f4e5a906..., removing the parts that could potentially lead to functional changes. The second commit leverages the new design to provide the fix for the `unicode_vkey` input test. -- v2: server: Fix handling of KEYEVENTF_UNICODE inputs with a non-zero vkey. server: Pass the adjusted vkey to send_hook_ll_message. https://gitlab.winehq.org/wine/wine/-/merge_requests/5702
From: Rémi Bernon <rbernon(a)codeweavers.com> --- server/queue.c | 57 ++++++++++++++++++++++++-------------------------- 1 file changed, 27 insertions(+), 30 deletions(-) diff --git a/server/queue.c b/server/queue.c index c4b9389695f..629042feb93 100644 --- a/server/queue.c +++ b/server/queue.c @@ -1738,13 +1738,12 @@ static void queue_hardware_message( struct desktop *desktop, struct message *msg /* send the low-level hook message for a given hardware message */ static int send_hook_ll_message( struct desktop *desktop, struct message *hardware_msg, - const hw_input_t *input, struct msg_queue *sender ) + int id, lparam_t lparam, struct msg_queue *sender ) { struct thread *hook_thread; struct msg_queue *queue; struct message *msg; timeout_t timeout = 2000 * -10000; /* FIXME: load from registry */ - int id = (input->type == INPUT_MOUSE) ? WH_MOUSE_LL : WH_KEYBOARD_LL; if (!(hook_thread = get_first_global_hook( id ))) return 0; if (!(queue = hook_thread->queue)) return 0; @@ -1756,20 +1755,13 @@ static int send_hook_ll_message( struct desktop *desktop, struct message *hardwa msg->win = 0; msg->msg = id; msg->wparam = hardware_msg->msg; + msg->lparam = lparam; msg->x = hardware_msg->x; msg->y = hardware_msg->y; msg->time = hardware_msg->time; msg->data_size = hardware_msg->data_size; msg->result = NULL; - if (input->type == INPUT_KEYBOARD) - { - unsigned short vkey = input->kbd.vkey & 0xff; - if (input->kbd.flags & KEYEVENTF_UNICODE) vkey = VK_PACKET; - msg->lparam = (input->kbd.scan << 16) | vkey; - } - else msg->lparam = input->mouse.data << 16; - if (!(msg->data = memdup( hardware_msg->data, hardware_msg->data_size )) || !(msg->result = alloc_message_result( sender, queue, msg, timeout ))) { @@ -1996,6 +1988,7 @@ static int queue_mouse_message( struct desktop *desktop, user_handle_t win, cons struct thread *foreground; unsigned int i, time, flags; struct hw_msg_source source = { IMDT_MOUSE, origin }; + lparam_t wparam = input->mouse.data << 16; int wait = 0, x, y; static const unsigned int messages[] = @@ -2060,7 +2053,7 @@ static int queue_mouse_message( struct desktop *desktop, user_handle_t win, cons if ((device = current->process->rawinput_mouse) && (device->flags & RIDEV_NOLEGACY)) { if (flags & MOUSEEVENTF_MOVE) update_desktop_cursor_pos( desktop, win, x, y ); - update_desktop_mouse_state( desktop, flags, input->mouse.data << 16 ); + update_desktop_mouse_state( desktop, flags, wparam ); return 0; } @@ -2075,7 +2068,7 @@ static int queue_mouse_message( struct desktop *desktop, user_handle_t win, cons msg->win = get_user_full_handle( win ); msg->msg = messages[i]; - msg->wparam = input->mouse.data << 16; + msg->wparam = wparam; msg->lparam = 0; msg->x = x; msg->y = y; @@ -2084,10 +2077,10 @@ static int queue_mouse_message( struct desktop *desktop, user_handle_t win, cons /* specify a sender only when sending the last message */ if (!(flags & ((1 << ARRAY_SIZE( messages )) - 1))) { - if (!(wait = send_hook_ll_message( desktop, msg, input, sender ))) + if (!(wait = send_hook_ll_message( desktop, msg, WH_MOUSE_LL, wparam, sender ))) queue_hardware_message( desktop, msg, 0 ); } - else if (!send_hook_ll_message( desktop, msg, input, NULL )) + else if (!send_hook_ll_message( desktop, msg, WH_MOUSE_LL, wparam, NULL )) queue_hardware_message( desktop, msg, 0 ); } return wait; @@ -2102,9 +2095,9 @@ static int queue_keyboard_message( struct desktop *desktop, user_handle_t win, c struct hardware_msg_data *msg_data; struct message *msg; struct thread *foreground; - unsigned char vkey = input->kbd.vkey; - hw_input_t hook_input = *input; + unsigned char vkey = input->kbd.vkey, hook_vkey = vkey; unsigned int message_code, time; + lparam_t lparam = input->kbd.scan << 16; int wait; if (!(time = input->kbd.time)) time = get_tick_count(); @@ -2179,17 +2172,17 @@ static int queue_keyboard_message( struct desktop *desktop, user_handle_t win, c { switch (vkey) { - case VK_INSERT: hook_input.kbd.vkey = vkey = VK_NUMPAD0; break; - case VK_END: hook_input.kbd.vkey = vkey = VK_NUMPAD1; break; - case VK_DOWN: hook_input.kbd.vkey = vkey = VK_NUMPAD2; break; - case VK_NEXT: hook_input.kbd.vkey = vkey = VK_NUMPAD3; break; - case VK_LEFT: hook_input.kbd.vkey = vkey = VK_NUMPAD4; break; - case VK_CLEAR: hook_input.kbd.vkey = vkey = VK_NUMPAD5; break; - case VK_RIGHT: hook_input.kbd.vkey = vkey = VK_NUMPAD6; break; - case VK_HOME: hook_input.kbd.vkey = vkey = VK_NUMPAD7; break; - case VK_UP: hook_input.kbd.vkey = vkey = VK_NUMPAD8; break; - case VK_PRIOR: hook_input.kbd.vkey = vkey = VK_NUMPAD9; break; - case VK_DELETE: hook_input.kbd.vkey = vkey = VK_DECIMAL; break; + case VK_INSERT: hook_vkey = vkey = VK_NUMPAD0; break; + case VK_END: hook_vkey = vkey = VK_NUMPAD1; break; + case VK_DOWN: hook_vkey = vkey = VK_NUMPAD2; break; + case VK_NEXT: hook_vkey = vkey = VK_NUMPAD3; break; + case VK_LEFT: hook_vkey = vkey = VK_NUMPAD4; break; + case VK_CLEAR: hook_vkey = vkey = VK_NUMPAD5; break; + case VK_RIGHT: hook_vkey = vkey = VK_NUMPAD6; break; + case VK_HOME: hook_vkey = vkey = VK_NUMPAD7; break; + case VK_UP: hook_vkey = vkey = VK_NUMPAD8; break; + case VK_PRIOR: hook_vkey = vkey = VK_NUMPAD9; break; + case VK_DELETE: hook_vkey = vkey = VK_DECIMAL; break; default: break; } } @@ -2220,7 +2213,7 @@ static int queue_keyboard_message( struct desktop *desktop, user_handle_t win, c msg->win = get_user_full_handle( win ); msg->msg = message_code; - msg->lparam = (input->kbd.scan << 16) | 1u; /* repeat count */ + msg->lparam = lparam | 1u; /* repeat count */ if (origin == IMO_INJECTED) msg_data->flags = LLKHF_INJECTED; if (input->kbd.flags & KEYEVENTF_UNICODE && !vkey) @@ -2240,7 +2233,11 @@ static int queue_keyboard_message( struct desktop *desktop, user_handle_t win, c msg_data->flags |= (flags & (KF_EXTENDED | KF_ALTDOWN | KF_UP)) >> 8; } - if (!(wait = send_hook_ll_message( desktop, msg, &hook_input, sender ))) + /* TODO: The following is incorrect (we also need to check for not !vkey), + * but we do it to temporarily retain existing behavior */ + if (input->kbd.flags & KEYEVENTF_UNICODE) hook_vkey = VK_PACKET; + + if (!(wait = send_hook_ll_message( desktop, msg, WH_KEYBOARD_LL, lparam | hook_vkey, sender ))) queue_hardware_message( desktop, msg, 1 ); return wait; @@ -2315,7 +2312,7 @@ static void queue_pointer_message( struct pointer *pointer, int repeated ) msg->x = x; msg->y = y; - if (!send_hook_ll_message( desktop, msg, input, NULL )) + if (!send_hook_ll_message( desktop, msg, WH_MOUSE_LL, 0, NULL )) queue_hardware_message( desktop, msg, 0 ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5702
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> --- dlls/user32/tests/input.c | 4 ++-- server/queue.c | 13 +++++-------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/dlls/user32/tests/input.c b/dlls/user32/tests/input.c index f332aeddd08..c123f8fa439 100644 --- a/dlls/user32/tests/input.c +++ b/dlls/user32/tests/input.c @@ -1163,9 +1163,9 @@ static void test_SendInput_keyboard_messages( WORD vkey, WORD scan, WCHAR wch, W struct send_input_keyboard_test unicode_vkey[] = { {.scan = 0x3c0, .vkey = vkey, .flags = KEYEVENTF_UNICODE, .expect_state = {/*[vkey] = 0x80*/}, - .expect = {KEY_HOOK(WM_KEYDOWN, 0xc0, vkey, .todo_value = TRUE), KEY_MSG(WM_KEYDOWN, 0xc0, vkey, .todo_value = TRUE), WIN_MSG(WM_CHAR, wch, MAKELONG(1, 0xc0), .todo_value = TRUE), {0}}}, + .expect = {KEY_HOOK(WM_KEYDOWN, 0xc0, vkey), KEY_MSG(WM_KEYDOWN, 0xc0, vkey), WIN_MSG(WM_CHAR, wch, MAKELONG(1, 0xc0)), {0}}}, {.scan = 0x3c0, .vkey = vkey, .flags = KEYEVENTF_UNICODE | KEYEVENTF_KEYUP, - .expect = {KEY_HOOK(WM_KEYUP, 0xc0, vkey, .todo_value = TRUE), KEY_MSG(WM_KEYUP, 0xc0, vkey, .todo_value = TRUE), {0}}}, + .expect = {KEY_HOOK(WM_KEYUP, 0xc0, vkey), KEY_MSG(WM_KEYUP, 0xc0, vkey), {0}}}, {0}, }; diff --git a/server/queue.c b/server/queue.c index 629042feb93..6d4396234f2 100644 --- a/server/queue.c +++ b/server/queue.c @@ -2098,6 +2098,7 @@ static int queue_keyboard_message( struct desktop *desktop, user_handle_t win, c unsigned char vkey = input->kbd.vkey, hook_vkey = vkey; unsigned int message_code, time; lparam_t lparam = input->kbd.scan << 16; + unsigned int flags = 0; int wait; if (!(time = input->kbd.time)) time = get_tick_count(); @@ -2213,29 +2214,25 @@ static int queue_keyboard_message( struct desktop *desktop, user_handle_t win, c msg->win = get_user_full_handle( win ); msg->msg = message_code; - msg->lparam = lparam | 1u; /* repeat count */ if (origin == IMO_INJECTED) msg_data->flags = LLKHF_INJECTED; if (input->kbd.flags & KEYEVENTF_UNICODE && !vkey) { - msg->wparam = VK_PACKET; + vkey = hook_vkey = VK_PACKET; } else { - unsigned int flags = 0; if (input->kbd.flags & KEYEVENTF_EXTENDEDKEY) flags |= KF_EXTENDED; /* FIXME: set KF_DLGMODE and KF_MENUMODE when needed */ if (input->kbd.flags & KEYEVENTF_KEYUP) flags |= KF_REPEAT | KF_UP; else if (desktop->keystate[vkey] & 0x80) flags |= KF_REPEAT; - msg->wparam = vkey; - msg->lparam |= flags << 16; + lparam &= 0xff0000; /* mask off scan code high bits for non-unicode input */ msg_data->flags |= (flags & (KF_EXTENDED | KF_ALTDOWN | KF_UP)) >> 8; } - /* TODO: The following is incorrect (we also need to check for not !vkey), - * but we do it to temporarily retain existing behavior */ - if (input->kbd.flags & KEYEVENTF_UNICODE) hook_vkey = VK_PACKET; + msg->wparam = vkey; + msg->lparam = (flags << 16) | lparam | 1u /* repeat count */; if (!(wait = send_hook_ll_message( desktop, msg, WH_KEYBOARD_LL, lparam | hook_vkey, sender ))) queue_hardware_message( desktop, msg, 1 ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5702
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=145762 Your paranoid android. === w11pro64 (32 bit report) === user32: input.c:4127: Test failed: button_up_hwnd 0: got MSG_TEST_WIN hwnd 00050040, msg WM_LBUTTONUP, wparam 0, lparam 0x320032 input.c:4127: Test failed: button_up_hwnd 1 (missing): MSG_TEST_WIN hwnd 00050040, WM_LBUTTONUP, wparam 0, lparam 0x320032 input.c:4199: Test failed: layered 0: button_up_hwnd_todo 0: got MSG_TEST_WIN hwnd 00050040, msg WM_LBUTTONUP, wparam 0, lparam 0x320032 input.c:4199: Test failed: layered 0: button_up_hwnd_todo 1 (missing): MSG_TEST_WIN hwnd 00050040, WM_LBUTTONUP, wparam 0, lparam 0x320032 input.c:4179: Test failed: layered 4: button_up_hwnd_todo 0: got MSG_TEST_WIN hwnd 00050040, msg WM_LBUTTONUP, wparam 0, lparam 0x320032 input.c:4179: Test failed: layered 4: button_up_hwnd_todo 1 (missing): MSG_TEST_WIN hwnd 00050040, WM_LBUTTONUP, wparam 0, lparam 0x320032 input.c:4179: Test failed: layered 6: button_up_hwnd_todo 0: got MSG_TEST_WIN hwnd 001100F0, msg WM_LBUTTONUP, wparam 0, lparam 0x320032 input.c:4179: Test failed: layered 6: button_up_hwnd_todo 1 (missing): MSG_TEST_WIN hwnd 001100F0, WM_LBUTTONUP, wparam 0, lparam 0x320032 input.c:4230: Test failed: button_up_hwnd 0: got MSG_TEST_WIN hwnd 00050040, msg WM_LBUTTONUP, wparam 0, lparam 0x320032 input.c:4230: Test failed: button_up_hwnd 1 (missing): MSG_TEST_WIN hwnd 00050040, WM_LBUTTONUP, wparam 0, lparam 0x320032 === debian11b (32 bit WoW report) === user32: input.c:4067: Test succeeded inside todo block: button_down_hwnd_todo 1: got MSG_TEST_WIN hwnd 0004009A, msg WM_LBUTTONDOWN, wparam 0x1, lparam 0x320032 === debian11b (build log) === 009c:err:winediag:gnutls_process_attach failed to load libgnutls, no support for encryption
On Wed May 22 12:29:28 2024 +0000, Alexandros Frantzis wrote:
changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/5702/diffs?diff_id=114711&start_sha=3686f6da09806ec3378646b3ec46c547c26ed787#14b74bf2d7f44f52a31e4ee1baa51371f14df769_2099_2098) Done in v2.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5702#note_71133
On Wed May 22 12:29:28 2024 +0000, Alexandros Frantzis wrote:
changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/5702/diffs?diff_id=114711&start_sha=3686f6da09806ec3378646b3ec46c547c26ed787#14b74bf2d7f44f52a31e4ee1baa51371f14df769_2101_2100) Done in v2 (kept `lparam`). I have kept the `msg_data->flags` setting in the if statement, but we can move it out if preferred.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5702#note_71134
This merge request was approved by Rémi Bernon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5702
participants (4)
-
Alexandros Frantzis -
Alexandros Frantzis (＠afrantzis)
-
Marvin -
Rémi Bernon