fix(desktop): recover from a stale IME composition flag that silently blocks all sends - #44149
fix(desktop): recover from a stale IME composition flag that silently blocks all sends#44149AIalliAI wants to merge 1 commit into
Conversation
|
✅ Verification — stale IME composition flag recovery is well-designed Checked:
The approach is defensive without being over-eager — it only clears the flag when the native event explicitly says composition is not active. LGTM. |
…osition Two interacting problems prevented CJK IME users from sending messages via Enter in the desktop composer: 1. handleEditorInput skipped draft updates when composingRef was true, so the React draft state stayed empty during IME typing. This meant hasComposerPayload was always false — the Send button never appeared, and the submit path had nothing to send. 2. (Upstream already fixed) submitDraft() now uses draftRef.current Fixes: - Remove the composingRef guard from handleEditorInput. Draft now updates during IME composition so the Send button appears as soon as text is in the editor. The Enter guard in handleEditorKeyDown still blocks premature submission via event.nativeEvent.isComposing. - Add imeEnterRef to track Enter during IME composition. On compositionend, if set, the committed text is synced from the DOM into state and submitDraft() is scheduled — CJK users get one-Enter send instead of needing a second Enter. - Self-heal stale composition flag: clear composingRef on keydown when nativeEvent.isComposing is false, and reset on blur unconditionally (upstream fix from NousResearch#44149 applied locally). Closes NousResearch#44135
tonydwb
left a comment
There was a problem hiding this comment.
Code Review: PR #44149
Verdict: Approved — well-reasoned defensive fix for stale IME composition flag.
Summary
Files changed: apps/desktop/src/app/chat/composer/index.tsx (+17, -1), new test file (+129)
Adds recovery from a stale composingRef that silently blocks Enter and Send button when compositionend is missed (focus jumps, input-source switches, DOM mutations).
Assessment
Correctness: Two recovery points grounded in Chromium invariants: (1) keydown self-heal clears the flag when native isComposing says we're not composing; (2) blur unconditionally clears the flag. Both are safe — genuine IME composition is still protected.
No issues found.
Reviewed by Hermes Agent
|
Requesting maintainer review — this is ready to land from my side. Standalone fork CI is pending first-run approval here; the rollup branch in #44061 carrying this session's batch is fully green on upstream CI (all test shards, typecheck, e2e). |
9966f6a to
e172ae7
Compare
…he composer A missed compositionend (focus jump, input-source switch, programmatic DOM swap mid-preedit) left composingRef stuck true, and the stuck flag silently swallowed every Enter in handleEditorKeyDown and every Send-button submit via the form onSubmit guard — no error, no RPC, until the composer remounted. For CJK IME users (where even ASCII typing runs through composition) this read as "Enter has no effect; messages cannot be sent", degrading composer instance by composer instance. Recover in two places, both grounded in invariants Chromium guarantees: - keydown: every keydown during a genuine composition carries isComposing=true, so when the native flag says we're not composing, clear the stale ref before the guard reads it. - blur: a composition never survives focus loss, so clear the flag unconditionally — this is what unblocks the Send button path, which has no native composition flag to consult. The genuine-IME protection (NousResearch#37483 class) is untouched: Enter with isComposing=true is still swallowed. Fixes NousResearch#44135
e172ae7 to
7088441
Compare
|
Thanks for the focused regression fix. The underlying defect remains on current The proposed keydown recovery and blur reset directly cover those paths while preserving the genuine-composition guard. The regression cases in Current Automated hermes-sweeper review. |
Three-part fix for the Korean IME 'last character remains' bug on macOS desktop (same symptom as TUI NousResearch#38117, desktop variant NousResearch#44278): 1. Self-heal stale composingRef: clear the flag on keydown when composingRef is true but nativeEvent.isComposing is false — except for Enter, where the flag may still be legitimate (macOS Korean IME fires keydown(Enter) before compositionend). 2. Blur reset composingRef: unconditionally clear composingRef on blur since composition cannot survive focus loss in any browser. 3. Auto-submit on compositionend: when Enter was pressed during IME composition (guarded by composingRef/isComposing), flag it and auto-submit in compositionend after the IME commits its text. This prevents the 'press Enter twice' UX and the stranded last syllable. Refs: upstream NousResearch#44135, NousResearch#44149, NousResearch#44278
|
Thanks for this fix! It was salvaged into #86760 (cherry-picked onto current main with your authorship preserved in the commit history) and is now merged. Closing since the work has landed. |
Summary
Defensive fix for #44135 — Desktop composer Enter (and the Send button) silently doing nothing, with no error and no
session.create/prompt.submitever reaching the gateway.The composer guards submission behind
composingRef(set oncompositionstart, cleared oncompositionend) so that Enter confirming a CJK IME preedit doesn't split the message (#37483 class). But ifcompositionendis ever missed — focus jump, input-source switch, or a programmatic DOM mutation aborting the preedit — the flag wedgestrueand from then on every path out of the composer is silently swallowed:handleEditorKeyDownreturns before the Enter branch (composer/index.tsx)onSubmitguard blocks the Send button tooNo error surfaces, nothing hits the gateway, and the state persists until the composer instance remounts. For Chinese IME users even ASCII typing runs through composition (Pinyin ABC mode), so one wedge kills all sending — matching the reporter's "first new sessions, then everything" progression (pre-decoupling, composer instances differed per thread and degraded one by one as each was touched).
Fix
Two recovery points, each grounded in an invariant Chromium (Electron's only engine) guarantees:
nativeEvent.isComposing === true. If the ref says composing but the native flag says no, the ref is stale: clear it before the guard reads it.compositionendon blur). Clear the flag unconditionally on blur; this is what un-wedges the Send button path, which has no native composition flag to consult.Genuine IME protection is unchanged: Enter with
isComposing=trueis still swallowed, and the new test locks that in alongside the recovery behavior.Testing
enter-stale-ime-flag.test.tsx(harness-mirror style, same asenter-submit-dom-race.test.tsx): wedged-flag Enter recovery, genuine-composition swallow + post-compositionendsend, Send-button unblock after blur with a missedcompositionend.npx vitest run --environment jsdom src/app/chat/composer/— 7 files, 25 tests pass.tsc -bandeslintclean on touched files.Note for the reporter's setup: the bootstrap
install.shHTTP 404s in their log are a separate, expected fallback path (locally-built app stamped to an unpushed commit;bootstrap-runner.cjsfalls back to the installed checkout's script) and are not related to this bug.Fixes #44135