[PATCH 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. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5702
From: Rémi Bernon <rbernon(a)codeweavers.com> --- server/queue.c | 56 ++++++++++++++++++++++++-------------------------- 1 file changed, 27 insertions(+), 29 deletions(-) diff --git a/server/queue.c b/server/queue.c index c4b9389695f..53194028153 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; @@ -2103,8 +2096,9 @@ static int queue_keyboard_message( struct desktop *desktop, user_handle_t win, c struct message *msg; struct thread *foreground; unsigned char vkey = input->kbd.vkey; - hw_input_t hook_input = *input; + unsigned char 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 +2173,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 +2214,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 +2234,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 +2313,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 | 16 +++++++++------- 2 files changed, 11 insertions(+), 9 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 53194028153..1cb65b2768b 100644 --- a/server/queue.c +++ b/server/queue.c @@ -2098,7 +2098,7 @@ static int queue_keyboard_message( struct desktop *desktop, user_handle_t win, c unsigned char vkey = input->kbd.vkey; unsigned char hook_vkey = vkey; unsigned int message_code, time; - lparam_t lparam = input->kbd.scan << 16; + lparam_t hook_lparam; int wait; if (!(time = input->kbd.time)) time = get_tick_count(); @@ -2214,31 +2214,33 @@ 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; + msg->lparam = input->kbd.scan << 16; + hook_lparam = msg->lparam | msg->wparam; } else { unsigned int flags = 0; + lparam_t scan_lparam = (input->kbd.scan & 0xff) << 16; + 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; + msg->lparam = scan_lparam | (flags << 16); msg_data->flags |= (flags & (KF_EXTENDED | KF_ALTDOWN | KF_UP)) >> 8; + hook_lparam = scan_lparam | hook_vkey; } - /* 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->lparam |= 1u; /* repeat count */ - if (!(wait = send_hook_ll_message( desktop, msg, WH_KEYBOARD_LL, lparam | hook_vkey, sender ))) + if (!(wait = send_hook_ll_message( desktop, msg, WH_KEYBOARD_LL, hook_lparam, sender ))) queue_hardware_message( desktop, msg, 1 ); return wait; -- 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=145754 Your paranoid android. === 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
Rémi Bernon (@rbernon) commented about server/queue.c:
struct message *msg; struct thread *foreground; unsigned char vkey = input->kbd.vkey; - hw_input_t hook_input = *input; + unsigned char hook_vkey = vkey;
```suggestion:-1+0 unsigned char vkey = input->kbd.vkey, hook_vkey = vkey; ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5702#note_71114
Rémi Bernon (@rbernon) commented about server/queue.c:
unsigned char vkey = input->kbd.vkey; unsigned char hook_vkey = vkey; unsigned int message_code, time; - lparam_t lparam = input->kbd.scan << 16; + lparam_t hook_lparam;
Could we keep only hook_vkey as a hook-specific variable? Something like that: ``` lparam_t lparam = input->kbd.scan << 16; unsigned int flags = 0; /* ... */ if (input->kbd.flags & KEYEVENTF_UNICODE && !vkey) vkey = hook_vkey = VK_PACKET; else { 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; lparam &= 0xff0000; /* mask off scan code high bits for non-unicode input */ } msg->wparam = vkey; msg->lparam = (flags << 16) | lparam | 1 /* repeat count */; if (!(wait = send_hook_ll_message( desktop, msg, WH_KEYBOARD_LL, lparam | hook_vkey, sender ))) queue_hardware_message( desktop, msg, 1 ); ``` (Or using a `scan` variable, and `(scan << 16)` in place of lparam). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5702#note_71115
participants (4)
-
Alexandros Frantzis -
Alexandros Frantzis (＠afrantzis)
-
Marvin -
Rémi Bernon