fix(ime): preserve Korean IME composition through split-commit and Backspace#11624
fix(ime): preserve Korean IME composition through split-commit and Backspace#11624ohah wants to merge 2 commits into
Conversation
…ckspace Follow-up to warpdotdev#9730. Two Korean IME flows on macOS remained broken after that fix: 1. Split-commit (e.g. typing 'ㅏ' after '간' to produce '가나'): the new marked '나' is placed as a buffer selection by setMarkedText:, then the queued commit '가' overwrites that selection — losing '나' until the next composition update redraws the buffer. 2. Backspace on a marked syllable (e.g. marked 'ㄷ' on '가나ㄷ'): the macOS Korean IME asks for it via setMarkedText('ㄷ') + insertText('ㄷ') + setMarkedText('') + doCommandBySelector(deleteBackward:) warpdotdev#9730's doCommandBySelector: is a no-op, so the queued commit lands as plain text after the marked text is cleared, leaving a stray 'ㄷ' the user has to delete with an extra Backspace press. Changes in host_view.m: - For split-commit, dispatch ClearMarkedText → commit → SetMarkedText in keyDownImpl: so the new marked text is placed cleanly after the commit rather than being overwritten by it. - Track deleteBackward:/deleteForward: that the IME signals via doCommandBySelector: during interpretKeyEvents:, and suppress the queued commit on that branch — the IME's intent there is just "remove the marked text", not "commit then delete". - Always end IME state with unmarkText after a commit-only flow, matching the pre-warpdotdev#9730 invariant; the new hasNewMarked branch protects newly-started marked text in split-commit cases. Plus regression tests for both flows in marked_text_tests.rs. Fixes warpdotdev#8919.
|
This PR is not linked to an issue that is marked with Issue-state enforcement details:
Readiness check:
To continue, link this PR to a same-repo issue such as Powered by Oz |
There was a problem hiding this comment.
This PR is not linked to an issue that is marked with ready-to-implement.
Issue-state enforcement details:
-
Associated same-repo issues checked: #8919
-
Required readiness label:
ready-to-implement
Readiness check:
- #8919: missing
ready-to-implement; readiness labels present: none
To continue, link this PR to a same-repo issue such as Closes #123 in the PR description, and make sure that issue has ready-to-implement.
Powered by Oz
…ion tests Make the link between the regression tests and the linked issue explicit in the test docstrings so future readers can jump back to the user report that motivated each test.
Description
Follow-up to #9730 (merged). That PR addressed the Japanese IME Enter-form-submit case, but two multi-byte IME flows on macOS still misbehave:
1. The in-progress jamo/character is invisible until the next keystroke (split-commit)
This is the bug tracked in #3944 — "Text input in multi-byte Japanese doesn't appear until confirmation". The same root cause affects Korean IME: typing "가나다" slowly, the
나does not appear after가; it only shows up once다is typed and the buffer re-renders.Cause: When typing
ㅏafter간, the IME sendssetMarkedText('가') → insertText('가') → setMarkedText('') → setMarkedText('나')within a singleinterpretKeyEvents:pass. The trailingsetMarkedText('나')places나as a buffer-level selection in the input field. The queued commit가then dispatches throughuser_insert, whose selection-replacement semantics overwrite나with가— losing the new composing jamo until the next composition update.#9730's
imeTouchedMarkedTextDuringInterpretguard only makesunmarkTextconditional. It preserves themarkedTextobject insidehost_view.m, but the Rust input field's buffer selection still gets clobbered by the commit.2. Backspace requires two presses to delete a marked syllable
With a marked
ㄷon가나ㄷ, the user observes가나다 → 가나ㄷ → 가나ㄷ (no change) → 가나— the second Backspace is wasted.Cause: macOS Korean IME implements Backspace on a marked syllable as:
doCommandBySelector:is currently a no-op (// no-op as we will be handling control characters in KeyDown events), so the IME'sdeleteBackward:signal is ignored. Only the queued commitㄷis applied, leaving a stray plain-textㄷthat the next Backspace then deletes.Changes (
crates/warpui/src/platform/mac/objc/host_view.m)keyDownImpl:'s commit branch, when a new marked text was started during the sameinterpretKeyEvents:, dispatchClearMarkedText → commit → SetMarkedTextinstead of just the commit. This avoids the buffer-selection overwrite. NewlastMarkedSelectedRangeinstance variable holds the most recentselectedRangefor the re-dispatch.doCommandBySelector:now tracksdeleteBackward:/deleteForward:signaled by the IME duringinterpretKeyEvents:via a newimeRequestedDeleteDuringInterpretflag. When set,keyDownImpl:suppresses the queued commit so the marked text simply disappears, matching native macOS apps' behavior.!imeTouchedMarkedTextDuringInterpretguard around the trailingunmarkText. The new split-commit branch above is what now protects newly-started marked text in split-commit cases, so the pre-fix(macos/ime): don't submit form when Enter confirms Japanese IME conversion #9730 invariant (commit ends IME state) can be restored — without it the stale marked-text state leaks into the next keystroke and breaks Backspace.Linked Issue
Fixes #3944. Also addresses the Korean-specific symptoms reported in #8919 (follow-up to #9730).
ready-to-implement.Screenshots / Videos
Testing
Manual (macOS, Apple Korean IME,
cargo run --features ime_marked_text)나missing until다is typed가나다 → 가나ㄷ → 가나ㄷ → 가나(one press wasted)가나다 → 가나ㄷ → 가나 → 가⌥E + g´gAutomated (
app/src/editor/view/marked_text_tests.rs)Two regression tests added:
test_split_commit_with_new_marked_text— verifies split-commit leaves the buffer as<commit><new marked>test_korean_ime_backspace_clears_marked_syllable— verifies Backspace cleanly removes the marked syllable with no stray plain-text leftoverNote on prior PR (#9713)
A previous attempt (#9713) added a
(!handled || wasComposing)guard to dispatch committed text on handled key events. @abhishekp106 closed it in favor of #9730 citing concerns about double-handled events. This PR drops thewasComposingguard entirely — the Enter case is already covered by #9730 + thedeleteBackward:handling here, so the guard isn't needed.Agent Mode