[PATCH 0/1] MR7701: winemac.drv: Only send key down events to the window's inputContext.

[Tim Clem (＠tclem)]

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.

This change fixes doubled input with Romaji and the problematic third-party method we've encountered.

[PATCH 1/1] winemac.drv: Only send key down events to the window's inputContext.

[Tim Clem]

From: Tim Clem

Fixes doubled input with certain input sources.
---
 dlls/winemac.drv/cocoa_window.m | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/dlls/winemac.drv/cocoa_window.m b/dlls/winemac.drv/cocoa_window.m
index 0d2862218ec..1cc87c27510 100644
--- a/dlls/winemac.drv/cocoa_window.m
+++ b/dlls/winemac.drv/cocoa_window.m
@@ -3992,7 +3992,10 @@ void macdrv_send_text_input_event(int pressed, unsigned int flags, int repeat, i
                 window = [[WineApplicationController sharedController] frontWineWindow];
         }

-        if (window)
+        /* Only key down events should go to the window's inputContext. Key up
+           events bound for a window should be treated as handled without going
+           to the context. */
+        if (window && pressed)
         {
             NSUInteger localFlags = flags;
             CGEventRef c;
@@ -4013,6 +4016,8 @@ void macdrv_send_text_input_event(int pressed, unsigned int flags, int repeat, i
             window.commandDone = FALSE;
             ret = [[[window contentView] inputContext] handleEvent:event] && !window.commandDone;
         }
+        else if (window && !pressed)
+            ret = TRUE;
         else
             ret = FALSE;

[Byeongsik Jeon (＠bsjeon)]

Byeongsik Jeon (@bsjeon) commented about dlls/winemac.drv/cocoa_window.m:
>          window.commandDone = FALSE;
>          ret = [[[window contentView] inputContext] handleEvent:event] && !window.commandDone;
>      }
> -
>    else if (window && !pressed)
> -
>        ret = TRUE;

Here, is there a reason why ret is TRUE?

ImeProcessKey should return FALSE if ime does not handle key events. To avoid unnecessary code execution and waiting, my opinion is that the fix should be made earlier in macdrv_ImeProcessKey().

[Byeongsik Jeon (＠bsjeon)]

This issue is very annoying when “Korean 2-Set Keyboard” is selected as the input source. For example, when entering a number or space character, the KeyUp event behaves like a KeyDown event, so the input is doubled, once on KeyDown and once on KeyUp.

The same problem occurs when I select a third-party open-source MAC Korean IME, so I investigated its source, but I couldn't find any suspicious code.

My conclusion is that if the ime is not in composition (in other words, hasMarkedText is FALSE), NSTextInputContext::handleEvent treats KeyUp events like KeyDown.
When I tested a specialized Japanese key sequence that satisfied this condition, I was able to reproduce the same issue with Japanese IMEs.

As you describe, all the NSTextInputContext documentation I researched seemed to only be interested in KeyDown events. Unfortunately, I couldn't find any documentation that explicitly talked about handling KeyUp events. I wasn't sure if this was a macOS bug or intended, so my work stalled.

As a compromise, I tried to think about it from a different perspective.

1. For a long time, winemac ime did not handle KeyUp events.
 2. This issue occurs after commit faa342a2f168.
 3. However, commit faa342a2f168 itself is correct.
 4. as far as I know, there hasn't been any issue that NSTextInputContext:handleEvent should handle KeyUp events.

Therefore, my opinion is that a fix like this MR with commit faa342a2f168 is not unreasonable.

In addition to this issue, Korean input is currently broken in Wine macOS build. If this issue is fixed, I would like to try upstream.

[Tim Clem (＠tclem)]

On Thu Apr  3 02:20:03 2025 +0000, Byeongsik Jeon wrote:
> Here, is there a reason why ret is TRUE?
> ImeProcessKey should return FALSE if ime does not handle key events. To
> avoid unnecessary code execution and waiting, my opinion is that the fix
> should be made earlier in macdrv_ImeProcessKey().

In my opinion, from the perspective of ImeProcessKey, key up events *are* handled if there's an active macOS input source. They shouldn't go to the inputContext, but they are paired with a key down event that did, so they are effectively consumed. I'm not sure how ImeProcessKey usually gets used, but it strikes me that having it return true for unpaired key up events like that would be unexpected.

You're right that handling it a level up in macdrv_ImeProcessKey seems better, to avoid the dispatch to the main thread and the subsequent wait. I'll update the patch.

[Tim Clem (＠tclem)]

> In addition to this issue, Korean input is currently broken in Wine macOS build. If this issue is fixed, I would like to try upstream.

That's a separate problem than this? Could you file a bug with the details?