diff --git a/.gitattributes b/.gitattributes index b37363335a4..e975ca4959f 100644 --- a/.gitattributes +++ b/.gitattributes @@ -16,4 +16,7 @@ # its minified bundle lines also make a diff nobody can read. Review the hand-written # source patch under xterm-src/ instead. /config/patches/@xterm__xterm@*.patch -diff -text -/config/patches/xterm-src/*.patch text eol=lf +# A unified-diff context line is prefixed with a space, so a blank source line is a line +# holding exactly one space. Stripping it corrupts the patch, so exempt these from the +# whitespace checks rather than leave `git diff --check` reporting damage as a defect. +/config/patches/xterm-src/*.patch text eol=lf -whitespace diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index eca968e1b05..4c60ab11da6 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -25,6 +25,12 @@ import { SYNC_FIT_PANES_EVENT, TOGGLE_TERMINAL_PANE_EXPAND_EVENT } from '@/const import { syncZoomCSSVar } from '@/lib/ui-zoom' import { resolveLeftSidebarStyleVariables } from '@/lib/left-sidebar-appearance' import { canShowRightSidebarForView } from '@/lib/right-sidebar-visibility' +import { isImeCompositionKeyDown } from '@/lib/ime-composition-keyboard-event' +import { + toDoubleTapShortcutDispatchInput, + toShortcutDispatchInput, + type ShortcutDispatchInput +} from '@/lib/app-shortcut-dispatch-input' import { isPairedWebClientWindow, shouldRenderDesktopWindowChrome @@ -180,8 +186,7 @@ import { keybindingMatchesAction, type KeybindingActionId, type KeybindingContext, - type KeybindingMatchOptions, - type PhysicalModifierToken + type KeybindingMatchOptions } from '../../shared/keybindings' import { PLUGIN_COMMAND_ALIAS_ACTION_IDS } from '../../shared/plugins/plugin-command-actions' import { registerAppCommandDispatcher } from '@/lib/app-command-dispatch' @@ -246,20 +251,6 @@ function getKeybindingContext(target: EventTarget | null): KeybindingContext { : 'app' } -// Abstraction over a real KeyboardEvent and a synthetic double-tap gesture so one dispatch path serves both; KeybindingInput-compatible. -type ShortcutDispatchInput = { - key?: string - code?: string - altKey?: boolean - metaKey?: boolean - ctrlKey?: boolean - shiftKey?: boolean - doubleTapModifier?: PhysicalModifierToken - target: EventTarget | null - defaultPrevented: boolean - preventDefault: () => void -} - // Why: Windows and Linux both remove the native title bar, so we render our own min/max/close buttons (Fluent/Win11-style SVGs). function WindowControls(): React.JSX.Element { const [maximized, setMaximized] = useState(false) @@ -1920,6 +1911,11 @@ function App(): React.JSX.Element { } const onKeyDown = (e: KeyboardEvent): void => { + // Ahead of the detector: a composing chord must not arm a double-tap either, and the + // synthetic input a detection produces carries no markers to be refused by later. + if (isImeCompositionKeyDown(e)) { + return + } const detected = doubleTapDetector.process( toModifierDoubleTapEvent({ type: 'keyDown', @@ -1936,30 +1932,20 @@ function App(): React.JSX.Element { if (e.repeat) { return } - if (detected) { - // Synthetic input: no key/modifier flags, so only DoubleTap bindings match. - dispatchShortcutInput({ - doubleTapModifier: detected.modifier, - target: e.target, - defaultPrevented: e.defaultPrevented, - preventDefault: () => e.preventDefault() - }) - return + const input = detected + ? toDoubleTapShortcutDispatchInput(e, detected.modifier) + : toShortcutDispatchInput(e) + if (input) { + dispatchShortcutInput(input) } - dispatchShortcutInput({ - key: e.key, - code: e.code, - altKey: e.altKey, - metaKey: e.metaKey, - ctrlKey: e.ctrlKey, - shiftKey: e.shiftKey, - target: e.target, - defaultPrevented: e.defaultPrevented, - preventDefault: () => e.preventDefault() - }) } const onKeyUp = (e: KeyboardEvent): void => { + // The detector is stateful across keydown/keyup, so a composing release must not + // complete a tap pair the matching press was already withheld from. + if (isImeCompositionKeyDown(e)) { + return + } doubleTapDetector.process( toModifierDoubleTapEvent({ type: 'keyUp', diff --git a/src/renderer/src/lib/app-shortcut-dispatch-input.test.ts b/src/renderer/src/lib/app-shortcut-dispatch-input.test.ts new file mode 100644 index 00000000000..b1e22e20395 --- /dev/null +++ b/src/renderer/src/lib/app-shortcut-dispatch-input.test.ts @@ -0,0 +1,66 @@ +// @vitest-environment happy-dom +// Guarding the shared matcher is not enough on its own: this adapter rebuilds the input +// field-by-field, so any field it forgets is a field the matcher can never refuse on. The +// regression these pin is a composing Mod+Alt+W that the matcher rejected directly but that +// resolved `tab.closeAll` once it had been through the adapter. + +import { describe, expect, it } from 'vitest' +import { keybindingMatchesAction } from '../../../shared/keybindings' +import { + toDoubleTapShortcutDispatchInput, + toShortcutDispatchInput +} from './app-shortcut-dispatch-input' + +function closeAllTabsEvent(over: Partial = {}): KeyboardEvent { + return new KeyboardEvent('keydown', { + altKey: true, + code: 'KeyW', + key: 'w', + metaKey: true, + ...over + }) +} + +describe('toShortcutDispatchInput', () => { + it('carries a real chord through to a matching action', () => { + const input = toShortcutDispatchInput(closeAllTabsEvent()) + + expect(input).not.toBeNull() + expect(keybindingMatchesAction('tab.closeAll', input!, 'darwin')).toBe(true) + }) + + it.each([ + ['isComposing', { isComposing: true }], + ['keyCode 229', { keyCode: 229 }] + ])('refuses to build an input for a chord marked by %s', (_marker, over) => { + expect(toShortcutDispatchInput(closeAllTabsEvent(over))).toBeNull() + }) + + // Defense in depth, and the field-level assertion the null-return above cannot make: the + // copy must carry the markers verbatim so the matcher can refuse on its own. Dropping them + // here is the original defect, and it is invisible to every test that only checks the null. + it('copies the IME markers verbatim rather than dropping them', () => { + const input = toShortcutDispatchInput(closeAllTabsEvent())! + + expect(input.isComposing).toBe(false) + expect(input.keyCode).toBe(closeAllTabsEvent().keyCode) + expect(Object.hasOwn(input, 'isComposing')).toBe(true) + expect(Object.hasOwn(input, 'keyCode')).toBe(true) + }) +}) + +describe('toDoubleTapShortcutDispatchInput', () => { + it('builds a gesture from an ordinary release', () => { + const input = toDoubleTapShortcutDispatchInput(new KeyboardEvent('keydown'), 'Shift') + + expect(input?.doubleTapModifier).toBe('Shift') + }) + + // A composing chord must not arm a double-tap: the synthetic input it produces carries no + // key or modifier flags, so there is nothing left downstream for the matcher to refuse on. + it('refuses to build a gesture while an IME owns the keystroke', () => { + expect( + toDoubleTapShortcutDispatchInput(new KeyboardEvent('keydown', { isComposing: true }), 'Shift') + ).toBeNull() + }) +}) diff --git a/src/renderer/src/lib/app-shortcut-dispatch-input.ts b/src/renderer/src/lib/app-shortcut-dispatch-input.ts new file mode 100644 index 00000000000..0352d4db3d5 --- /dev/null +++ b/src/renderer/src/lib/app-shortcut-dispatch-input.ts @@ -0,0 +1,68 @@ +import type { PhysicalModifierToken } from '../../../shared/keybindings' +import { isImeCompositionKeyDown } from '@/lib/ime-composition-keyboard-event' + +/** + * Abstraction over a real KeyboardEvent and a synthetic double-tap gesture so one dispatch + * path serves both; KeybindingInput-compatible. + * + * The IME markers are part of the contract. The shared matcher refuses a chord an IME owns by + * reading them off this object, so an adapter that builds it field-by-field and omits them + * re-opens the hole no matter what the matcher does — which is exactly how a composing + * Mod+Alt+W reached `tab.closeAll` after the matcher was already guarded. + */ +export type ShortcutDispatchInput = { + key?: string + code?: string + altKey?: boolean + metaKey?: boolean + ctrlKey?: boolean + shiftKey?: boolean + isComposing?: boolean + keyCode?: number + doubleTapModifier?: PhysicalModifierToken + target: EventTarget | null + defaultPrevented: boolean + preventDefault: () => void +} + +/** Null when an IME owns the keystroke, so the window listener has nothing to dispatch. */ +export function toShortcutDispatchInput(event: KeyboardEvent): ShortcutDispatchInput | null { + if (isImeCompositionKeyDown(event)) { + return null + } + return { + altKey: event.altKey, + code: event.code, + ctrlKey: event.ctrlKey, + defaultPrevented: event.defaultPrevented, + isComposing: event.isComposing, + key: event.key, + keyCode: event.keyCode, + metaKey: event.metaKey, + preventDefault: () => event.preventDefault(), + shiftKey: event.shiftKey, + target: event.target + } +} + +/** + * The double-tap gesture carries no key or modifier flags, so only DoubleTap bindings match. + * It still passes the markers through: the gesture is only as trustworthy as the release that + * completed it. + */ +export function toDoubleTapShortcutDispatchInput( + event: KeyboardEvent, + doubleTapModifier: PhysicalModifierToken +): ShortcutDispatchInput | null { + if (isImeCompositionKeyDown(event)) { + return null + } + return { + defaultPrevented: event.defaultPrevented, + doubleTapModifier, + isComposing: event.isComposing, + keyCode: event.keyCode, + preventDefault: () => event.preventDefault(), + target: event.target + } +}