Repository navigation
fix(app): keep the composer editor inert during IME composition - #55295
Mirac1eSky wants to merge 3 commits into
Conversation
Typing CJK with an IME in the V2 composer breaks on the first keystroke: the first letter is committed as literal text and only the rest reaches the candidate window (e.g. 'ceshi' -> 测试 turns into 'c' + 'eshi'). anomalyco#39632, anomalyco#49448, anomalyco#38674. Root cause: the composer editor reacts to every composition keystroke (onInput -> store write, keydown -> dispatch, keyup -> selection sync) and can rewrite the contentEditable DOM mid-composition. A DOM or selection mutation while a composition is active makes the browser abort it, committing whatever partial text exists as plain input. The editor is now fully inert while a composition is in flight and during a 100ms window after compositionend: no input handling, no keydown dispatch, no cursor sync, no editor DOM rewrites. State is reconciled once from the DOM on compositionend. The confirming Enter of an IME candidate can no longer submit the message either. Ports the V1 fix from anomalyco#44826 to the V2 composer, as requested when that PR was closed. Co-authored-by: powoct <powoct@gmail.com> Refs anomalyco#39632 Refs anomalyco#49448 Refs anomalyco#38674
There was a problem hiding this comment.
I couldn't verify this. The bug needs a real IME in a desktop browser, and I can only drive the TUI here. Typecheck passes, and the composer tests pass with the package's test:unit flags. Public API: unchanged (only the in-app composer controller gets new members).
Concerns:
- Characters typed right after a composition don't reach the prompt. For 100 ms after
compositionend,onInputignores every input, including plain input that isn't part of a composition. Korean IMEs end the current syllable and insert the next character (a space,?,@, …) in the same keystroke. Theirinputevent arrives inside that window and is skipped. The stored prompt then lacks that character until the next input event. If Enter comes right after, the message is sent without it, and a@typed this way doesn't open the context popover. The sync done oncompositionendreads the DOM before that character is inserted. - Other keys are swallowed too. The early return in
onKeyDowndrops Escape, Ctrl+G, shell-mode Backspace and history arrows for 100 ms after any composition. Because it also checkskeyCode === 229, it drops them always on Android keyboards, which report 229 for most keys. A tighter guard would help: only treat Enter as an IME confirmation, and keeponInputworking for input that isn't part of a composition. - The new tests don't cover the fix.
suggestions/machine.tsis unchanged, and the machine already ignored Enter whencomposingwas true, so both tests pass on the base.packages/app/AGENTS.mdasks that a regression test fail before the fix. Please drop them, or replace them with a test that drives the editor's composition events.
Could you also share a short video or before/after recordings on Safari and Chrome? Ideally include a Korean IME, given the first point.
| // here re-renders mid-composition and splits it into per-letter | ||
| // micro compositions. State is reconciled once by the | ||
| // compositionend write-back. | ||
| if (event.isComposing || props.controller.imeActive()) return |
There was a problem hiding this comment.
imeActive() stays true for 100 ms after compositionend, so this also drops input that isn't part of a composition. Korean IMEs end the composition and insert the next character (a space, ?, @) in the same keystroke, and that input event lands here with isComposing === false. The compositionend sync has already run, before that character was inserted, so the store misses it until the next input. A quick Enter then sends the prompt without it. Gating only on event.isComposing / the active composing flag would avoid this.
This returns early for every key in the post-composition window, and for any key reporting keyCode === 229. Android soft keyboards report 229 for most keys. So Escape (stop / cancel queue edit), Ctrl+G, shell-mode Backspace and history arrows are ignored there, and for 100 ms after every composition on desktop. Since the goal is to keep the confirming Enter from submitting, could the window apply only to Enter (and maybe Tab)?
| expect(result.handled).toBe(row.handled ?? false) | ||
| }) | ||
|
|
||
| test("ignores Enter while IME composition is active", () => { |
There was a problem hiding this comment.
machine.ts isn't changed in this PR, and it already ignored Enter when composing is true, so both tests pass on the base and don't cover the fix (see the regression-test rule in packages/app/AGENTS.md). Please drop them, or test the editor's composition handling instead.
|
Data point from a WKWebView host, on the released 2.0.32 — still reproducible, and your event-order diagnosis is the one that matches. I maintain a macOS Tauri app that embeds the web UI in native child webviews. A user typing a pinyin prompt and pressing Enter to confirm the candidate sent the message, and the message body carried the raw pinyin ( I checked the shipped bundle rather than the sources: the composer in Because the page is not ours, we had to patch the engine view from the host for now: a document-start script redefines One question about the 100 ms window: the typematic minimum is ~30 ms, so a user who commits a candidate and immediately presses Enter to send can land inside the window and have the send ignored. We used 10 ms, which only has to outlive WebKit's runloop turn. If 100 ms comes from a measured input method, which one? If it is headroom rather than a measurement, a shorter window (or scoping it to On testing: this does not need a real IME. Dispatching |
- Confirm window 100ms -> 10ms, scoped to Enter/Tab; other keys (Escape, arrows) and Android keyCode-229 events are unaffected - onInput gated only while a composition is in flight, so the same-keystroke character Korean IMEs insert after compositionend reaches the store - Drop the two machine tests that passed on base; add editor-level regression tests driving the real controller with synthetic composition events (zh/ja/ko + typematic-speed follow-up)
There was a problem hiding this comment.
Thanks for the update. Both earlier points are fixed. onInput now skips input only while a composition is actually in progress. The guard after compositionend is 10 ms and covers only Enter and Tab, and keyCode 229 is only checked for those two keys. The machine tests that passed on the base are gone.
I still couldn't verify this on a real IME, so a short before/after recording on Safari and Chrome would help. The new browser test file and the packages/app typecheck pass. Public API: unchanged.
One remaining point is the new test file (see inline). It is large for what it checks, and its result depends on real time.
| return Object.assign(new Event("input"), { isComposing: init.isComposing ?? false }) as InputEvent | ||
| } | ||
|
|
||
| const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)) |
There was a problem hiding this comment.
This file only calls the controller helpers (imeComposing / imeInserting) with hand-built events, so it doesn't exercise the editor handlers that use them. It also depends on real time. The sleep(5) / sleep(35) calls race the 10 ms window. The keyCode 13 assertion on line 64 passes only if onCompositionEnd and the next call run within 10 ms, which can fail on a loaded CI runner. packages/app/AGENTS.md asks for one test per contract and no timing-sensitive tests. Could you cut it to two or three cases that don't depend on wall-clock time? For example: Enter confirming a candidate is ignored, input after compositionend is accepted, and Escape is not swallowed. Driving them through the ComposerEditor element would be better still.
|
Thanks @yusiwen — the WKWebView trace and the On the 100 ms: it was inherited as-is from the V1 fix (#44826), which I had only verified with Chinese IMEs — headroom, not a measurement. I've now re-tested the guard by driving the actual composer controller with synthetic composition/keyboard events (zh pinyin, ja, ko sequences, plus typematic-speed follow-ups), and 10 ms covers the compositionend→keydown runloop turn while leaving a fast second Enter intact. Your measured 10 ms is now the window. Pushed a revision addressing all three inline concerns:
Verified locally this time: |
…tracts Pin performance.now instead of sleeping: the guard reads the monotonic clock on purpose, so wall-clock sleeps raced the 10ms confirm window on loaded CI runners. Three tests now cover the same contracts (confirm Enter guarded / fast send allowed, input gated only mid-composition, non-confirm keys and Android 229 unaffected). Each fails against the previous revision.
|
Done — cut to three cases, no wall-clock. The guard reads On the recording: the machine-observable behavior is now covered by the deterministic tests above, and the released-build behavior was already traced end-to-end on real IMEs in @yusiwen's WKWebView measurements. Happy to add a recording if a maintainer feels it's still needed. |
There was a problem hiding this comment.
Thanks, the tests are fixed. They are down to three cases and pin performance.now() instead of sleeping, so they no longer depend on real time. The new IME test file and the packages/app typecheck pass. Public API: unchanged.
I still couldn't try this with a real IME, so a short recording on Safari and Chrome would still help.
|
Data point: same root cause on Android (non-CJK IME) — word suggestions/autocorrect never appear Environment
SymptomTyping works, but the keyboard never shows its word suggestions and never autocorrects in the composer. The same keyboard does show suggestions in a plain Checked, not the cause
Hypothesis = what this PR fixesEach composition keystroke triggers Ask
Related: #57740 (Gboard voice typing blocked in the same composer), #39632, #49448, #38674 |

Issue for this PR
Refs #39632, #49448, #38674. Ports the V1 fix from #44826 to the V2 composer, as requested when that PR was closed ("no longer taking PRs for V1 — port it over and open it for V2").
Type of change
What does this PR do?
Typing CJK with an IME in the composer breaks on the first keystroke: the first letter is committed as literal text and only the rest reaches the candidate window (e.g. pinyin
ceshi→ 测试 turns intoc+eshi; #39632'sceuu→c+euu).Root cause: the composer editor is not inert during IME composition. It reacts to every composition keystroke (
onInput→ store write,keydown→ dispatch,keyup→ selection sync) and can rewrite the contentEditable DOM mid-composition viarenderComposerEditor(replaceChildren) once the one-shotlocalInputflag is consumed. A DOM or selection mutation while a composition is active makes the browser abort it, committing whatever partial text exists as plain input — which splits the composition into per-letter micro compositions.The editor is now fully inert while a composition is in flight and during a 100 ms window after
compositionend:onInputignores composition keystrokes (event.isComposing/imeActive()); state is reconciled once from the DOM oncompositionend.onKeyDownreturns early while composing, so no state dispatch happens mid-composition and the Enter that confirms an IME candidate can no longer submit the message (Safari firescompositionendbefore the confirming keydown, soisComposingalone is not enough — the 100 ms window covers it).onKeyUp/onPointerUpskip cursor sync (no selection mutation) while composing.renderComposerEditorDOM rewrites while composing; combined with thecompositionendwrite-back this keeps DOM and store consistent.key.downdispatch reportsimeComposing(event)instead of rawevent.isComposing, so the suggestion machine also ignores Enter/Tab during composition.How did you verify your code works?
nihao+ Space commits 你好 as one segment, no per-letter splitting, plain-English Enter / Shift+Enter unchanged, and the confirming Enter no longer submits.packages/app/src/composer/suggestions/machine.test.ts(Enter during composition is inert; Enter after composition ends selects the active suggestion).bun test/ typecheck on the machine where I ported this (no bun toolchain available) — relying on CI here. The port is mechanical: v1.19'spackages/session-ui/src/v2/components/prompt-inputmaps 1:1 ontopackages/app/src/composer/editor.Checklist
Added by the opencode bot (AI-generated).
Related: #39632, #38674, #49448, #41647