fix(frontend): don't send on Enter while an IME is composing#247
Merged
Conversation
Pressing Enter to pick a candidate from a Chinese (or Japanese/Korean) IME sent the message instead, committing the raw pinyin. While an IME is composing, the browser still fires keydown for Enter and marks it isComposing — the keystroke belongs to the candidate popup, not to us. Nothing checked that flag, so the composer read "confirm this word" as "send". Add a shared isComposing() helper (nativeEvent.isComposing, plus the legacy keyCode 229 some browsers still emit) and guard on it. In the composer the guard sits at the top of handleKeyDown rather than on the send branch alone: the @// picker also claims Enter/Arrow/Escape, and those keys navigate the IME popup while it is open. Apply the same guard to the other Enter-to-submit text inputs that had the identical bug: new channel/workspace/session dialogs, the workbench file-name input, the kanban task input, and friend lookup. Password fields are left alone — browsers disable IME there. Verified against the in-cluster gateway by driving the real composer with a keydown carrying isComposing: true, A/B'd by stashing the fix. Before: draft cleared (sent). After: draft kept; a normal Enter still sends, and exactly one message was persisted across both dispatches. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Change description
Pressing Enter to pick a candidate from a Chinese IME sent the message instead of selecting the word — so you got the raw pinyin posted to the channel.
Cause. While an IME is composing, the browser still fires
keydownfor Enter; it just marks itisComposing: true, because that keystroke belongs to the candidate popup rather than to the app.MessageComposeronly checked!e.shiftKey, and nothing in the codebase looked at that flag, so "confirm this word" was read as "send".Fix. A shared
isComposing()helper infrontend/src/lib/ime.ts(nativeEvent.isComposing, plus the legacykeyCode === 229some browsers still emit instead), guarded at every Enter-to-submit text input.In the composer the guard sits at the top of
handleKeyDownrather than on the send branch alone — the@//picker also claims Enter, Arrow, and Escape, and all of those navigate the IME popup while it's open. Guarding only the send path would have left the picker stealing candidate navigation.The same bug existed in six other Enter-to-submit inputs, all fixed here: new channel / workspace / session dialogs, the workbench file-name input, the kanban task input, and friend lookup. Password fields are deliberately left alone — browsers disable IME there, so a guard would be dead code.
Change type
Test
pytestpassed in fullnpm run buildno error reportednpm run lint/npm test/npm run buildof the corresponding package has passedVerified end-to-end against the in-cluster gateway (kind + Helm) by driving the real composer with a
keydowncarryingisComposing: true— exactly what the browser emits mid-composition — and A/B'd by stashing the fix:The final send round-tripped through the gateway and rendered in the channel, and exactly one message was persisted despite dispatching both a composing Enter and a normal Enter — confirming the composing one was swallowed, not merely delayed.
No unit test added:
isComposing()is a two-term predicate whose only real behaviour is the browser flag it reads, so a test would restate the implementation. The repo's vitest suite covers domain logic (yaml, registry) rather than helpers of this shape, and the behaviour that matters here was verified in a real browser instead.Database migration
Security and Release Impact
.env, logs, databases, uploaded files, private keys, tokens or production configurationsReviewer notes
tsc --noEmitreports 2 errors infrontend/src/features/chat/PdfViewer.tsxfrom pdfjs API drift (RenderParameters.canvas,PDFDocumentProxy.destroy). Confirmed present on a clean tree, untouched by this PR, and they don't failnpm run build(vite builddoesn't typecheck). Worth a separate fix.node_moduleswas stale and missing declared deps (yaml,@codemirror/*);npm ciresolved it. The lockfile is untouched by this PR.Related Issues
Closes #
🤖 Generated with Claude Code