fix(macos/ime): don't submit form when Enter confirms Japanese IME conversion#9730
Conversation
When an IME (e.g. macOS Japanese, Google Japanese Input) does a split-commit during a single interpretKeyEvents: pass — calling insertText: with the just-committed phrase and then setMarkedText: to queue the next in-progress character — the trailing [self unmarkText] in keyDownImpl was unconditionally clobbering that newly-set marked text in our state. The IME's internal state still held the queued character, but WarpHostView's markedText was empty and warp_marked_text_cleared had been dispatched. On the next keystroke (typically Enter to commit), wasComposing was therefore false, so warp_handle_view_event ran without the composing flag and the keybinding system handled Enter as a command submission. The IME's insertText: of the queued character then arrived at line 184 with handled=YES and was silently dropped. Track whether the IME modified marked text via setMarkedText: or unmarkText during the current interpretKeyEvents: pass. Skip the trailing unmarkText if the IME has already touched marked text in that pass — the IME has either cleared it (our call is a redundant ClearMarkedText) or replaced it with new in-progress text we must preserve. CHANGELOG-BUG-FIX: Fix Japanese IME losing the last character of a phrase that ends right before a punctuation mark on macOS.
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @s-zaizen on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I reviewed this pull request and requested human review from: @vorporeal. Comment Powered by Oz |
|
@cla-bot check |
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @s-zaizen on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
|
The cla-bot has been summoned, and re-checked this pull request! |
There was a problem hiding this comment.
Overview
This PR updates the macOS NSTextInputClient path in host_view.m to preserve IME-marked text when an IME commits text and sets new marked text during the same interpretKeyEvents: pass.
Concerns
- No blocking correctness, security, or error-handling issues found in the changed lines.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
|
reassigning to @abhishekp106, who has done the most IME work |
abhishekp106
left a comment
There was a problem hiding this comment.
This looks good! This is neatly scoped and from local testing doesn't regress anything as far as I can tell. Thanks for taking the time to submit this and writing a detailed PR description. Congrats on your first contribution! I'm very excited to keep improving IMEs in Warp.
…nversion (warpdotdev#9730) ## Description On macOS, some Japanese IMEs (built-in macOS Japanese, Google Japanese Input, etc.) commit a phrase mid-typing via a single `interpretKeyEvents:` pass that fires `insertText:` (the just-committed phrase) and then `setMarkedText:` (the next in-progress character). The trailing `[self unmarkText]` in `keyDownImpl` was unconditionally clobbering that freshly-set marked text in our state. After that, the IME's internal state still held the queued character but `WarpHostView`'s `markedText` was empty and `warp_marked_text_cleared` had been dispatched. On the next keystroke (typically Enter to commit), `wasComposing` was therefore `false`, so `warp_handle_view_event` ran without the composing flag and the keybinding system handled Enter as a command submission. The IME's `insertText:` of the queued character arrived at the post-dispatch insert path with `handled=YES` and was silently dropped. The fix tracks whether the IME modified marked text via `setMarkedText:` or `unmarkText` during the current `interpretKeyEvents:` pass, and skips the trailing `unmarkText` if the IME has already touched marked text in that pass — the IME has either cleared it (so our call would have been a redundant `ClearMarkedText`) or replaced it with new in-progress text we must preserve. This change is contained to the macOS Objective-C IME plumbing in `host_view.m`. The Linux/Windows IME path (winit `Ime::Preedit` / `Ime::Commit` in `crates/warpui/src/windowing/winit/event_loop/mod.rs`) is structurally different and not affected by this PR. ## Linked Issue Fixes warpdotdev#7261 > Note: the issue body explicitly reports `Operating system (OS): macOS` (`15.6(24G84)`), so the `os:linux` label appears to have been applied in error during automated triage. This PR addresses the macOS path only; please drop the `os:linux` label or relabel as `os:mac` during review. - [x] The linked issue is labeled `ready-to-spec` or `ready-to-implement`. - [x] Where appropriate, screenshots or a short video of the implementation are included below (especially for user-visible or UI changes). ## Screenshots / Videos ### Before Fix https://github.com/user-attachments/assets/f3acf468-2860-4326-b8ab-f8ff4990d5a0 ### After Fix https://github.com/user-attachments/assets/a45d9c18-8619-4422-9406-e3cb66cc380e ## Testing Manual verification on macOS 15.x with the macOS built-in Japanese IME (Hiragana / Romaji mode): 1. Reproduced the bug on `master` with the steps from warpdotdev#7261 — typing `漢字の入力(変換)が、` and pressing Enter dropped the trailing `、` and submitted the buffer immediately. 2. Verified the fix on this branch with `cargo run --bin warp-oss` (with `FeatureFlag::ImeMarkedText` enabled locally so inline preedit could be observed). After the fix the queued `、` is preserved across the split-commit, the next Enter commits it via the IME, and a subsequent Enter submits the buffer as expected. 3. Sanity-checked dead-key composition (`⌥e` then `a` → `á`) on a US keyboard layout to confirm the dead-key suppression branch in `keyDownImpl` is unaffected. 4. Ran the presubmit checks locally: - `cargo fmt --check` — pass - `cargo clippy --workspace --exclude warp_completer --all-targets --tests -- -D warnings` — pass - `cargo clippy -p warp_completer --all-targets --tests -- -D warnings` — pass - `./script/run-clang-format.py -r --extensions 'c,h,cpp,m' ./crates/warpui/src/ ./app/src/` — pass - `cargo nextest run --no-fail-fast --workspace --exclude command-signatures-v2` — passes for the IME-relevant suites; the only failures observed locally (`settings::init::tests::test_migration_does_not_rerun_when_marker_present` and the SSH-IAP integration suite) reproduce identically on `master` and are unrelated to this change. - `cargo test --doc` — pass ## Agent Mode - [ ] Warp Agent Mode - This PR was created via Warp's AI Agent Mode <!-- CHANGELOG-BUG-FIX: Fix Japanese IME losing the last character of a phrase that ends right before a punctuation mark on macOS. --> (cherry picked from commit 3ff78d2)
…nversion (warpdotdev#9730) ## Description On macOS, some Japanese IMEs (built-in macOS Japanese, Google Japanese Input, etc.) commit a phrase mid-typing via a single `interpretKeyEvents:` pass that fires `insertText:` (the just-committed phrase) and then `setMarkedText:` (the next in-progress character). The trailing `[self unmarkText]` in `keyDownImpl` was unconditionally clobbering that freshly-set marked text in our state. After that, the IME's internal state still held the queued character but `WarpHostView`'s `markedText` was empty and `warp_marked_text_cleared` had been dispatched. On the next keystroke (typically Enter to commit), `wasComposing` was therefore `false`, so `warp_handle_view_event` ran without the composing flag and the keybinding system handled Enter as a command submission. The IME's `insertText:` of the queued character arrived at the post-dispatch insert path with `handled=YES` and was silently dropped. The fix tracks whether the IME modified marked text via `setMarkedText:` or `unmarkText` during the current `interpretKeyEvents:` pass, and skips the trailing `unmarkText` if the IME has already touched marked text in that pass — the IME has either cleared it (so our call would have been a redundant `ClearMarkedText`) or replaced it with new in-progress text we must preserve. This change is contained to the macOS Objective-C IME plumbing in `host_view.m`. The Linux/Windows IME path (winit `Ime::Preedit` / `Ime::Commit` in `crates/warpui/src/windowing/winit/event_loop/mod.rs`) is structurally different and not affected by this PR. ## Linked Issue Fixes warpdotdev#7261 > Note: the issue body explicitly reports `Operating system (OS): macOS` (`15.6(24G84)`), so the `os:linux` label appears to have been applied in error during automated triage. This PR addresses the macOS path only; please drop the `os:linux` label or relabel as `os:mac` during review. - [x] The linked issue is labeled `ready-to-spec` or `ready-to-implement`. - [x] Where appropriate, screenshots or a short video of the implementation are included below (especially for user-visible or UI changes). ## Screenshots / Videos ### Before Fix https://github.com/user-attachments/assets/f3acf468-2860-4326-b8ab-f8ff4990d5a0 ### After Fix https://github.com/user-attachments/assets/a45d9c18-8619-4422-9406-e3cb66cc380e ## Testing Manual verification on macOS 15.x with the macOS built-in Japanese IME (Hiragana / Romaji mode): 1. Reproduced the bug on `master` with the steps from warpdotdev#7261 — typing `漢字の入力(変換)が、` and pressing Enter dropped the trailing `、` and submitted the buffer immediately. 2. Verified the fix on this branch with `cargo run --bin warp-oss` (with `FeatureFlag::ImeMarkedText` enabled locally so inline preedit could be observed). After the fix the queued `、` is preserved across the split-commit, the next Enter commits it via the IME, and a subsequent Enter submits the buffer as expected. 3. Sanity-checked dead-key composition (`⌥e` then `a` → `á`) on a US keyboard layout to confirm the dead-key suppression branch in `keyDownImpl` is unaffected. 4. Ran the presubmit checks locally: - `cargo fmt --check` — pass - `cargo clippy --workspace --exclude warp_completer --all-targets --tests -- -D warnings` — pass - `cargo clippy -p warp_completer --all-targets --tests -- -D warnings` — pass - `./script/run-clang-format.py -r --extensions 'c,h,cpp,m' ./crates/warpui/src/ ./app/src/` — pass - `cargo nextest run --no-fail-fast --workspace --exclude command-signatures-v2` — passes for the IME-relevant suites; the only failures observed locally (`settings::init::tests::test_migration_does_not_rerun_when_marker_present` and the SSH-IAP integration suite) reproduce identically on `master` and are unrelated to this change. - `cargo test --doc` — pass ## Agent Mode - [ ] Warp Agent Mode - This PR was created via Warp's AI Agent Mode <!-- CHANGELOG-BUG-FIX: Fix Japanese IME losing the last character of a phrase that ends right before a punctuation mark on macOS. -->
…nversion (warpdotdev#9730) ## Description On macOS, some Japanese IMEs (built-in macOS Japanese, Google Japanese Input, etc.) commit a phrase mid-typing via a single `interpretKeyEvents:` pass that fires `insertText:` (the just-committed phrase) and then `setMarkedText:` (the next in-progress character). The trailing `[self unmarkText]` in `keyDownImpl` was unconditionally clobbering that freshly-set marked text in our state. After that, the IME's internal state still held the queued character but `WarpHostView`'s `markedText` was empty and `warp_marked_text_cleared` had been dispatched. On the next keystroke (typically Enter to commit), `wasComposing` was therefore `false`, so `warp_handle_view_event` ran without the composing flag and the keybinding system handled Enter as a command submission. The IME's `insertText:` of the queued character arrived at the post-dispatch insert path with `handled=YES` and was silently dropped. The fix tracks whether the IME modified marked text via `setMarkedText:` or `unmarkText` during the current `interpretKeyEvents:` pass, and skips the trailing `unmarkText` if the IME has already touched marked text in that pass — the IME has either cleared it (so our call would have been a redundant `ClearMarkedText`) or replaced it with new in-progress text we must preserve. This change is contained to the macOS Objective-C IME plumbing in `host_view.m`. The Linux/Windows IME path (winit `Ime::Preedit` / `Ime::Commit` in `crates/warpui/src/windowing/winit/event_loop/mod.rs`) is structurally different and not affected by this PR. ## Linked Issue Fixes warpdotdev#7261 > Note: the issue body explicitly reports `Operating system (OS): macOS` (`15.6(24G84)`), so the `os:linux` label appears to have been applied in error during automated triage. This PR addresses the macOS path only; please drop the `os:linux` label or relabel as `os:mac` during review. - [x] The linked issue is labeled `ready-to-spec` or `ready-to-implement`. - [x] Where appropriate, screenshots or a short video of the implementation are included below (especially for user-visible or UI changes). ## Screenshots / Videos ### Before Fix https://github.com/user-attachments/assets/f3acf468-2860-4326-b8ab-f8ff4990d5a0 ### After Fix https://github.com/user-attachments/assets/a45d9c18-8619-4422-9406-e3cb66cc380e ## Testing Manual verification on macOS 15.x with the macOS built-in Japanese IME (Hiragana / Romaji mode): 1. Reproduced the bug on `master` with the steps from warpdotdev#7261 — typing `漢字の入力(変換)が、` and pressing Enter dropped the trailing `、` and submitted the buffer immediately. 2. Verified the fix on this branch with `cargo run --bin warp-oss` (with `FeatureFlag::ImeMarkedText` enabled locally so inline preedit could be observed). After the fix the queued `、` is preserved across the split-commit, the next Enter commits it via the IME, and a subsequent Enter submits the buffer as expected. 3. Sanity-checked dead-key composition (`⌥e` then `a` → `á`) on a US keyboard layout to confirm the dead-key suppression branch in `keyDownImpl` is unaffected. 4. Ran the presubmit checks locally: - `cargo fmt --check` — pass - `cargo clippy --workspace --exclude warp_completer --all-targets --tests -- -D warnings` — pass - `cargo clippy -p warp_completer --all-targets --tests -- -D warnings` — pass - `./script/run-clang-format.py -r --extensions 'c,h,cpp,m' ./crates/warpui/src/ ./app/src/` — pass - `cargo nextest run --no-fail-fast --workspace --exclude command-signatures-v2` — passes for the IME-relevant suites; the only failures observed locally (`settings::init::tests::test_migration_does_not_rerun_when_marker_present` and the SSH-IAP integration suite) reproduce identically on `master` and are unrelated to this change. - `cargo test --doc` — pass ## Agent Mode - [ ] Warp Agent Mode - This PR was created via Warp's AI Agent Mode <!-- CHANGELOG-BUG-FIX: Fix Japanese IME losing the last character of a phrase that ends right before a punctuation mark on macOS. -->
…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.
Description
On macOS, some Japanese IMEs (built-in macOS Japanese, Google Japanese Input, etc.) commit a phrase mid-typing via a single
interpretKeyEvents:pass that firesinsertText:(the just-committed phrase) and thensetMarkedText:(the next in-progress character). The trailing[self unmarkText]inkeyDownImplwas unconditionally clobbering that freshly-set marked text in our state.After that, the IME's internal state still held the queued character but
WarpHostView'smarkedTextwas empty andwarp_marked_text_clearedhad been dispatched. On the next keystroke (typically Enterto commit),
wasComposingwas thereforefalse, sowarp_handle_view_eventran without the composing flag and the keybinding system handled Enter as a command submission. The IME'sinsertText:of the queued character arrived at the post-dispatch insert path withhandled=YESand was silently dropped.The fix tracks whether the IME modified marked text via
setMarkedText:orunmarkTextduring the currentinterpretKeyEvents:pass, and skips the trailingunmarkTextif the IME has already touchedmarked text in that pass — the IME has either cleared it (so our call would have been a redundant
ClearMarkedText) or replaced it with new in-progress text we must preserve.This change is contained to the macOS Objective-C IME plumbing in
host_view.m. The Linux/Windows IME path (winitIme::Preedit/Ime::Commitincrates/warpui/src/windowing/winit/event_loop/mod.rs) is structurally different and not affected by this PR.Linked Issue
Fixes #7261
ready-to-specorready-to-implement.Screenshots / Videos
Before Fix
2026-05-01.19.18.15.mov
After Fix
2026-05-01.19.18.41.mov
Testing
Manual verification on macOS 15.x with the macOS built-in Japanese IME (Hiragana / Romaji mode):
masterwith the steps from [Bug] Pressing Enter after Japanese IME conversion unexpectedly submits the form in Warp.dev #7261 — typing漢字の入力(変換)が、and pressing Enter dropped the trailing、and submitted the buffer immediately.cargo run --bin warp-oss(withFeatureFlag::ImeMarkedTextenabled locally so inline preedit could be observed). After the fix the queued、is preserved acrossthe split-commit, the next Enter commits it via the IME, and a subsequent Enter submits the buffer as expected.
⌥ethena→á) on a US keyboard layout to confirm the dead-key suppression branch inkeyDownImplis unaffected.cargo fmt --check— passcargo clippy --workspace --exclude warp_completer --all-targets --tests -- -D warnings— passcargo clippy -p warp_completer --all-targets --tests -- -D warnings— pass./script/run-clang-format.py -r --extensions 'c,h,cpp,m' ./crates/warpui/src/ ./app/src/— passcargo nextest run --no-fail-fast --workspace --exclude command-signatures-v2— passes for the IME-relevant suites; the only failures observed locally(
settings::init::tests::test_migration_does_not_rerun_when_marker_presentand the SSH-IAP integration suite) reproduce identically onmasterand are unrelated to this change.cargo test --doc— passAgent Mode