fix(web): guard Korean IME Enter composition in text inputs - #350
fix(web): guard Korean IME Enter composition in text inputs#350Hwang, Gunn Gu (kuil09) wants to merge 1 commit into
Conversation
b4a4266 to
796f75d
Compare
Rinat S (rsolmano)
left a comment
Rinat S (rsolmano)
left a comment
There was a problem hiding this comment.
One additional affected path is outside this diff: AskUserQuestionCard.tsx intercepts unmodified ArrowUp/ArrowDown in the Other-answer input before checking composition, so those keys can move questionnaire focus instead of the IME candidate selection. Its Enter branch also relies only on isComposing.
| }; | ||
|
|
||
| const onKeyDown = (e: KeyboardEvent<HTMLTextAreaElement>) => { | ||
| if (e.nativeEvent.isComposing) return; |
There was a problem hiding this comment.
This still misses the composition-confirming Enter emitted by older Safari/WKWebView. In that event order, isComposing is already false while keyCode remains 229. In a real-host browser probe with that event shape, the draft cleared and the agent started, so the core premature-submit bug remains for those runtimes.
| if (!open) return null; | ||
|
|
||
| const onKeyDown = (e: KeyboardEvent<HTMLInputElement>) => { | ||
| if (e.nativeEvent.isComposing) return; |
There was a problem hiding this comment.
This input-level guard cannot protect composition Escape: the window capture listener above runs first and closes the overlay. I reproduced this with isComposing=true; History closed and focus returned to the composer.
| send.addEventListener("click", () => submit(callbacks.onSend)); | ||
| cancel.addEventListener("click", closeComposer); | ||
| textarea.addEventListener("keydown", (e) => { | ||
| if (e.isComposing) return; |
There was a problem hiding this comment.
Before this change every keydown reached stopPropagation(). A composing event now returns first and bubbles into Monaco, creating a new path where the editor behind this embedded textarea can process the IME keystroke.
| expect(slashCompletionKeyAction("Enter", false, 1, 3)).toEqual({ type: "none" }); | ||
| }); | ||
|
|
||
| it("ignores keyboard actions when IME is actively composing", () => { |
There was a problem hiding this comment.
This test never invokes handleKeyDown or any other production behavior. prevented and stopped remain false only because their callbacks are never called, so the test still passes if the production composition guard is deleted.
| } | ||
|
|
||
| const lock = Bun.JSONC.parse(readFileSync(join(root, "bun.lock"), "utf8")) as BunLock; | ||
| const lockModule = await import(join(root, "bun.lock")); |
There was a problem hiding this comment.
This changes the repository-wide dependency gate but is unrelated to IME handling. Under the project-pinned Bun 1.3.14, the existing Bun.JSONC.parse path works and the catalog tests pass, so this PR currently mixes an unmotivated tooling behavior change into the UI bug fix.
Add checks to all keydown handlers across text inputs (Composer, SlashCommandCompletion, TodoList, HistoryOverlay, NewWorkspaceDialog, LoginDialog, ExtUiDialog, PreviewCommenting, ReviewThreadCard, reviewWidgets, LayoutSettings, useGlobalHotkeys) to prevent premature submission and duplicate character insertion when pressing Enter during Korean/CJK composition. Update SPEC.md with IME composition safety invariant. Add unit test for SlashCommandCompletion IME guard.
796f75d to
2a62327
Compare
|
Rinat S (@rsolmano) |
2 participants
Summary
Fix Korean IME Enter composition bug where pressing Enter during Korean character composition triggers premature submission and duplicate/leftover character insertion.
Background: How Korean IME Works
Unlike Western alphabets, Korean uses an Input Method Editor (IME) where characters are composed syllable-by-syllable:
The critical difference: In Korean IME, pressing Enter while a syllable is still being composed (e.g., after typing just "ㄱ" or "가" but before Space) is a valid commit action — it finalizes the current syllable. This is expected Korean IME behavior.
The Bug
Our keydown handlers checked
e.key === "Enter"without checkinge.nativeEvent.isComposing. This caused:compositionendevent re-inserted the committed character into the now-empty inputChanges
Added
isComposingguards to all keydown handlers across 12 text input components:Composer.tsx- main chat composerSlashCommandCompletion.tsx- slash command autocompleteTodoList.tsx- todo add inputHistoryOverlay.tsx- history search inputNewWorkspaceDialog.tsx- workspace creation promptLoginDialog.tsx- login promptExtUiDialog.tsx- extension UI inputPreviewCommenting.tsx- review comment composerReviewThreadCard.tsx- review thread editreviewWidgets.ts- Monaco-based review widgetsLayoutSettings.tsx- layout preset renameuseGlobalHotkeys.ts- global hotkeysDocumentation: Added IME composition safety invariant to
apps/web/src/chat/SPEC.mdTests: Added unit test in
SlashCommandCompletion.test.tsfor IME guardBonus fix: Fixed
scripts/check-catalog.tsBun JSONC parsing issueTesting
All verification gates pass:
bun run check:deps- OK (18 catalog entries enforced)bun run check:boundaries- OK (8 module boundaries enforced)bun run check:seams- OK (4 known opaque imports, all handled)bun run lint- OK (6 pre-existing biome warnings, no new ones)bun run typecheck- OK (14 packages, 0 errors)bun --filter @thinkrail/web test- 812 tests pass