diff --git a/src/main/browser/browser-guest-ui.test.ts b/src/main/browser/browser-guest-ui.test.ts index 946ed38a7ef..b5edb09691f 100644 --- a/src/main/browser/browser-guest-ui.test.ts +++ b/src/main/browser/browser-guest-ui.test.ts @@ -801,6 +801,45 @@ describe('setupGuestShortcutForwarding', () => { expect(rendererSendMock).not.toHaveBeenCalledWith('ui:openQuickOpen') }) + // `Electron.Input` carries `isComposing`, so the guest path can refuse an IME-owned press the + // same way the renderer does. A completed gesture dispatches `{ doubleTapModifier }` with no + // key or modifier flags, leaving the matcher nothing to refuse on — it has to stop here. + it.each([ + ['a marked second press', 1], + ['a marked release', 0] + ])('does not complete a guest double-tap from %s', (_case, markedIndex) => { + setupGuestShortcutForwarding({ + browserTabId, + guest: makeGuest(), + resolveRenderer: () => makeRenderer(), + getKeybindings: () => ({ + 'worktree.quickOpen': ['DoubleTap+Shift'] + }) + }) + + const modifierInput = { + code: 'ShiftLeft', + key: 'Shift', + shift: true, + meta: false, + control: false, + alt: false + } + const sequence = [ + { ...modifierInput, type: 'keyUp' as const }, + { ...modifierInput, type: 'keyDown' as const } + ] + triggerBeforeInput(modifierInput) + const preventDefaults = sequence.map((step, index) => + triggerBeforeInput(index === markedIndex ? { ...step, isComposing: true } : step) + ) + + for (const preventDefault of preventDefaults) { + expect(preventDefault).not.toHaveBeenCalled() + } + expect(rendererSendMock).not.toHaveBeenCalledWith('ui:openQuickOpen') + }) + // A floating-workspace-owned guest routes close/index chords to the floating-scoped // IPC *carrying its source id*; a main-owned guest keeps main routing. describe('source-aware close/index routing', () => { diff --git a/src/main/browser/browser-guest-ui.ts b/src/main/browser/browser-guest-ui.ts index b9c52b39aa7..0c6aed69c41 100644 --- a/src/main/browser/browser-guest-ui.ts +++ b/src/main/browser/browser-guest-ui.ts @@ -500,6 +500,9 @@ export function setupGuestShortcutForwarding(args: { control: input.control, alt: input.alt, meta: input.meta, + // Electron.Input carries isComposing (no numeric keyCode); without it the detector + // could arm a gesture from a keystroke the IME owns. + isComposing: input.isComposing, isAutoRepeat: input.isAutoRepeat }), Date.now() diff --git a/src/main/window/createMainWindow.test.ts b/src/main/window/createMainWindow.test.ts index 8a61f157584..775e07a6380 100644 --- a/src/main/window/createMainWindow.test.ts +++ b/src/main/window/createMainWindow.test.ts @@ -1315,6 +1315,41 @@ describe('createMainWindow', () => { const togglePreventDefault = triggerDoubleTapShift() expect(togglePreventDefault).toHaveBeenCalledTimes(1) expect(webContents.send).toHaveBeenCalledWith('ui:dictationKeyDown') + + // `Electron.Input` carries `isComposing`. A completed gesture dispatches + // `{ doubleTapModifier }` with no key or modifier flags, so the shortcut matcher has nothing + // left to refuse on — the ownership check has to happen before the detector sees the press. + const modifierInput = { + code: 'ShiftLeft', + key: 'Shift', + shift: true, + meta: false, + control: false, + alt: false + } + for (const markedIndex of [1, 0]) { + webContents.send.mockClear() + const steps = [ + { ...modifierInput, type: 'keyUp' }, + { ...modifierInput, type: 'keyDown' } + ] + windowHandlers['before-input-event']( + { preventDefault: vi.fn() } as never, + { ...modifierInput, type: 'keyDown' } as never + ) + for (const [index, step] of steps.entries()) { + windowHandlers['before-input-event']( + { preventDefault: vi.fn() } as never, + (index === markedIndex ? { ...step, isComposing: true } : step) as never + ) + } + expect(webContents.send).not.toHaveBeenCalledWith('ui:dictationKeyDown') + // Clear the trailing armed state so the next case starts from idle. + windowHandlers['before-input-event']( + { preventDefault: vi.fn() } as never, + { code: 'KeyA', key: 'a', type: 'keyDown' } as never + ) + } }) it('forwards ctrl/cmd+j to the worktree palette toggle event', () => { diff --git a/src/main/window/createMainWindow.ts b/src/main/window/createMainWindow.ts index 49c3eb4c125..a8fe4f2ae73 100644 --- a/src/main/window/createMainWindow.ts +++ b/src/main/window/createMainWindow.ts @@ -834,6 +834,9 @@ export function createMainWindow( control: input.control, alt: input.alt, meta: input.meta, + // Electron.Input carries isComposing (no numeric keyCode); without it the detector + // could arm a gesture from a keystroke the IME owns. + isComposing: input.isComposing, isAutoRepeat: input.isAutoRepeat }), Date.now() diff --git a/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.test.tsx b/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.test.tsx index d9f846fbbda..3b03442891d 100644 --- a/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.test.tsx +++ b/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.test.tsx @@ -1713,6 +1713,92 @@ describe('FloatingTerminalPanel close behavior', () => { expect(mocks.createTab).not.toHaveBeenCalled() }) + // The double-tap gesture is dispatched as `{ doubleTapModifier }` with no key or modifier + // flags, so once it is armed the shortcut matcher has nothing left to refuse on. These drive + // the real window listeners rather than the detector, because the panel is where the ownership + // decision is read off the event and handed down. + describe('while an IME owns a focused floating terminal keystroke', () => { + async function bindDoubleTapListeners(): Promise<{ + keyDown: (event: unknown) => void + keyUp: (event: unknown) => void + shiftTap: (over?: Record) => Record + }> { + setFloatingTabs([makeTab({ id: 'tab-1' })]) + ;(storeBox.state as FloatingPanelStoreState).keybindings = { + 'tab.newTerminal': ['DoubleTap+Shift'] + } + const element = await renderPanel(true) + const panel = findByProp(element, 'data-floating-terminal-panel') + const panelElement = { contains: vi.fn().mockReturnValue(true), focus: vi.fn() } + const target = { + classList: { contains: vi.fn((token: string) => token === 'xterm-helper-textarea') }, + closest: vi.fn((selector: string) => + selector === '[data-floating-terminal-panel]' ? panelElement : null + ) + } + Object.setPrototypeOf(target, HTMLElement.prototype) + attachRef(panel.props.ref, panelElement) + vi.stubGlobal('document', { + activeElement: target, + addEventListener: vi.fn(), + removeEventListener: vi.fn() + }) + runEffects() + const listener = (type: string): ((event: unknown) => void) | undefined => + vi.mocked(window.addEventListener).mock.calls.find(([name]) => name === type)?.[1] as + | ((event: unknown) => void) + | undefined + const keyDown = listener('keydown') + const keyUp = listener('keyup') + if (!keyDown || !keyUp) { + throw new Error('keyboard listeners not registered') + } + return { + keyDown, + keyUp, + shiftTap: (over = {}) => ({ + altKey: false, + code: 'ShiftLeft', + ctrlKey: false, + defaultPrevented: false, + key: 'Shift', + metaKey: false, + preventDefault: vi.fn(), + repeat: false, + shiftKey: true, + stopImmediatePropagation: vi.fn(), + stopPropagation: vi.fn(), + target, + ...over + }) + } + } + + it('does not complete a double-tap whose second press is marked', async () => { + const { keyDown, keyUp, shiftTap } = await bindDoubleTapListeners() + + keyDown(shiftTap()) + keyUp(shiftTap()) + // The real shape a composing press has — not `key: 'Process'`, which resolves to no + // binding and so would pass even with the ownership check deleted. + keyDown(shiftTap({ isComposing: true, keyCode: 229 })) + await flushAsyncWork() + + expect(mocks.createTab).not.toHaveBeenCalled() + }) + + it('does not let a marked release arm the second half of the gesture', async () => { + const { keyDown, keyUp, shiftTap } = await bindDoubleTapListeners() + + keyDown(shiftTap()) + keyUp(shiftTap({ isComposing: true, keyCode: 229 })) + keyDown(shiftTap()) + await flushAsyncWork() + + expect(mocks.createTab).not.toHaveBeenCalled() + }) + }) + it('routes focused floating tab switch shortcuts to the floating workspace', async () => { setFloatingTabs([makeTab({ id: 'tab-1' }), makeTab({ id: 'tab-2' })]) const element = await renderPanel(true) diff --git a/src/shared/window-shortcut-policy-ime-ownership.test.ts b/src/shared/window-shortcut-policy-ime-ownership.test.ts new file mode 100644 index 00000000000..5269ea2fdea --- /dev/null +++ b/src/shared/window-shortcut-policy-ime-ownership.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, it } from 'vitest' +import { + matchesRecentTabSwitcherChord, + resolveWindowShortcutAction, + type WindowShortcutInput +} from './window-shortcut-policy' +import type { KeybindingOverrides } from './keybindings' + +// `matchesRecentTabSwitcherChord` takes a whole event but hands the matcher a literal it rebuilds +// field-by-field, so the IME markers can go missing one level below a call site that looks +// perfectly safe. That is how it survived the sweep that fixed the four direct adapters. +describe('window shortcut resolution refuses input an IME owns', () => { + const recentTab: KeybindingOverrides = { 'tab.previousRecent': ['Mod+Tab'] } + + const ctrlTab = (over: Partial = {}): WindowShortcutInput => ({ + type: 'keyDown', + key: 'Tab', + code: 'Tab', + control: true, + ...over + }) + + it('matches a real Ctrl+Tab', () => { + expect(matchesRecentTabSwitcherChord(ctrlTab(), 'linux', recentTab)).toBe(true) + }) + + it.each([ + ['isComposing', { isComposing: true }], + ['keyCode 229', { keyCode: 229 }] + ])('refuses the switcher chord when marked by %s', (_marker, over) => { + expect(matchesRecentTabSwitcherChord(ctrlTab(over), 'linux', recentTab)).toBe(false) + }) + + // The resolver forwards its input untouched, so this pins that it keeps doing so. + it('resolves no action for a marked chord', () => { + const zoomIn: WindowShortcutInput = { key: '=', code: 'Equal', control: true } + + expect(resolveWindowShortcutAction(zoomIn, 'linux')).toEqual({ + type: 'zoom', + direction: 'in' + }) + expect(resolveWindowShortcutAction({ ...zoomIn, isComposing: true }, 'linux')).toBeNull() + }) +}) diff --git a/src/shared/window-shortcut-policy.ts b/src/shared/window-shortcut-policy.ts index 0099c7e88a0..553a4d85f60 100644 --- a/src/shared/window-shortcut-policy.ts +++ b/src/shared/window-shortcut-policy.ts @@ -22,6 +22,10 @@ export type WindowShortcutInput = { metaKey?: boolean ctrlKey?: boolean shiftKey?: boolean + // The matcher refuses a chord an IME owns by reading these, so anything that rebuilds this + // shape has to carry them. `Electron.Input` supplies isComposing; it has no numeric keyCode. + isComposing?: boolean + keyCode?: number // Set only by the double-tap detector; threads the synthetic input through // the main-process resolver so allowlisted actions can fire on double-tap. doubleTapModifier?: PhysicalModifierToken @@ -89,7 +93,11 @@ export function matchesRecentTabSwitcherChord( altKey: alt, metaKey: meta, ctrlKey: control, - shiftKey: false + shiftKey: false, + // Rebuilt rather than forwarded because Shift is deliberately normalized away, so the + // markers have to be carried across by hand or the matcher has nothing to refuse on. + isComposing: input.isComposing, + keyCode: input.keyCode }, platform, keybindings,