From bfc9378ab1bf445b1677e4408fadf8dc79f59ee8 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 2 Aug 2026 15:20:50 -0700 Subject: [PATCH] fix(ime): close remaining composition guard gaps --- .../components/NewWorkspaceComposerModal.tsx | 4 + .../browser-pane/BrowserAddressBar.test.tsx | 33 +++++ .../browser-pane/BrowserAddressBar.tsx | 7 + .../editor/EditorPanelHeaderPath.tsx | 4 + .../RichMarkdownLinkBubble.render.test.tsx | 61 +++++--- .../editor/RichMarkdownLinkBubble.tsx | 7 + .../components/github-project/ProjectCell.tsx | 7 + .../github/GitHubMarkdownComposer.tsx | 22 ++- .../github/github-markdown-composer-props.ts | 14 ++ .../mobile/use-mobile-page-escape.test.tsx | 40 ++++++ .../mobile/use-mobile-page-escape.ts | 4 + .../sidebar/WorkspaceKanbanSearchField.tsx | 2 +- .../keyboard-handlers-ime.test.tsx | 109 +++++++++++++++ .../terminal-pane/keyboard-handlers.ts | 24 ++++ .../terminal-ime-deferred-newline.test.ts | 19 ++- .../terminal-ime-deferred-newline.ts | 7 +- ...erminal-shortcut-composition-guard.test.ts | 4 + .../terminal-shortcut-composition-guard.ts | 3 +- src/renderer/src/components/ui/command.tsx | 3 + .../src/components/ui/context-menu.tsx | 9 ++ src/renderer/src/components/ui/dialog.tsx | 5 + ...issable-layer-ime-escape-invariant.test.ts | 50 +++++++ .../ui/dismissable-layer-ime-escape.test.tsx | 130 ++++++++++++++++++ .../src/components/ui/dropdown-menu.tsx | 9 ++ src/renderer/src/components/ui/hover-card.tsx | 5 + src/renderer/src/components/ui/popover.tsx | 5 + src/renderer/src/components/ui/select.tsx | 5 + src/renderer/src/components/ui/sheet.tsx | 5 + src/renderer/src/components/ui/tooltip.tsx | 5 + .../ui/use-ime-aware-escape-key-down.ts | 21 +++ .../src/lib/ime-escape-guard-ordering.test.ts | 46 +++++-- 31 files changed, 615 insertions(+), 54 deletions(-) create mode 100644 src/renderer/src/components/github/github-markdown-composer-props.ts create mode 100644 src/renderer/src/components/mobile/use-mobile-page-escape.test.tsx create mode 100644 src/renderer/src/components/ui/dismissable-layer-ime-escape-invariant.test.ts create mode 100644 src/renderer/src/components/ui/dismissable-layer-ime-escape.test.tsx create mode 100644 src/renderer/src/components/ui/use-ime-aware-escape-key-down.ts diff --git a/src/renderer/src/components/NewWorkspaceComposerModal.tsx b/src/renderer/src/components/NewWorkspaceComposerModal.tsx index f04a8f80de4..b7da3599f2f 100644 --- a/src/renderer/src/components/NewWorkspaceComposerModal.tsx +++ b/src/renderer/src/components/NewWorkspaceComposerModal.tsx @@ -18,6 +18,7 @@ import { } from '@/lib/quick-workspace-agent-selection' import type { LinkedWorkItemSummary } from '@/lib/new-workspace' import { shouldAllowComposerEnterSubmitTarget } from '@/lib/new-workspace-enter-guard' +import { isImeCompositionKeyDown } from '@/lib/ime-composition-keyboard-event' import { isScreenSubmitShortcut } from '@/lib/screen-submit-shortcut' import type { TuiAgent, @@ -247,6 +248,9 @@ function QuickTabBody({ return } const onKeyDown = (event: KeyboardEvent): void => { + if (isImeCompositionKeyDown(event)) { + return + } if (event.key !== 'Enter' && event.key !== 'Escape') { return } diff --git a/src/renderer/src/components/browser-pane/BrowserAddressBar.test.tsx b/src/renderer/src/components/browser-pane/BrowserAddressBar.test.tsx index 0cf202ff2d8..6245c716c08 100644 --- a/src/renderer/src/components/browser-pane/BrowserAddressBar.test.tsx +++ b/src/renderer/src/components/browser-pane/BrowserAddressBar.test.tsx @@ -212,6 +212,39 @@ describe('BrowserAddressBar autocomplete preview', () => { expect(onSubmit).not.toHaveBeenCalled() }) + it('keeps suggestions open for an IME-owned Escape', async () => { + const onNavigate = vi.fn() + const onSubmit = vi.fn() + + await act(async () => { + root.render( + + ) + }) + + const input = container.querySelector('input[data-orca-browser-address-bar]') + expect(input).not.toBeNull() + + await act(async () => { + input?.focus() + }) + await act(async () => { + input?.dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowDown', bubbles: true })) + }) + + await act(async () => { + input?.dispatchEvent( + new KeyboardEvent('keydown', { key: 'Escape', bubbles: true, isComposing: true }) + ) + }) + + expect(container.querySelector('[data-current-address-value="true"]')?.textContent).toBe( + 'http://localhost:3000/review-one' + ) + expect(onNavigate).not.toHaveBeenCalled() + expect(onSubmit).not.toHaveBeenCalled() + }) + it('dismisses suggestions when focus moves into an Electron webview guest', async () => { const onNavigate = vi.fn() const onSubmit = vi.fn() diff --git a/src/renderer/src/components/browser-pane/BrowserAddressBar.tsx b/src/renderer/src/components/browser-pane/BrowserAddressBar.tsx index b4e6969bf52..0dc732b177e 100644 --- a/src/renderer/src/components/browser-pane/BrowserAddressBar.tsx +++ b/src/renderer/src/components/browser-pane/BrowserAddressBar.tsx @@ -4,6 +4,7 @@ import { Globe } from 'lucide-react' import { Input } from '@/components/ui/input' import { Popover, PopoverContent, PopoverTrigger } from '@/components/ui/popover' import { cn } from '@/lib/utils' +import { isImeCompositionKeyDown } from '@/lib/ime-composition-keyboard-event' import { useAppStore } from '@/store' import { DEFAULT_SEARCH_ENGINE, type SearchEngine } from '../../../../shared/browser-url' import { buildBrowserAddressBarSuggestions } from './browser-address-bar-suggestions' @@ -229,6 +230,9 @@ export default function BrowserAddressBar({ const handleKeyDown = useCallback( (event: React.KeyboardEvent) => { + if (isImeCompositionKeyDown(event)) { + return + } if (event.key === 'Escape') { cancelSuggestionPreview() return @@ -329,6 +333,9 @@ export default function BrowserAddressBar({ } const handleEscape = (event: KeyboardEvent): void => { + if (isImeCompositionKeyDown(event)) { + return + } if (event.key !== 'Escape') { return } diff --git a/src/renderer/src/components/editor/EditorPanelHeaderPath.tsx b/src/renderer/src/components/editor/EditorPanelHeaderPath.tsx index f1d0ae5e7b6..3d55f58290e 100644 --- a/src/renderer/src/components/editor/EditorPanelHeaderPath.tsx +++ b/src/renderer/src/components/editor/EditorPanelHeaderPath.tsx @@ -11,6 +11,7 @@ import { import { Input } from '@/components/ui/input' import { useShortcutLabel } from '@/hooks/useShortcutLabel' import { translate } from '@/i18n/i18n' +import { isImeCompositionKeyDown } from '@/lib/ime-composition-keyboard-event' import type { OpenFile } from '@/store/slices/editor' import { CLOSE_ALL_CONTEXT_MENUS_EVENT } from '../tab-bar/SortableTab' import { useEditorHeaderFileRename } from './editor-header-file-rename' @@ -96,6 +97,9 @@ export function EditorPanelHeaderPath({ onClick={(event) => event.stopPropagation()} onDoubleClick={(event) => event.stopPropagation()} onKeyDown={(event) => { + if (isImeCompositionKeyDown(event)) { + return + } if (event.key === 'Enter') { event.preventDefault() event.stopPropagation() diff --git a/src/renderer/src/components/editor/RichMarkdownLinkBubble.render.test.tsx b/src/renderer/src/components/editor/RichMarkdownLinkBubble.render.test.tsx index eae0896a1aa..fcd44f2fc4a 100644 --- a/src/renderer/src/components/editor/RichMarkdownLinkBubble.render.test.tsx +++ b/src/renderer/src/components/editor/RichMarkdownLinkBubble.render.test.tsx @@ -28,28 +28,36 @@ const LINK_BUBBLE: LinkBubbleState = { copyEnabled: true } -function renderBubble(): { onDismiss: ReturnType } { +function renderBubble({ isEditing = false }: { isEditing?: boolean } = {}): { + onDismiss: ReturnType + onEditCancel: ReturnType + onAncestorKeyDown: ReturnType +} { const anchorElement = document.createElement('div') document.body.append(anchorElement) const onDismiss = vi.fn() + const onEditCancel = vi.fn() + const onAncestorKeyDown = vi.fn() render( - - - +
+ + + +
) onDismiss.mockClear() - return { onDismiss } + return { onDismiss, onEditCancel, onAncestorKeyDown } } describe('RichMarkdownLinkBubble viewport dismissal', () => { @@ -80,4 +88,25 @@ describe('RichMarkdownLinkBubble viewport dismissal', () => { expect(onDismiss).toHaveBeenCalled() }) + + it('keeps link editing open for an IME-owned Escape', () => { + const { onDismiss, onEditCancel, onAncestorKeyDown } = renderBubble({ isEditing: true }) + const input = document.querySelector('.rich-markdown-link-input') + + fireEvent.keyDown(input!, { key: 'Escape', keyCode: 27, isComposing: true }) + + expect(onEditCancel).not.toHaveBeenCalled() + expect(onDismiss).not.toHaveBeenCalled() + expect(onAncestorKeyDown).not.toHaveBeenCalled() + }) + + it('still cancels link editing for ordinary Escape', () => { + const { onDismiss, onEditCancel } = renderBubble({ isEditing: true }) + const input = document.querySelector('.rich-markdown-link-input') + + fireEvent.keyDown(input!, { key: 'Escape', keyCode: 27 }) + + expect(onEditCancel).toHaveBeenCalledOnce() + expect(onDismiss).not.toHaveBeenCalled() + }) }) diff --git a/src/renderer/src/components/editor/RichMarkdownLinkBubble.tsx b/src/renderer/src/components/editor/RichMarkdownLinkBubble.tsx index 0751d7216e5..07d7b6d21d9 100644 --- a/src/renderer/src/components/editor/RichMarkdownLinkBubble.tsx +++ b/src/renderer/src/components/editor/RichMarkdownLinkBubble.tsx @@ -6,6 +6,7 @@ import { translate } from '@/i18n/i18n' import { Button } from '@/components/ui/button' import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' import { addViewportSizeChangeListener } from '@/hooks/viewport-size-change-listener' +import { isImeCompositionKeyDown } from '@/lib/ime-composition-keyboard-event' export type LinkBubbleState = { kind: 'markdown' | 'html-superscript' @@ -134,6 +135,9 @@ function LinkEditInput({ value={value} onChange={(e) => setValue(e.target.value)} onKeyDown={(e) => { + if (isImeCompositionKeyDown(e)) { + return + } if (e.key === 'Enter') { e.preventDefault() onSave(value.trim()) @@ -298,6 +302,9 @@ export function RichMarkdownLinkBubble({ }} onKeyDown={(event) => { event.stopPropagation() + if (isImeCompositionKeyDown(event)) { + return + } if (event.key === 'Escape') { event.preventDefault() onDismiss() diff --git a/src/renderer/src/components/github-project/ProjectCell.tsx b/src/renderer/src/components/github-project/ProjectCell.tsx index 305fccc3873..72cf0f60888 100644 --- a/src/renderer/src/components/github-project/ProjectCell.tsx +++ b/src/renderer/src/components/github-project/ProjectCell.tsx @@ -11,6 +11,7 @@ import { TYPE_FIELD_DATA_TYPE } from './columns' import { Popover, PopoverContent, PopoverTrigger } from '@/components/ui/popover' import { Input } from '@/components/ui/input' import { cn } from '@/lib/utils' +import { isImeCompositionKeyDown } from '@/lib/ime-composition-keyboard-event' import { useRepoAssigneesBySlug, useRepoLabelsBySlug } from '@/hooks/useGitHubSlugMetadata' import { useAppStore } from '@/store' import { callRuntimeRpc, getActiveRuntimeTarget } from '@/runtime/runtime-rpc-client' @@ -726,6 +727,9 @@ function TextCell({ } }} onKeyDown={(e) => { + if (isImeCompositionKeyDown(e)) { + return + } if (e.key === 'Enter') { e.preventDefault() setEditing(false) @@ -779,6 +783,9 @@ function DateCell({ } }} onKeyDown={(e) => { + if (isImeCompositionKeyDown(e)) { + return + } if (e.key === 'Enter') { e.preventDefault() ;(e.target as HTMLInputElement).blur() diff --git a/src/renderer/src/components/github/GitHubMarkdownComposer.tsx b/src/renderer/src/components/github/GitHubMarkdownComposer.tsx index 0d7d78d1e6f..da1ade7e3d5 100644 --- a/src/renderer/src/components/github/GitHubMarkdownComposer.tsx +++ b/src/renderer/src/components/github/GitHubMarkdownComposer.tsx @@ -6,6 +6,7 @@ import { ImageIcon, Paperclip } from 'lucide-react' import { Button } from '@/components/ui/button' import { Input } from '@/components/ui/input' import { cn } from '@/lib/utils' +import { isImeCompositionKeyDown } from '@/lib/ime-composition-keyboard-event' import { isScreenSubmitShortcut } from '@/lib/screen-submit-shortcut' import { createRichMarkdownExtensions } from '@/components/editor/rich-markdown-extensions' import { RichMarkdownToolbar } from '@/components/editor/RichMarkdownToolbar' @@ -26,26 +27,13 @@ import { } from '@/components/github/github-markdown-composer-tabbar' import { hasBoundedGitHubMarkdownImageUrlText } from '@/components/github/github-markdown-image-url' import { useImageInput } from '@/components/github/use-image-input' -import type { GitHubOwnerRepo } from '../../../../shared/types' import { translate } from '@/i18n/i18n' import { useAppStore } from '@/store' import { getRichMarkdownSpellcheckAttribute, useRichMarkdownSpellcheckAttribute } from '@/components/editor/rich-markdown-spellcheck' - -type GitHubMarkdownComposerProps = { - value: string - onChange: (value: string) => void - placeholder: string - minHeightClassName?: string - className?: string - disabled?: boolean - autoFocus?: boolean - onSubmitShortcut?: () => void - layout?: 'stacked' | 'tabbed' - previewGithubRepo?: GitHubOwnerRepo | null -} +import type { GitHubMarkdownComposerProps } from './github-markdown-composer-props' export function GitHubMarkdownComposer({ value, @@ -129,6 +117,9 @@ export function GitHubMarkdownComposer({ spellcheck: getRichMarkdownSpellcheckAttribute(richMarkdownSpellcheckEnabled) }, handleKeyDown: (_view, event) => { + if (isImeCompositionKeyDown(event)) { + return false + } if (isScreenSubmitShortcut(event)) { const submit = onSubmitShortcutRef.current if (submit) { @@ -306,6 +297,9 @@ export function GitHubMarkdownComposer({ value={imageUrl} onChange={(event) => setImageUrl(event.target.value)} onKeyDown={(event) => { + if (isImeCompositionKeyDown(event)) { + return + } if (isScreenSubmitShortcut(event)) { event.preventDefault() event.stopPropagation() diff --git a/src/renderer/src/components/github/github-markdown-composer-props.ts b/src/renderer/src/components/github/github-markdown-composer-props.ts new file mode 100644 index 00000000000..89b29a09da1 --- /dev/null +++ b/src/renderer/src/components/github/github-markdown-composer-props.ts @@ -0,0 +1,14 @@ +import type { GitHubOwnerRepo } from '../../../../shared/types' + +export type GitHubMarkdownComposerProps = { + value: string + onChange: (value: string) => void + placeholder: string + minHeightClassName?: string + className?: string + disabled?: boolean + autoFocus?: boolean + onSubmitShortcut?: () => void + layout?: 'stacked' | 'tabbed' + previewGithubRepo?: GitHubOwnerRepo | null +} diff --git a/src/renderer/src/components/mobile/use-mobile-page-escape.test.tsx b/src/renderer/src/components/mobile/use-mobile-page-escape.test.tsx new file mode 100644 index 00000000000..154bfa64556 --- /dev/null +++ b/src/renderer/src/components/mobile/use-mobile-page-escape.test.tsx @@ -0,0 +1,40 @@ +// @vitest-environment happy-dom + +import { cleanup, renderHook } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { useMobilePageEscape } from './use-mobile-page-escape' + +afterEach(() => { + cleanup() + document.body.replaceChildren() +}) + +describe('useMobilePageEscape', () => { + it('leaves a composing input focused for marked Escape', () => { + const onClose = vi.fn() + const input = document.createElement('input') + document.body.append(input) + renderHook(() => useMobilePageEscape(onClose)) + input.focus() + + input.dispatchEvent( + new KeyboardEvent('keydown', { key: 'Escape', bubbles: true, isComposing: true }) + ) + + expect(document.activeElement).toBe(input) + expect(onClose).not.toHaveBeenCalled() + }) + + it('still blurs a focused input for ordinary Escape', () => { + const onClose = vi.fn() + const input = document.createElement('input') + document.body.append(input) + renderHook(() => useMobilePageEscape(onClose)) + input.focus() + + input.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true })) + + expect(document.activeElement).not.toBe(input) + expect(onClose).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/components/mobile/use-mobile-page-escape.ts b/src/renderer/src/components/mobile/use-mobile-page-escape.ts index 302175cd685..512b55965e2 100644 --- a/src/renderer/src/components/mobile/use-mobile-page-escape.ts +++ b/src/renderer/src/components/mobile/use-mobile-page-escape.ts @@ -1,4 +1,5 @@ import { useEffect } from 'react' +import { isImeCompositionKeyDown } from '@/lib/ime-composition-keyboard-event' function isEditableElement(target: EventTarget | null): target is HTMLElement { return ( @@ -13,6 +14,9 @@ export function useMobilePageEscape(onClose: () => void): void { // Why: mirror Automations/Tasks — Esc first exits field focus, then closes the page. useEffect(() => { function onKeyDown(event: KeyboardEvent): void { + if (isImeCompositionKeyDown(event)) { + return + } if (event.key !== 'Escape' || event.defaultPrevented) { return } diff --git a/src/renderer/src/components/sidebar/WorkspaceKanbanSearchField.tsx b/src/renderer/src/components/sidebar/WorkspaceKanbanSearchField.tsx index 0518d889603..e8f1c66341c 100644 --- a/src/renderer/src/components/sidebar/WorkspaceKanbanSearchField.tsx +++ b/src/renderer/src/components/sidebar/WorkspaceKanbanSearchField.tsx @@ -120,7 +120,7 @@ export default function WorkspaceKanbanSearchField({ // this field is the only handler — it must cover both outcomes. // Mid-composition Escape belongs to the IME, which cancels the // in-progress reading rather than the query behind it. - if (event.key !== 'Escape' || isImeCompositionKeyDown(event)) { + if (isImeCompositionKeyDown(event) || event.key !== 'Escape') { return } event.preventDefault() diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers-ime.test.tsx b/src/renderer/src/components/terminal-pane/keyboard-handlers-ime.test.tsx index c684c8a70a3..57b6804fb60 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers-ime.test.tsx +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers-ime.test.tsx @@ -6,6 +6,7 @@ import type { PtyTransport } from './pty-transport' import { useTerminalKeyboardShortcuts } from './keyboard-handlers' import { installTerminalImeCompositionRoute, + XTERM_COMPOSITION_SESSION_END_EVENT, XTERM_COMPOSITION_SESSION_START_EVENT } from './terminal-ime-composition-route' @@ -31,6 +32,7 @@ function keyboardEvent( function createHarness(options: { paneCount?: number } = {}): { deps: KeyboardHandlersDeps editable: HTMLInputElement + endComposition: () => void sendInput: ReturnType startComposition: () => void terminalInput: HTMLTextAreaElement @@ -103,6 +105,13 @@ function createHarness(options: { paneCount?: number } = {}): { return { deps, editable, + endComposition: () => { + terminalElement.dispatchEvent( + new CustomEvent(XTERM_COMPOSITION_SESSION_END_EVENT, { + detail: { id: 1, data: '한' } + }) + ) + }, sendInput, terminalInput, startComposition: () => { @@ -326,6 +335,106 @@ describe('Windows IME keyboard ownership', () => { hook.unmount() harness.dispose() }) + + it('clears held Shift when a consumed keyup reports that the modifier is no longer down', () => { + const harness = createHarness() + const hook = renderHook(() => useTerminalKeyboardShortcuts(harness.deps)) + harness.terminalInput.dispatchEvent( + keyboardEvent('keydown', { + key: 'Shift', + code: 'ShiftLeft', + keyCode: 16, + timeStamp: 1, + shiftKey: true + }) + ) + harness.startComposition() + harness.terminalInput.dispatchEvent( + keyboardEvent('keyup', { + key: 'Process', + code: 'ShiftLeft', + keyCode: 229, + timeStamp: 10, + isComposing: true + }) + ) + harness.terminalInput.dispatchEvent( + keyboardEvent('keyup', { + key: 'Enter', + code: 'Enter', + keyCode: 13, + timeStamp: 20, + isComposing: true + }) + ) + + harness.endComposition() + vi.runAllTimers() + + expect(harness.sendInput).not.toHaveBeenCalled() + hook.unmount() + harness.dispose() + }) + + it('uses held Shift for a bare balancing Enter keyup during composition', () => { + const harness = createHarness() + const hook = renderHook(() => useTerminalKeyboardShortcuts(harness.deps)) + harness.terminalInput.dispatchEvent( + keyboardEvent('keydown', { + key: 'Shift', + code: 'ShiftLeft', + keyCode: 16, + timeStamp: 1, + shiftKey: true + }) + ) + harness.startComposition() + const keyup = keyboardEvent('keyup', { + key: 'Enter', + code: 'Enter', + keyCode: 13, + timeStamp: 20, + isComposing: true + }) + + harness.terminalInput.dispatchEvent(keyup) + + expect(keyup.defaultPrevented).toBe(true) + expect(harness.sendInput).not.toHaveBeenCalled() + harness.endComposition() + vi.runAllTimers() + expect(harness.sendInput).toHaveBeenCalledOnce() + expect(harness.sendInput).toHaveBeenCalledWith('\x1b\r') + hook.unmount() + harness.dispose() + }) + + it.each([{ code: 'Enter' }, { code: 'NumpadEnter' }])( + 'still defers a composed Shift+Enter reported as Process with code $code', + ({ code }) => { + const harness = createHarness() + const hook = renderHook(() => useTerminalKeyboardShortcuts(harness.deps)) + harness.startComposition() + const processEnter = keyboardEvent('keydown', { + key: 'Process', + code, + keyCode: 229, + timeStamp: 10, + isComposing: true, + shiftKey: true + }) + + harness.terminalInput.dispatchEvent(processEnter) + + expect(processEnter.defaultPrevented).toBe(true) + expect(harness.sendInput).not.toHaveBeenCalled() + harness.endComposition() + vi.runAllTimers() + expect(harness.sendInput).toHaveBeenCalledWith('\x1b\r') + hook.unmount() + harness.dispose() + } + ) }) describe('terminal shortcuts yield to a composition', () => { diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts index c07334c008d..68bc67de7a5 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts @@ -281,6 +281,25 @@ export function useTerminalKeyboardShortcuts({ const nativeOnlyShortcutTracker = createTerminalNativeOnlyShortcutTracker() const deferredNewlineSender = createTerminalImeDeferredNewlineSender() const modifiedEnterChordOwner = createTerminalImeModifiedEnterChordOwner() + const reconcileHeldImeEnterModifiers = ( + event: KeyboardEvent, + preserveModifierLostRedispatch = false + ): void => { + if (heldImeEnterModifiers.size === 0) { + return + } + for (const [kind, pressed] of [ + ['shift', event.shiftKey], + ['ctrl', event.ctrlKey] + ] as const) { + const belongsToActiveChord = + preserveModifierLostRedispatch && + modifiedEnterChordOwner.absorb({ kind, code: event.code, timeStamp: event.timeStamp }) + if (!pressed && !belongsToActiveChord && heldImeEnterModifiers.delete(kind)) { + modifiedEnterChordOwner.release({ kind, code: event.code, timeStamp: event.timeStamp }) + } + } + } const getHeldImeEnterModifier = () => heldImeEnterModifiers.size === 1 ? (heldImeEnterModifiers.values().next().value ?? null) @@ -297,6 +316,7 @@ export function useTerminalKeyboardShortcuts({ return kind ? { kind, code: event.code, timeStamp: event.timeStamp } : null } const onModifierDown = (e: KeyboardEvent): void => { + reconcileHeldImeEnterModifiers(e, e.key === 'Enter' && e.keyCode === 13) if (e.key === 'Alt') { optionKeyLocation = e.location } @@ -452,6 +472,7 @@ export function useTerminalKeyboardShortcuts({ deferredNewlineSender.absorbRedispatchedEnter(e)) ) { // Chromium can drop the modifier when re-dispatching the committing Enter. + reconcileHeldImeEnterModifiers(e) e.preventDefault() e.stopImmediatePropagation() return @@ -749,6 +770,9 @@ export function useTerminalKeyboardShortcuts({ } const onKeyUp = (e: KeyboardEvent): void => { + if (!isTerminalImeEnterKeyUp(e)) { + reconcileHeldImeEnterModifiers(e) + } if (e.key === 'Alt') { optionKeyLocation = 0 } diff --git a/src/renderer/src/components/terminal-pane/terminal-ime-deferred-newline.test.ts b/src/renderer/src/components/terminal-pane/terminal-ime-deferred-newline.test.ts index d049b5224e7..5193f1123db 100644 --- a/src/renderer/src/components/terminal-pane/terminal-ime-deferred-newline.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-ime-deferred-newline.test.ts @@ -475,6 +475,7 @@ describe('isTerminalImeProcessEnter', () => { const event = (overrides: Partial = {}) => ({ key: 'Process', + code: 'Enter', keyCode: 229, metaKey: false, ctrlKey: false, @@ -483,19 +484,23 @@ describe('isTerminalImeProcessEnter', () => { ...overrides }) as KeyboardEvent - it.each([{ shiftKey: true }, { shiftKey: false, ctrlKey: true }])( - 'recognizes a Windows IME modifier Enter reported as Process', - (modifiers) => { - expect(isTerminalImeProcessEnter(event(modifiers))).toBe(true) - } - ) + it.each([ + { shiftKey: true }, + { shiftKey: false, ctrlKey: true }, + { code: 'NumpadEnter' }, + { code: 'NumpadEnter', shiftKey: false, ctrlKey: true } + ])('recognizes a Windows IME modifier Enter reported as Process', (modifiers) => { + expect(isTerminalImeProcessEnter(event(modifiers))).toBe(true) + }) it.each([ { key: 'Enter' }, { keyCode: 13 }, { shiftKey: false }, { ctrlKey: true }, - { altKey: true } + { altKey: true }, + { code: 'KeyQ' }, + { code: 'KeyI', shiftKey: false, ctrlKey: true } ])('rejects a non-IME or ambiguous Process key', (override) => { expect(isTerminalImeProcessEnter(event(override))).toBe(false) }) diff --git a/src/renderer/src/components/terminal-pane/terminal-ime-deferred-newline.ts b/src/renderer/src/components/terminal-pane/terminal-ime-deferred-newline.ts index 3a7c7745198..e77794393c4 100644 --- a/src/renderer/src/components/terminal-pane/terminal-ime-deferred-newline.ts +++ b/src/renderer/src/components/terminal-pane/terminal-ime-deferred-newline.ts @@ -165,11 +165,16 @@ export function getTerminalImeModifiedEnterKind( } export function isTerminalImeProcessEnter( - event: Pick + event: Pick< + KeyboardEvent, + 'key' | 'code' | 'keyCode' | 'metaKey' | 'ctrlKey' | 'altKey' | 'shiftKey' + > ): boolean { return ( event.key === 'Process' && event.keyCode === 229 && + // Windows reports every IME-consumed key as Process/229; only code proves physical Enter. + (event.code === 'Enter' || event.code === 'NumpadEnter') && getTerminalImeModifiedEnterKind(event) !== null ) } diff --git a/src/renderer/src/components/terminal-pane/terminal-shortcut-composition-guard.test.ts b/src/renderer/src/components/terminal-pane/terminal-shortcut-composition-guard.test.ts index 1af3a015095..ee9992e9fe8 100644 --- a/src/renderer/src/components/terminal-pane/terminal-shortcut-composition-guard.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-shortcut-composition-guard.test.ts @@ -33,6 +33,10 @@ describe('terminalShortcutIsOwnedByIme', () => { // already handles composition, rather than merely declining to fix one. it.each([ ['the Process shape', { key: 'Process', code: 'Enter', keyCode: 229, isComposing: true }], + [ + 'the numpad Process shape', + { key: 'Process', code: 'NumpadEnter', keyCode: 229, isComposing: true } + ], ['the marked real shape', { key: 'Enter', code: 'Enter', keyCode: 13, isComposing: true }] ])('releases Enter in %s for a caller that defers it to the commit', (_shape, init) => { expect( diff --git a/src/renderer/src/components/terminal-pane/terminal-shortcut-composition-guard.ts b/src/renderer/src/components/terminal-pane/terminal-shortcut-composition-guard.ts index 60b0e179e5b..18b0b7b3c0d 100644 --- a/src/renderer/src/components/terminal-pane/terminal-shortcut-composition-guard.ts +++ b/src/renderer/src/components/terminal-pane/terminal-shortcut-composition-guard.ts @@ -42,5 +42,6 @@ export function terminalShortcutIsOwnedByIme( if (action?.type === 'switchInputSource') { return false } - return !(enterIsDeferredToCommit && event.code === 'Enter' && action?.type === 'sendInput') + const isPhysicalEnter = event.code === 'Enter' || event.code === 'NumpadEnter' + return !(enterIsDeferredToCommit && isPhysicalEnter && action?.type === 'sendInput') } diff --git a/src/renderer/src/components/ui/command.tsx b/src/renderer/src/components/ui/command.tsx index cbc9c0cf147..32f23bceea2 100644 --- a/src/renderer/src/components/ui/command.tsx +++ b/src/renderer/src/components/ui/command.tsx @@ -6,6 +6,7 @@ import { SearchIcon } from 'lucide-react' import { Dialog as DialogPrimitive } from 'radix-ui' import { cn } from '@/lib/utils' +import { useImeAwareEscapeKeyDown } from './use-ime-aware-escape-key-down' function Command({ className, ...props }: React.ComponentProps) { return ( @@ -42,6 +43,7 @@ function CommandDialog({ commandProps?: React.ComponentProps }) { const { className: commandClassName, ...commandRootProps } = commandProps ?? {} + const handleEscapeKeyDown = useImeAwareEscapeKeyDown() return ( @@ -67,6 +69,7 @@ function CommandDialog({ )} onOpenAutoFocus={onOpenAutoFocus} onCloseAutoFocus={onCloseAutoFocus} + onEscapeKeyDown={handleEscapeKeyDown} > {title} diff --git a/src/renderer/src/components/ui/context-menu.tsx b/src/renderer/src/components/ui/context-menu.tsx index 2b0423e0cd7..d9d4c39cf9c 100644 --- a/src/renderer/src/components/ui/context-menu.tsx +++ b/src/renderer/src/components/ui/context-menu.tsx @@ -3,6 +3,7 @@ import { CheckIcon, ChevronRightIcon, CircleIcon } from 'lucide-react' import { ContextMenu as ContextMenuPrimitive } from 'radix-ui' import { cn } from '@/lib/utils' +import { useImeAwareEscapeKeyDown } from './use-ime-aware-escape-key-down' function ContextMenu({ ...props }: React.ComponentProps) { return @@ -59,8 +60,11 @@ function ContextMenuSubTrigger({ function ContextMenuSubContent({ className, style, + onEscapeKeyDown, ...props }: React.ComponentProps) { + const handleEscapeKeyDown = useImeAwareEscapeKeyDown(onEscapeKeyDown) + return ( ) @@ -81,8 +86,11 @@ function ContextMenuSubContent({ function ContextMenuContent({ className, style, + onEscapeKeyDown, ...props }: React.ComponentProps) { + const handleEscapeKeyDown = useImeAwareEscapeKeyDown(onEscapeKeyDown) + return ( ) diff --git a/src/renderer/src/components/ui/dialog.tsx b/src/renderer/src/components/ui/dialog.tsx index 8b1badf9ab3..45b8df68930 100644 --- a/src/renderer/src/components/ui/dialog.tsx +++ b/src/renderer/src/components/ui/dialog.tsx @@ -7,6 +7,7 @@ import { Dialog as DialogPrimitive } from 'radix-ui' import { cn } from '@/lib/utils' import { Button } from '@/components/ui/button' import { translate } from '@/i18n/i18n' +import { useImeAwareEscapeKeyDown } from './use-ime-aware-escape-key-down' function Dialog({ ...props }: React.ComponentProps) { return @@ -48,11 +49,14 @@ function DialogContent({ children, overlayClassName, showCloseButton = true, + onEscapeKeyDown, ...props }: React.ComponentProps & { overlayClassName?: string showCloseButton?: boolean }) { + const handleEscapeKeyDown = useImeAwareEscapeKeyDown(onEscapeKeyDown) + return ( @@ -68,6 +72,7 @@ function DialogContent({ className )} {...props} + onEscapeKeyDown={handleEscapeKeyDown} > {children} {showCloseButton && ( diff --git a/src/renderer/src/components/ui/dismissable-layer-ime-escape-invariant.test.ts b/src/renderer/src/components/ui/dismissable-layer-ime-escape-invariant.test.ts new file mode 100644 index 00000000000..f66586b624d --- /dev/null +++ b/src/renderer/src/components/ui/dismissable-layer-ime-escape-invariant.test.ts @@ -0,0 +1,50 @@ +import { readdirSync, readFileSync } from 'node:fs' +import { basename, join } from 'node:path' +import { describe, expect, it } from 'vitest' + +const UI_DIR = 'src/renderer/src/components/ui' +const NON_DISMISSABLE_CONTENT = new Set(['accordion.tsx', 'collapsible.tsx', 'tabs.tsx']) +const MUST_GUARD = [ + 'command.tsx', + 'context-menu.tsx', + 'dialog.tsx', + 'dropdown-menu.tsx', + 'hover-card.tsx', + 'popover.tsx', + 'select.tsx', + 'sheet.tsx', + 'tooltip.tsx' +] + +function count(source: string, pattern: RegExp): number { + return source.match(pattern)?.length ?? 0 +} + +describe('Radix dismissable-layer wrappers guard IME Escape', () => { + it('guards every dismissable Primitive.Content exported from the UI layer', () => { + const guarded: string[] = [] + const offenders: string[] = [] + + for (const entry of readdirSync(UI_DIR, { withFileTypes: true })) { + if (!entry.isFile() || !entry.name.endsWith('.tsx') || entry.name.includes('.test.')) { + continue + } + const file = join(UI_DIR, entry.name) + const source = readFileSync(file, 'utf8') + const contentCount = count(source, /<\w+Primitive\.(?:Sub)?Content\b/g) + if (contentCount === 0 || NON_DISMISSABLE_CONTENT.has(entry.name)) { + continue + } + const hookCount = count(source, /const handleEscapeKeyDown = useImeAwareEscapeKeyDown\(/g) + const propCount = count(source, /onEscapeKeyDown=\{handleEscapeKeyDown\}/g) + if (hookCount !== contentCount || propCount !== contentCount) { + offenders.push(basename(file)) + } else { + guarded.push(basename(file)) + } + } + + expect(offenders).toEqual([]) + expect(guarded).toEqual(expect.arrayContaining(MUST_GUARD)) + }) +}) diff --git a/src/renderer/src/components/ui/dismissable-layer-ime-escape.test.tsx b/src/renderer/src/components/ui/dismissable-layer-ime-escape.test.tsx new file mode 100644 index 00000000000..8d2fa2c2244 --- /dev/null +++ b/src/renderer/src/components/ui/dismissable-layer-ime-escape.test.tsx @@ -0,0 +1,130 @@ +// @vitest-environment happy-dom + +import { cleanup, fireEvent, render, waitFor } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { useState } from 'react' +import { Dialog, DialogContent, DialogTitle } from './dialog' +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuSub, + DropdownMenuSubContent, + DropdownMenuSubTrigger, + DropdownMenuTrigger +} from './dropdown-menu' +import { Popover, PopoverContent, PopoverTrigger } from './popover' +import { Sheet, SheetContent, SheetTitle } from './sheet' + +type SurfaceProps = { onEscapeKeyDown: (event: KeyboardEvent) => void } + +function ControlledPopover({ onEscapeKeyDown }: SurfaceProps): React.JSX.Element { + const [open, setOpen] = useState(true) + return ( + + Open + + + + + ) +} + +function ControlledDialog({ onEscapeKeyDown }: SurfaceProps): React.JSX.Element { + const [open, setOpen] = useState(true) + return ( + + + Draft dialog + + + + ) +} + +function ControlledSheet({ onEscapeKeyDown }: SurfaceProps): React.JSX.Element { + const [open, setOpen] = useState(true) + return ( + + + Draft sheet + + + + ) +} + +function ControlledDropdownSub({ onEscapeKeyDown }: SurfaceProps): React.JSX.Element { + const [open, setOpen] = useState(true) + return ( + + Open + + + More + + + + + + + ) +} + +const SURFACES = [ + ['PopoverContent', ControlledPopover], + ['DialogContent', ControlledDialog], + ['SheetContent', ControlledSheet] +] as const + +afterEach(cleanup) + +describe('Radix dismissable-layer IME ownership', () => { + it.each(SURFACES)('%s keeps its draft open for a marked Escape', (_name, Surface) => { + const onEscapeKeyDown = vi.fn() + const view = render() + const input = view.getByRole('textbox', { name: 'Draft' }) + + fireEvent.keyDown(input, { key: 'Escape', keyCode: 27, isComposing: true }) + + expect(view.queryByRole('textbox', { name: 'Draft' })).not.toBeNull() + expect(onEscapeKeyDown).not.toHaveBeenCalled() + }) + + it.each(SURFACES)('%s still dismisses for an ordinary Escape', (_name, Surface) => { + const onEscapeKeyDown = vi.fn() + const view = render() + const input = view.getByRole('textbox', { name: 'Draft' }) + + fireEvent.keyDown(input, { key: 'Escape', keyCode: 27 }) + + expect(view.queryByRole('textbox', { name: 'Draft' })).toBeNull() + expect(onEscapeKeyDown).toHaveBeenCalledOnce() + }) + + async function openDropdownSub(onEscapeKeyDown: (event: KeyboardEvent) => void) { + const view = render() + fireEvent.keyDown(view.getByRole('menuitem', { name: 'More' }), { key: 'ArrowRight' }) + const input = await view.findByRole('textbox', { name: 'Draft' }) + return { input, view } + } + + it('keeps DropdownMenuSubContent open for a marked Escape', async () => { + const onEscapeKeyDown = vi.fn() + const { input, view } = await openDropdownSub(onEscapeKeyDown) + + fireEvent.keyDown(input, { key: 'Escape', keyCode: 27, isComposing: true }) + + expect(view.queryByRole('textbox', { name: 'Draft' })).not.toBeNull() + expect(onEscapeKeyDown).not.toHaveBeenCalled() + }) + + it('still dismisses DropdownMenuSubContent for an ordinary Escape', async () => { + const onEscapeKeyDown = vi.fn() + const { input, view } = await openDropdownSub(onEscapeKeyDown) + + fireEvent.keyDown(input, { key: 'Escape', keyCode: 27 }) + + await waitFor(() => expect(view.queryByRole('textbox', { name: 'Draft' })).toBeNull()) + expect(onEscapeKeyDown).toHaveBeenCalledOnce() + }) +}) diff --git a/src/renderer/src/components/ui/dropdown-menu.tsx b/src/renderer/src/components/ui/dropdown-menu.tsx index d7fa2787671..aa6eb650adf 100644 --- a/src/renderer/src/components/ui/dropdown-menu.tsx +++ b/src/renderer/src/components/ui/dropdown-menu.tsx @@ -3,6 +3,7 @@ import { CheckIcon, ChevronRightIcon, CircleIcon } from 'lucide-react' import { DropdownMenu as DropdownMenuPrimitive } from 'radix-ui' import { cn } from '@/lib/utils' +import { useImeAwareEscapeKeyDown } from './use-ime-aware-escape-key-down' function DropdownMenu({ ...props }: React.ComponentProps) { return @@ -24,8 +25,11 @@ function DropdownMenuContent({ className, sideOffset = 4, style, + onEscapeKeyDown, ...props }: React.ComponentProps) { + const handleEscapeKeyDown = useImeAwareEscapeKeyDown(onEscapeKeyDown) + return ( ) @@ -205,8 +210,11 @@ function DropdownMenuSubTrigger({ function DropdownMenuSubContent({ className, style, + onEscapeKeyDown, ...props }: React.ComponentProps) { + const handleEscapeKeyDown = useImeAwareEscapeKeyDown(onEscapeKeyDown) + return ( ) diff --git a/src/renderer/src/components/ui/hover-card.tsx b/src/renderer/src/components/ui/hover-card.tsx index c9ee04f500d..85fc2e909a0 100644 --- a/src/renderer/src/components/ui/hover-card.tsx +++ b/src/renderer/src/components/ui/hover-card.tsx @@ -4,6 +4,7 @@ import * as React from 'react' import { HoverCard as HoverCardPrimitive } from 'radix-ui' import { cn } from '@/lib/utils' +import { useImeAwareEscapeKeyDown } from './use-ime-aware-escape-key-down' function HoverCard({ ...props }: React.ComponentProps) { return @@ -17,8 +18,11 @@ function HoverCardContent({ className, align = 'center', sideOffset = 4, + onEscapeKeyDown, ...props }: React.ComponentProps) { + const handleEscapeKeyDown = useImeAwareEscapeKeyDown(onEscapeKeyDown) + return ( ) diff --git a/src/renderer/src/components/ui/popover.tsx b/src/renderer/src/components/ui/popover.tsx index 5523f5021ee..271ef7690e7 100644 --- a/src/renderer/src/components/ui/popover.tsx +++ b/src/renderer/src/components/ui/popover.tsx @@ -5,6 +5,7 @@ import { Popover as PopoverPrimitive } from 'radix-ui' import { cn } from '@/lib/utils' import { updatePopoverContentRef } from './popover-content-ref' +import { useImeAwareEscapeKeyDown } from './use-ime-aware-escape-key-down' function Popover(props: React.ComponentProps) { return @@ -25,6 +26,7 @@ function PopoverContent({ portalContainer, style, onWheel, + onEscapeKeyDown, ref: forwardedRef, ...props }: React.ComponentProps & { @@ -87,6 +89,8 @@ function PopoverContent({ [onWheel] ) + const handleEscapeKeyDown = useImeAwareEscapeKeyDown(onEscapeKeyDown) + return ( diff --git a/src/renderer/src/components/ui/select.tsx b/src/renderer/src/components/ui/select.tsx index d25c887b328..ee96cd320ff 100644 --- a/src/renderer/src/components/ui/select.tsx +++ b/src/renderer/src/components/ui/select.tsx @@ -5,6 +5,7 @@ import { CheckIcon, ChevronDownIcon, ChevronUpIcon } from 'lucide-react' import { Select as SelectPrimitive } from 'radix-ui' import { cn } from '@/lib/utils' +import { useImeAwareEscapeKeyDown } from './use-ime-aware-escape-key-down' function Select({ ...props }: React.ComponentProps) { return @@ -50,10 +51,13 @@ function SelectContent({ position = 'item-aligned', align = 'center', portalContainer, + onEscapeKeyDown, ...props }: React.ComponentProps & { portalContainer?: HTMLElement | null }) { + const handleEscapeKeyDown = useImeAwareEscapeKeyDown(onEscapeKeyDown) + return ( ) { return @@ -79,6 +80,7 @@ function SheetContent({ overlayClassName, overlayStyle, style, + onEscapeKeyDown, ...props }: React.ComponentProps & VariantProps & { @@ -86,6 +88,8 @@ function SheetContent({ overlayClassName?: string overlayStyle?: React.CSSProperties }) { + const handleEscapeKeyDown = useImeAwareEscapeKeyDown(onEscapeKeyDown) + return ( @@ -96,6 +100,7 @@ function SheetContent({ // document root and its header overlaps the titlebar drag strip. style={{ ...style, WebkitAppRegion: 'no-drag' } as React.CSSProperties} {...props} + onEscapeKeyDown={handleEscapeKeyDown} > {children} {showCloseButton && ( diff --git a/src/renderer/src/components/ui/tooltip.tsx b/src/renderer/src/components/ui/tooltip.tsx index bd45dec10a8..eb84d6744aa 100644 --- a/src/renderer/src/components/ui/tooltip.tsx +++ b/src/renderer/src/components/ui/tooltip.tsx @@ -2,6 +2,7 @@ import * as React from 'react' import { Tooltip as TooltipPrimitive } from 'radix-ui' import { cn } from '@/lib/utils' +import { useImeAwareEscapeKeyDown } from './use-ime-aware-escape-key-down' function TooltipProvider({ delayDuration = 0, @@ -32,8 +33,11 @@ function TooltipContent({ className, sideOffset = 0, children, + onEscapeKeyDown, ...props }: React.ComponentProps) { + const handleEscapeKeyDown = useImeAwareEscapeKeyDown(onEscapeKeyDown) + return ( {children} diff --git a/src/renderer/src/components/ui/use-ime-aware-escape-key-down.ts b/src/renderer/src/components/ui/use-ime-aware-escape-key-down.ts new file mode 100644 index 00000000000..dd9a9c1c326 --- /dev/null +++ b/src/renderer/src/components/ui/use-ime-aware-escape-key-down.ts @@ -0,0 +1,21 @@ +import * as React from 'react' + +import { isImeCompositionKeyDown } from '@/lib/ime-composition-keyboard-event' + +type EscapeKeyDownHandler = (event: KeyboardEvent) => void + +export function useImeAwareEscapeKeyDown( + onEscapeKeyDown?: EscapeKeyDownHandler +): EscapeKeyDownHandler { + return React.useCallback( + (event) => { + // Radix observes Escape during document capture, before element keydown handlers. + if (isImeCompositionKeyDown(event)) { + event.preventDefault() + return + } + onEscapeKeyDown?.(event) + }, + [onEscapeKeyDown] + ) +} diff --git a/src/renderer/src/lib/ime-escape-guard-ordering.test.ts b/src/renderer/src/lib/ime-escape-guard-ordering.test.ts index 2d82d9a7988..2259e784263 100644 --- a/src/renderer/src/lib/ime-escape-guard-ordering.test.ts +++ b/src/renderer/src/lib/ime-escape-guard-ordering.test.ts @@ -9,14 +9,28 @@ import { describe, expect, it } from 'vitest' // // Ordering is a property of the source, not of any one component's behaviour, so it is pinned // here once rather than re-tested in every composer that happens to handle Escape. +// It enforces textual guard-first order, including equivalent `||` bails, so the heuristic has +// one auditable source shape instead of approximating short-circuit control flow. const ROOTS = ['src/renderer/src', 'src/main', 'src/shared'] const GUARD = 'isImeCompositionKeyDown' -const ESCAPE = "=== 'Escape'" +const ESCAPE_COMPARISON = /[!=]==\s*['"]Escape['"]/ +const MUST_GUARD_EVERY_ESCAPE_HANDLER = new Set([ + 'src/renderer/src/components/NewWorkspaceComposerModal.tsx', + 'src/renderer/src/components/browser-pane/BrowserAddressBar.tsx', + 'src/renderer/src/components/editor/EditorPanelHeaderPath.tsx', + 'src/renderer/src/components/editor/RichMarkdownLinkBubble.tsx', + 'src/renderer/src/components/github/GitHubMarkdownComposer.tsx', + 'src/renderer/src/components/github-project/ProjectCell.tsx', + 'src/renderer/src/components/mobile/use-mobile-page-escape.ts' +]) const MUST_SCAN = [ + ...MUST_GUARD_EVERY_ESCAPE_HANDLER, 'src/renderer/src/components/native-chat/use-native-chat-composer-keydown.ts', 'src/renderer/src/components/new-workspace/SmartWorkspaceNameField.tsx', - 'src/renderer/src/components/settings/ShortcutRecorderButton.tsx' + 'src/renderer/src/components/settings/ShortcutRecorderButton.tsx', + 'src/renderer/src/components/sidebar/WorkspaceKanbanSearchField.tsx', + 'src/renderer/src/lib/task-page-window-shortcut-policy.ts' ] function sourceFiles(dir: string): string[] { @@ -65,10 +79,10 @@ function bodyBrace(source: string, from: number): number { return -1 } -// Returns the body of every keydown handler in `source`, brace-matched from its opening `{`. -function keydownHandlerBodies(source: string): string[] { +// Returns every function body in `source`, including inline JSX keydown handlers. +function functionBodies(source: string): string[] { const bodies: string[] = [] - const starts = /onKeyDown(?:Capture)?=\{|(?:function|const)\s+\w*[Kk]ey[Dd]own\w*\s*[=(]/g + const starts = /(?:on|handle)KeyDown(?:Capture)?\s*[:=]|(?:function|const)\s+\w+\s*[=(]/g for (let match = starts.exec(source); match !== null; match = starts.exec(source)) { const open = bodyBrace(source, match.index + match[0].length) if (open === -1) { @@ -91,25 +105,30 @@ function keydownHandlerBodies(source: string): string[] { } describe('composition guards sit above the Escape branch', () => { - const offenders: string[] = [] + const offenders = new Set() for (const root of ROOTS) { for (const file of sourceFiles(root)) { const source = readFileSync(file, 'utf8') - if (!source.includes(GUARD) || !source.includes(ESCAPE)) { + if (!source.includes(GUARD) || !ESCAPE_COMPARISON.test(source)) { continue } - for (const body of keydownHandlerBodies(source)) { + for (const body of functionBodies(source)) { const guardAt = body.indexOf(GUARD) - const escapeAt = body.indexOf(ESCAPE) - if (guardAt !== -1 && escapeAt !== -1 && escapeAt < guardAt) { - offenders.push(file) + const escapeAt = body.search(ESCAPE_COMPARISON) + const relativeFile = file.split(sep).join('/') + const requiresGuard = MUST_GUARD_EVERY_ESCAPE_HANDLER.has(relativeFile) + if ( + escapeAt !== -1 && + ((requiresGuard && guardAt === -1) || (guardAt !== -1 && escapeAt < guardAt)) + ) { + offenders.add(file) } } } } it('finds no keydown handler that dispatches Escape before consulting the guard', () => { - expect(offenders).toEqual([]) + expect([...offenders]).toEqual([]) }) // A scanner that parses nothing reports zero offenders forever, so the coverage it claims is @@ -120,11 +139,10 @@ describe('composition guards sit above the Escape branch', () => { const scanned = ROOTS.flatMap(sourceFiles) .filter((file) => { const source = readFileSync(file, 'utf8') - return source.includes(GUARD) && keydownHandlerBodies(source).some((b) => b.includes(GUARD)) + return source.includes(GUARD) && functionBodies(source).some((body) => body.includes(GUARD)) }) .map((file) => file.split(sep).join('/')) expect(scanned).toEqual(expect.arrayContaining(MUST_SCAN)) - expect(scanned.length).toBeGreaterThan(8) }) })