Skip to content

Handle the command selector an IME hands back after committing - #752

Open
blancdu wants to merge 1 commit into
gnachman:masterfrom
blancdu:fix-ime-handed-back-selector-13030
Open

blancdu wants to merge 1 commit into
gnachman:masterfrom
blancdu:fix-ime-handed-back-selector-13030

Conversation

@blancdu

@blancdu blancdu commented Sep 10, 2026

Copy link
Copy Markdown

Fixes gnachman/iterm2#13030 on the GitLab tracker. Same symptom as the older #10817 there.

The bug

An input method can finish a composition by committing the text with
-insertText:replacementRange: and then handing the original keystroke back to the client
as a command selector. iTerm2 performed the first half and dropped the second.

With the built-in Korean 2-Set IME: type 가가, press Backspace twice. On the second press
the IME commits the remaining and then delivers deleteBackward:. The delete was
dropped, so the jamo stayed and a third Backspace was needed. Return during composition had
the identical shape with insertNewline:.

Debug log for the second Backspace, trimmed:

iTermKeyboardHandler.m:90   _hadMarkedTextBeforeHandlingKeypressEvent=1
iTermKeyboardHandler.m:392  Calling interpretKeyEvents: ... keyCode=51
PTYTextView.m               setMarkedTextㄱ
iTermKeyboardHandler.m:188  insertText:ㄱ replacementRange:{9223372036854775807, 0}
PTYTextView.m               insertText: clear marked text
PTYSession.m:5014           writeTask:ㄱ
iTermKeyboardHandler.m:165  doCommandBySelector:deleteBackward:
iTermKeyboardHandler.m:183  returning from doCommandBySelector:deleteBackward:
iTermKeyboardHandler.m:500  PTYTextView keyDown END

No 0x7f is written for that keypress. The next one belongs to the third Backspace.

Why it could not be handled

-doCommandBySelector: could not act on it for two reasons: its body is behind
experimentalKeyHandling / enableCharacterAccentMenu, both defaulting to NO, and its
inner condition excludes _hadMarkedTextBeforeHandlingKeypressEvent.

That flag is true in two different situations:

  • the IME consumed the keystroke to edit its composition, and there is nothing to do
  • the IME committed and handed the keystroke back, and it is now ours

Only the first should suppress. _keyPressHandled already separates them: it is reset in
-handleKeyDownEvent:... and set only when -insertText:replacementRange: committed
non-empty text during this keypress. Note the ordering in the log - the marked text is
cleared before the selector arrives, so -hasMarkedText is already NO by then.

The post-Cocoa fallback in -handleEventWithCocoa: cannot recover the event either: it is
gated on the same flag and on _keyPressHandled.

About insertNewline:

The comment at the existing condition says Return during composition is deliberately not
passed to the delegate, so I want to flag that this PR changes it knowingly rather than by
accident.

The argument for including it: the IME uses one mechanism for both keys. It commits and
hands the key back, identically, whether that key is Backspace or Return. Honouring one and
dropping the other splits a single rule by selector name. WezTerm honours both; Terminal.app
honours the delete and drops the Return, so both behaviours have precedent.

If you would rather keep Return as it is, dropping the insertNewline: line from the
condition is the whole change - the Backspace fix stands on its own.

Testing

Built from master and verified by hand with the built-in Korean 2-Set IME on macOS 26.6.2.

Before the patch, on master, both symptoms reproduce.

After:

  • 가가 + Backspace x2 leaves no jamo
  • holding Backspace deletes cleanly, no double deletion
  • + Return commits and submits in one press
  • Return with no composition active is unaffected
  • builds with no new warnings

Not tested: IME reconversion that uses a real replacementRange (Japanese/Chinese). Worth
noting because -insertText:replacementRange: calls -doCommandBySelector: itself for that
path. Reading it, the new branch should not fire there - the marked text has not been
cleared yet at that point, so ![self hasMarkedText] is false; and for reconversion of
already-committed text _hadMarkedTextBeforeHandlingKeypressEvent is false. But I have not
exercised it, so it deserves a look.

When an input method finishes a composition, it can commit the text via
-insertText:replacementRange: and then hand the original keystroke back to
the client as a command selector. iTerm2 performed the first half and
dropped the second.

With the built-in Korean 2-Set IME this is visible as a stray jamo: type
가가, press Backspace twice, and the second press commits the remaining ㄱ
via -insertText: and then delivers deleteBackward:. The delete was dropped,
so the jamo stayed and a third Backspace was needed to remove it. Return
during composition had the same shape with insertNewline:.

-doCommandBySelector: could not act on it because its body is gated behind
two advanced settings that default to NO, and because its inner condition
excludes _hadMarkedTextBeforeHandlingKeypressEvent. That flag is true both
when the IME consumed the key to edit its composition (nothing to do) and
when the IME committed and handed the key back (ours to handle). The class
already distinguishes the two: _keyPressHandled is set only when
-insertText:replacementRange: committed text during this keypress.

The post-Cocoa fallback in -handleEventWithCocoa: cannot recover the event
either, since it is gated on the same flag and on _keyPressHandled.

Verified against a debug log of both repros. Issue 13030, and the same
symptom in issue 10817.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant