[PATCH v3 0/1] MR7701: winemac.drv: Only send key down events to the window's inputContext.
Sending key events through `-[NSTextInputContext handleEvent:]`, as we do in `macdrv_send_text_input_event`, results in doubled characters for certain macOS input sources. The Romaji source does this reliably, as do certain third-party sources. (The built-in dictation does it as well, but we already blacklist that - !5660 - because it remains "active" even when it's not.) I can't find this spelled out directly, but if you read between the lines, it seems clear that you're only supposed to send key *down* events to `-[NSTextInputContext handleEvent:]` and not key ups. Supporting evidence: 1. [The old documentation on the text system](https://developer.apple.com/library/archive/documentation/TextFonts/Conceptu...) only mentions `handleEvent:` being called in the context of `-keyDown:`. 2. [The guide to implementing a custom text view/NSTextInputClient](https://developer.apple.com/library/archive/documentation/TextFonts/Conceptu...) only mentions overriding `-keyDown:` and sending those events to `-handleEvent:`. 3. [iTerm only sends key down events to `-handleEvent:`](https://github.com/gnachman/iTerm2/blob/6134ea0a9d9d0fee5e7d7704fc98efec1fc7...). You have to unpack the logic a bit from there, but `-handleKeyDownEvent:...` is the only method that calls `-handleEventWithCocoa:inputContext:`, which is what (usually) calls `-handleEvent:` on the `inputContext`. 4. [The InputMethodKit method on the IMKServerInput protocol to handle NSEvents directly](https://developer.apple.com/documentation/objectivec/nsobject/1385363-handle...) only mentions getting key down events. So presumably input source authors sometimes do not check the type of the NSEvent they're receiving and process all key events equally, be they downs or ups, which results in the doubled input. This patch fixes the issues with Romaji and the problematic third-party method we've encountered. -- v3: winemac.drv: Only send key down events to the window's inputContext. https://gitlab.winehq.org/wine/wine/-/merge_requests/7701
From: Tim Clem <tclem(a)codeweavers.com> Rename and remove an argument to macdrv_send_text_input to reflect its purpose. Fixes doubled input with certain input sources. --- dlls/winemac.drv/cocoa_window.m | 10 +++++++--- dlls/winemac.drv/keyboard.c | 10 +++++++++- dlls/winemac.drv/macdrv_cocoa.h | 4 ++-- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/dlls/winemac.drv/cocoa_window.m b/dlls/winemac.drv/cocoa_window.m index 0d2862218ec..4ef9c305b61 100644 --- a/dlls/winemac.drv/cocoa_window.m +++ b/dlls/winemac.drv/cocoa_window.m @@ -3977,9 +3977,13 @@ uint32_t macdrv_window_background_color(void) } /*********************************************************************** - * macdrv_send_text_input_event + * macdrv_send_keydown_to_input_source + * + * Sends a key down event to the active window's inputContext so that it can be + * processed by input sources (AKA IMEs). This is only called when there is an + * active non-keyboard input source. */ -void macdrv_send_text_input_event(int pressed, unsigned int flags, int repeat, int keyc, void* himc, int* done) +void macdrv_send_keydown_to_input_source(unsigned int flags, int repeat, int keyc, void* himc, int* done) { OnMainThreadAsync(^{ BOOL ret; @@ -4004,7 +4008,7 @@ void macdrv_send_text_input_event(int pressed, unsigned int flags, int repeat, i // An NSEvent created with +keyEventWithType:... is internally marked // as synthetic and doesn't get sent through input methods. But one // created from a CGEvent doesn't have that problem. - c = CGEventCreateKeyboardEvent(NULL, keyc, pressed); + c = CGEventCreateKeyboardEvent(NULL, keyc, true); CGEventSetFlags(c, localFlags); CGEventSetIntegerValueField(c, kCGKeyboardEventAutorepeat, repeat); event = [NSEvent eventWithCGEvent:c]; diff --git a/dlls/winemac.drv/keyboard.c b/dlls/winemac.drv/keyboard.c index fe4f7d70a10..f06f98f2cbe 100644 --- a/dlls/winemac.drv/keyboard.c +++ b/dlls/winemac.drv/keyboard.c @@ -1239,6 +1239,14 @@ UINT macdrv_ImeProcessKey(HIMC himc, UINT wparam, UINT lparam, const BYTE *key_s if (!macdrv_using_input_method()) return 0; + if (!pressed) + { + /* Only key down events should be sent to the Cocoa input context. We do + not handle key ups, and instead let those go through as a normal + WM_KEYUP. */ + return 0; + } + switch (vkey) { case VK_SHIFT: @@ -1274,7 +1282,7 @@ UINT macdrv_ImeProcessKey(HIMC himc, UINT wparam, UINT lparam, const BYTE *key_s TRACE("flags 0x%08x keyc 0x%04x\n", flags, keyc); - macdrv_send_text_input_event(pressed, flags, repeat, keyc, himc, &done); + macdrv_send_keydown_to_input_source(flags, repeat, keyc, himc, &done); while (!done) NtUserMsgWaitForMultipleObjectsEx(0, NULL, INFINITE, QS_POSTMESSAGE | QS_SENDMESSAGE, 0); return done > 0; diff --git a/dlls/winemac.drv/macdrv_cocoa.h b/dlls/winemac.drv/macdrv_cocoa.h index 20d82b9bef8..917c8103d41 100644 --- a/dlls/winemac.drv/macdrv_cocoa.h +++ b/dlls/winemac.drv/macdrv_cocoa.h @@ -568,8 +568,8 @@ extern void macdrv_order_cocoa_window(macdrv_window w, macdrv_window prev, extern int macdrv_get_view_backing_size(macdrv_view v, int backing_size[2]); extern void macdrv_set_view_backing_size(macdrv_view v, const int backing_size[2]); extern uint32_t macdrv_window_background_color(void); -extern void macdrv_send_text_input_event(int pressed, unsigned int flags, int repeat, int keyc, - void* data, int* done); +extern void macdrv_send_keydown_to_input_source(unsigned int flags, int repeat, int keyc, + void* data, int* done); extern int macdrv_is_any_wine_window_visible(void); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7701
On Thu Apr 3 17:46:46 2025 +0000, Byeongsik Jeon wrote:
Yes. This will match the native behavior. Got it. I updated the patch. Thanks again!
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7701#note_99892
On Thu Apr 3 17:51:32 2025 +0000, Tim Clem wrote:
Got it. I updated the patch. Thanks again! That seems correct to me, it's also how winex11 processes its key events (even though the IME processing happens somewhere else in the call chain).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7701#note_99972
This merge request was approved by Rémi Bernon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7701
This merge request was approved by Brendan Shanks. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7701
participants (4)
-
Brendan Shanks (＠bshanks) -
Rémi Bernon
-
Tim Clem
-
Tim Clem (＠tclem)