mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 08:03:28 +00:00
fix(ime): carry the composition markers through the last two rebuilt inputs
Two more places rebuilt a keybinding input field-by-field and dropped the
markers the matcher refuses on.
`matchesRecentTabSwitcherChord` is the harder one: its callers pass the raw
event, so a call-site sweep reads clean, but the helper normalizes Shift away
by hand-building a literal one level down. Ctrl+Tab therefore still resolved
mid-composition.
The two `before-input-event` handlers omitted `isComposing` when feeding the
double-tap detector. `Electron.Input` does carry it — a completed gesture
dispatches `{ doubleTapModifier }` with no key or modifier flags, so once one
is armed the matcher has nothing left to refuse on.
Also covers the floating terminal panel, whose product fix already landed but
whose suite passed unchanged when both ownership decisions were mutated away.
Every guard here is mutation-checked: disabling one fails exactly the tests
that name it, and no others.
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -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', () => {
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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<string, unknown>) => Record<string, unknown>
|
||||
}> {
|
||||
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)
|
||||
|
||||
@@ -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> = {}): 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()
|
||||
})
|
||||
})
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user