From 363b0fc1b4a5bd6c142d1f1df9fb9743cca550a3 Mon Sep 17 00:00:00 2001 From: Shxiao Date: Thu, 24 Sep 2026 18:56:48 +0900 Subject: [PATCH] fix(composer): keep IME-composition Enter out of the slash-skill catalog (#5741) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(composer): keep IME-composition Enter out of the slash-skill catalog `handlePromptTextareaKeyDown` called `handleSkillSuggestionKeyDown` with no composition guard, unlike its two siblings: the inline-skill editor wraps the same call in `isIMEComposing(...)` with a comment naming this exact hazard, and `handlePromptHistoryKeyDown` checks it too. `PromptInputTextarea` calls the prop's `onKeyDown` *before* its own `isIMEComposing` check, so the suggestion handler won the event. With an IME active and the `/` catalog open, pressing Enter to commit the composition instead ran `event.preventDefault()` and applied the highlighted suggestion — discarding the text still being composed and selecting a skill the user never chose. The catalog opens from the live `textInput.value`, and React `onChange` fires during composition, so a CJK user typing `/res` to reach a skill hits it directly. Guard the main composer's call site the same way the inline-skill editor does, and pin it with a happy-dom regression test: `keyCode: 229` (the keydown a browser reports for a key an active IME consumed) must leave both the catalog and the draft untouched, while a plain Enter still selects the highlighted skill. Verified: - new test `tests/unit/components/workspace/input-box-skill-suggestion-ime.dom.test.tsx` fails on main (the IME case applies the skill) and passes here; the plain-Enter control case passes on both - `rstest run --pool threads tests/unit/components/workspace` -> 69 files / 448 tests passed - `tsc --noEmit`, `eslint`, `prettier --check` clean on the touched files * fix(composer): ignore the Enter that follows IME compositionend Safari emits the confirming Enter after compositionend, with neither isComposing nor keyCode 229. Drop that Enter in PromptInputTextarea for a short window, before the skill catalog or prompt history can treat it as a plain key. --------- Co-authored-by: Shxiao101 --- .../components/ai-elements/prompt-input.tsx | 14 +- .../src/components/workspace/input-box.tsx | 13 +- frontend/src/lib/ime.ts | 17 ++ ...nput-box-skill-suggestion-ime.dom.test.tsx | 159 ++++++++++++++++++ frontend/tests/unit/lib/ime.test.ts | 63 +++++++ 5 files changed, 261 insertions(+), 5 deletions(-) create mode 100644 frontend/tests/unit/components/workspace/input-box-skill-suggestion-ime.dom.test.tsx create mode 100644 frontend/tests/unit/lib/ime.test.ts diff --git a/frontend/src/components/ai-elements/prompt-input.tsx b/frontend/src/components/ai-elements/prompt-input.tsx index dd9a20a29d..51a5419473 100644 --- a/frontend/src/components/ai-elements/prompt-input.tsx +++ b/frontend/src/components/ai-elements/prompt-input.tsx @@ -36,7 +36,7 @@ import { } from "@/components/ui/select"; import type { PromptInputFilePart } from "@/core/uploads"; import { splitUnsupportedUploadFiles } from "@/core/uploads"; -import { isIMEComposing } from "@/lib/ime"; +import { isCompositionConfirmEnter, isIMEComposing } from "@/lib/ime"; import { cn } from "@/lib/utils"; import type { ChatStatus } from "ai"; import { @@ -890,8 +890,15 @@ export const PromptInputTextarea = ({ const attachments = usePromptInputAttachments(); const sanitizeIncomingFiles = usePromptInputValidation(); const [isComposing, setIsComposing] = useState(false); + const compositionEndedAtRef = useRef(0); const handleKeyDown: KeyboardEventHandler = (e) => { + // Drop the post-compositionend Enter before parent handlers (the skill + // catalog and prompt history) can treat it as a plain Enter. + if (isCompositionConfirmEnter(e, compositionEndedAtRef.current)) { + e.preventDefault(); + return; + } onKeyDown?.(e); if (e.defaultPrevented) { return; @@ -963,7 +970,10 @@ export const PromptInputTextarea = ({ setIsComposing(false)} + onCompositionEnd={() => { + compositionEndedAtRef.current = Date.now(); + setIsComposing(false); + }} onCompositionStart={() => setIsComposing(true)} onKeyDown={handleKeyDown} onPaste={handlePaste} diff --git a/frontend/src/components/workspace/input-box.tsx b/frontend/src/components/workspace/input-box.tsx index e1b181b9e9..c44722eb86 100644 --- a/frontend/src/components/workspace/input-box.tsx +++ b/frontend/src/components/workspace/input-box.tsx @@ -1958,9 +1958,16 @@ export function InputBox({ const handlePromptTextareaKeyDown = useCallback( (event: KeyboardEvent) => { - handleSkillSuggestionKeyDown(event); - if (event.defaultPrevented) { - return; + // Same rule as the inline-skill editor: the catalog's navigation keys must + // win over Enter-to-submit, but not mid-composition, where Enter belongs + // to the IME candidate rather than the list. Safari's confirming Enter + // carries neither composition flag; PromptInputTextarea drops it before + // this handler runs. + if (!isIMEComposing(event)) { + handleSkillSuggestionKeyDown(event); + if (event.defaultPrevented) { + return; + } } handleSelectedSlashSkillKeyDown(event); if (event.defaultPrevented) { diff --git a/frontend/src/lib/ime.ts b/frontend/src/lib/ime.ts index 5ccd913d8d..71ead9571d 100644 --- a/frontend/src/lib/ime.ts +++ b/frontend/src/lib/ime.ts @@ -8,3 +8,20 @@ export function isIMEComposing( ): boolean { return isComposing || event.nativeEvent.isComposing || event.keyCode === 229; } + +// Safari emits the Enter that confirms a composition after compositionend, +// with isComposing false and keyCode 13. React has already flushed the +// composing state by the time that keydown runs. +export const COMPOSITION_CONFIRM_ENTER_MS = 50; + +export function isCompositionConfirmEnter( + event: IMEKeyboardEvent, + compositionEndedAt: number, + now = Date.now(), +): boolean { + if (event.key !== "Enter" || event.shiftKey || isIMEComposing(event)) { + return false; + } + const elapsed = now - compositionEndedAt; + return elapsed >= 0 && elapsed < COMPOSITION_CONFIRM_ENTER_MS; +} diff --git a/frontend/tests/unit/components/workspace/input-box-skill-suggestion-ime.dom.test.tsx b/frontend/tests/unit/components/workspace/input-box-skill-suggestion-ime.dom.test.tsx new file mode 100644 index 0000000000..d83f07cef5 --- /dev/null +++ b/frontend/tests/unit/components/workspace/input-box-skill-suggestion-ime.dom.test.tsx @@ -0,0 +1,159 @@ +import { afterEach, describe, expect, it, rs } from "@rstest/core"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { cleanup, fireEvent, render, screen } from "@testing-library/react"; +import type { ReactNode } from "react"; + +import { PromptInputProvider } from "@/components/ai-elements/prompt-input"; +import { InputBox } from "@/components/workspace/input-box"; +import { ThreadContext } from "@/components/workspace/messages/context"; +import { AuthProvider } from "@/core/auth/AuthProvider"; +import { DEFAULT_LOCALE } from "@/core/i18n"; +import { I18nProvider } from "@/core/i18n/context"; +import { COMPOSITION_CONFIRM_ENTER_MS } from "@/lib/ime"; + +rs.mock("next/navigation", () => ({ + useRouter: () => ({ push: rs.fn(), replace: rs.fn(), refresh: rs.fn() }), + usePathname: () => "/workspace", + useSearchParams: () => new URLSearchParams(), +})); + +rs.mock("@/core/models/hooks", () => ({ + useModels: () => ({ + models: [], + tokenUsageEnabled: false, + isLoading: false, + isFetching: false, + error: null, + refetch: rs.fn(), + }), +})); + +rs.mock("@/core/skills/hooks", () => ({ + useSkills: () => ({ + skills: [ + { + name: "research", + description: "Research a topic", + category: "general", + license: "MIT", + enabled: true, + editable: false, + }, + ], + isLoading: false, + error: null, + }), +})); + +// Each test gets its own thread id: the composer persists the selected skill in +// a debounced, thread-scoped draft, and a shared id would let the first test's +// selection land in the second test's storage key. +function renderComposer(threadId: string) { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, + }); + const tree: ReactNode = ( + + + + + + + + + + + + ); + return render(tree); +} + +/** Focus the composer and type a leading slash query that opens the catalog. */ +function openSkillCatalog(container: HTMLElement): HTMLTextAreaElement { + const textarea = container.querySelector("textarea"); + if (!(textarea instanceof HTMLTextAreaElement)) { + throw new Error("composer textarea not rendered"); + } + fireEvent.focus(textarea); + fireEvent.change(textarea, { target: { value: "/res" } }); + screen.getByRole("listbox", { name: "Skill suggestions" }); + return textarea; +} + +afterEach(() => { + rs.restoreAllMocks(); + window.sessionStorage.clear(); + cleanup(); +}); + +describe("InputBox skill suggestion IME handling", () => { + it("leaves Enter to the IME candidate window while the catalog is open", () => { + const { container } = renderComposer("thread-ime-composing"); + const textarea = openSkillCatalog(container); + + // 229 is the keyCode browsers report for a keydown consumed by an active + // IME composition; the catalog must not claim it. + fireEvent.keyDown(textarea, { key: "Enter", keyCode: 229 }); + + expect( + screen.queryByRole("button", { name: "Remove /research" }), + ).toBeNull(); + expect(textarea.value).toBe("/res"); + }); + + it("does not apply a suggestion for the Enter that follows compositionend", () => { + const { container } = renderComposer("thread-ime-composition-end"); + const textarea = openSkillCatalog(container); + + // Safari reports the confirming Enter after compositionend, with neither + // isComposing nor keyCode 229 set. + fireEvent.compositionEnd(textarea); + fireEvent.keyDown(textarea, { key: "Enter", keyCode: 13 }); + + expect( + screen.queryByRole("button", { name: "Remove /research" }), + ).toBeNull(); + expect(textarea.value).toBe("/res"); + }); + + it("still selects the highlighted skill once the confirm window has passed", () => { + const { container } = renderComposer("thread-ime-after-window"); + const textarea = openSkillCatalog(container); + const endedAt = Date.now(); + const now = rs.spyOn(Date, "now"); + now.mockReturnValue(endedAt); + fireEvent.compositionEnd(textarea); + now.mockReturnValue(endedAt + COMPOSITION_CONFIRM_ENTER_MS); + + fireEvent.keyDown(textarea, { key: "Enter", keyCode: 13 }); + + expect( + screen.getByRole("button", { name: "Remove /research" }), + ).toBeTruthy(); + }); + + it("still selects the highlighted skill on a plain Enter", () => { + const { container } = renderComposer("thread-plain-enter"); + const textarea = openSkillCatalog(container); + + fireEvent.keyDown(textarea, { key: "Enter" }); + + expect( + screen.getByRole("button", { name: "Remove /research" }), + ).toBeTruthy(); + }); +}); diff --git a/frontend/tests/unit/lib/ime.test.ts b/frontend/tests/unit/lib/ime.test.ts new file mode 100644 index 0000000000..c94ac4039f --- /dev/null +++ b/frontend/tests/unit/lib/ime.test.ts @@ -0,0 +1,63 @@ +import { describe, expect, it } from "@rstest/core"; +import type { KeyboardEvent } from "react"; + +import { + COMPOSITION_CONFIRM_ENTER_MS, + isCompositionConfirmEnter, +} from "@/lib/ime"; + +function enter( + overrides: Partial> = {}, +): KeyboardEvent { + return { + key: "Enter", + shiftKey: false, + keyCode: 13, + nativeEvent: { isComposing: false }, + ...overrides, + } as KeyboardEvent; +} + +describe("isCompositionConfirmEnter", () => { + const endedAt = 1_000; + + it("matches the Enter that arrives just after compositionend", () => { + expect(isCompositionConfirmEnter(enter(), endedAt, endedAt)).toBe(true); + expect( + isCompositionConfirmEnter( + enter(), + endedAt, + endedAt + COMPOSITION_CONFIRM_ENTER_MS - 1, + ), + ).toBe(true); + }); + + it("lets a later Enter through", () => { + expect( + isCompositionConfirmEnter( + enter(), + endedAt, + endedAt + COMPOSITION_CONFIRM_ENTER_MS, + ), + ).toBe(false); + expect(isCompositionConfirmEnter(enter(), 0, endedAt)).toBe(false); + }); + + it("does not claim Shift+Enter or a keydown that is still composing", () => { + expect( + isCompositionConfirmEnter(enter({ shiftKey: true }), endedAt, endedAt), + ).toBe(false); + expect( + isCompositionConfirmEnter(enter({ keyCode: 229 }), endedAt, endedAt), + ).toBe(false); + expect( + isCompositionConfirmEnter( + enter({ + nativeEvent: { isComposing: true } as KeyboardEvent["nativeEvent"], + }), + endedAt, + endedAt, + ), + ).toBe(false); + }); +});