mirror of
https://github.com/stablyai/orca.git
synced 2026-10-18 08:03:22 +00:00
fix(shortcuts): harden native-only event tracking
This commit is contained in:
@@ -6,12 +6,9 @@ import type { IDisposable } from '@xterm/xterm'
|
||||
import type { ManagedPane, PaneManager } from '@/lib/pane-manager/pane-manager'
|
||||
import type { PtyTransport } from './pty-transport'
|
||||
import { safeFind } from '../terminal-search-safe-find'
|
||||
import {
|
||||
consumeNativeOnlyTerminalShortcutCompanion,
|
||||
getTerminalShortcutKeyIdentity,
|
||||
resolveTerminalShortcutAction
|
||||
} from './terminal-shortcut-policy'
|
||||
import { resolveTerminalShortcutAction } from './terminal-shortcut-policy'
|
||||
import type { MacOptionAsAlt } from './terminal-shortcut-policy'
|
||||
import { createTerminalNativeOnlyShortcutTracker } from './terminal-native-only-shortcut'
|
||||
import {
|
||||
keybindingMatchesAction,
|
||||
type KeybindingOverrides,
|
||||
@@ -264,7 +261,7 @@ export function useTerminalKeyboardShortcuts({
|
||||
// held. To distinguish left vs right Option, we record the Option key's
|
||||
// location from its own keydown event and clear it on keyup.
|
||||
let optionKeyLocation = 0
|
||||
const pendingNativeOnlyShortcutKeys = new Set<string>()
|
||||
const nativeOnlyShortcutTracker = createTerminalNativeOnlyShortcutTracker()
|
||||
const onModifierDown = (e: KeyboardEvent): void => {
|
||||
if (e.key === 'Alt') {
|
||||
optionKeyLocation = e.location
|
||||
@@ -305,10 +302,9 @@ export function useTerminalKeyboardShortcuts({
|
||||
}
|
||||
|
||||
const onKeyDown = (e: KeyboardEvent): void => {
|
||||
const shortcutKeyIdentity = getTerminalShortcutKeyIdentity(e)
|
||||
// Why: companions only attach to the latest native-only chord; clear so a
|
||||
// lost keyup (blur/focus steal) cannot suppress later Space text forever.
|
||||
pendingNativeOnlyShortcutKeys.clear()
|
||||
// Why: replace stale state only for this physical key so rollover cannot
|
||||
// disarm a still-held native-only chord before its Kitty keyup arrives.
|
||||
nativeOnlyShortcutTracker.prepareKeyDown(e)
|
||||
const manager = managerRef.current
|
||||
if (!manager) {
|
||||
return
|
||||
@@ -410,7 +406,7 @@ export function useTerminalKeyboardShortcuts({
|
||||
if (action.type === 'switchInputSource') {
|
||||
// Why: the OS must receive its default action, while xterm must receive
|
||||
// none of the keydown, keypress, or keyup sequence.
|
||||
pendingNativeOnlyShortcutKeys.add(shortcutKeyIdentity)
|
||||
nativeOnlyShortcutTracker.armKeyDown(e)
|
||||
e.stopImmediatePropagation()
|
||||
return
|
||||
}
|
||||
@@ -625,7 +621,7 @@ export function useTerminalKeyboardShortcuts({
|
||||
}
|
||||
|
||||
const onNativeOnlyShortcutCompanion = (e: KeyboardEvent): void => {
|
||||
if (consumeNativeOnlyTerminalShortcutCompanion(e, pendingNativeOnlyShortcutKeys)) {
|
||||
if (nativeOnlyShortcutTracker.consumeCompanion(e)) {
|
||||
// Why: canceling only the companion keypress prevents Chromium's text
|
||||
// insertion without canceling the keydown default used by the OS switch.
|
||||
if (e.type === 'keypress') {
|
||||
@@ -635,13 +631,10 @@ export function useTerminalKeyboardShortcuts({
|
||||
}
|
||||
}
|
||||
|
||||
// Why: modern Chromium can insert via beforeinput even when keydown is not
|
||||
// preventDefault-ed; block insertText only while a native-only chord is live.
|
||||
// Why: modern Chromium can skip keypress and insert via beforeinput; block
|
||||
// only this chord's text so an IME commit in the same window remains intact.
|
||||
const onNativeOnlyBeforeInput = (e: Event): void => {
|
||||
if (pendingNativeOnlyShortcutKeys.size === 0) {
|
||||
return
|
||||
}
|
||||
if (!(e instanceof InputEvent) || e.inputType !== 'insertText') {
|
||||
if (!(e instanceof InputEvent) || !nativeOnlyShortcutTracker.shouldSuppressBeforeInput(e)) {
|
||||
return
|
||||
}
|
||||
e.preventDefault()
|
||||
@@ -649,7 +642,7 @@ export function useTerminalKeyboardShortcuts({
|
||||
}
|
||||
|
||||
const onNativeOnlyBlur = (): void => {
|
||||
pendingNativeOnlyShortcutKeys.clear()
|
||||
nativeOnlyShortcutTracker.clear()
|
||||
}
|
||||
|
||||
window.addEventListener('keydown', onModifierDown, { capture: true })
|
||||
|
||||
+48
-24
@@ -11,8 +11,10 @@
|
||||
*/
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import {
|
||||
consumeNativeOnlyTerminalShortcutCompanion,
|
||||
getTerminalShortcutKeyIdentity,
|
||||
createTerminalNativeOnlyShortcutTracker,
|
||||
getTerminalShortcutKeyIdentity
|
||||
} from './terminal-native-only-shortcut'
|
||||
import {
|
||||
resolveTerminalShortcutAction,
|
||||
type TerminalShortcutEvent
|
||||
} from './terminal-shortcut-policy'
|
||||
@@ -70,7 +72,7 @@ describe('issue #8299 Shift+Space input-source switch regression', () => {
|
||||
})
|
||||
|
||||
it('leaves unbound Shift+Space as non-action on macOS/Linux/Windows', () => {
|
||||
// Why: Ghostty parity is opt-in; non-IME users keep ordinary Space input.
|
||||
// Why: native-only behavior is opt-in; non-IME users keep ordinary Space input.
|
||||
for (const [isMac, isWindows] of [
|
||||
[true, false],
|
||||
[false, false],
|
||||
@@ -118,17 +120,11 @@ describe('issue #8299 Shift+Space input-source switch regression', () => {
|
||||
|
||||
it('suppresses keypress/keyup companions including keypress without code', () => {
|
||||
// Why: Chromium keypress may omit code and only report key:" ".
|
||||
const pending = new Set([
|
||||
getTerminalShortcutKeyIdentity(shortcutEvent({ key: ' ', code: 'Space', shiftKey: true }))
|
||||
])
|
||||
expect(pending.has('Space')).toBe(true)
|
||||
expect(
|
||||
consumeNativeOnlyTerminalShortcutCompanion({ type: 'keypress', key: ' ' }, pending)
|
||||
).toBe(true)
|
||||
expect(consumeNativeOnlyTerminalShortcutCompanion({ type: 'keyup', key: ' ' }, pending)).toBe(
|
||||
true
|
||||
)
|
||||
expect(pending.size).toBe(0)
|
||||
const tracker = createTerminalNativeOnlyShortcutTracker()
|
||||
tracker.armKeyDown(shortcutEvent({ key: ' ', code: 'Space', shiftKey: true }))
|
||||
expect(tracker.consumeCompanion({ type: 'keypress', key: ' ' })).toBe(true)
|
||||
expect(tracker.consumeCompanion({ type: 'keyup', key: ' ' })).toBe(true)
|
||||
expect(tracker.consumeCompanion({ type: 'keyup', key: ' ' })).toBe(false)
|
||||
})
|
||||
|
||||
it('normalizes Space identities across key/code aliases', () => {
|
||||
@@ -139,19 +135,47 @@ describe('issue #8299 Shift+Space input-source switch regression', () => {
|
||||
})
|
||||
|
||||
it('does not suppress unrelated companions while Space is pending', () => {
|
||||
const pending = new Set(['Space'])
|
||||
const tracker = createTerminalNativeOnlyShortcutTracker()
|
||||
tracker.armKeyDown({ key: ' ', code: 'Space' })
|
||||
expect(tracker.consumeCompanion({ type: 'keypress', key: 'a', code: 'KeyA' })).toBe(false)
|
||||
expect(tracker.consumeCompanion({ type: 'keydown', key: ' ', code: 'Space' })).toBe(false)
|
||||
expect(tracker.consumeCompanion({ type: 'keypress', key: ' ' })).toBe(true)
|
||||
})
|
||||
|
||||
it('keeps Space armed through unrelated key rollover until its keyup', () => {
|
||||
const tracker = createTerminalNativeOnlyShortcutTracker()
|
||||
tracker.armKeyDown({ key: ' ', code: 'Space' })
|
||||
tracker.prepareKeyDown({ key: 'a', code: 'KeyA' })
|
||||
|
||||
expect(
|
||||
consumeNativeOnlyTerminalShortcutCompanion(
|
||||
{ type: 'keypress', key: 'a', code: 'KeyA' },
|
||||
pending
|
||||
)
|
||||
tracker.shouldSuppressBeforeInput({ data: 'a', inputType: 'insertText', isComposing: false })
|
||||
).toBe(false)
|
||||
expect(tracker.consumeCompanion({ type: 'keyup', key: ' ', code: 'Space' })).toBe(true)
|
||||
})
|
||||
|
||||
it('replaces stale state when the same physical key is pressed again', () => {
|
||||
const tracker = createTerminalNativeOnlyShortcutTracker()
|
||||
tracker.armKeyDown({ key: ' ', code: 'Space' })
|
||||
tracker.prepareKeyDown({ key: ' ', code: 'Space' })
|
||||
|
||||
expect(tracker.consumeCompanion({ type: 'keyup', key: ' ', code: 'Space' })).toBe(false)
|
||||
})
|
||||
|
||||
it('suppresses only the shortcut text on the beforeinput fallback', () => {
|
||||
const tracker = createTerminalNativeOnlyShortcutTracker()
|
||||
tracker.armKeyDown({ key: ' ', code: 'Space' })
|
||||
expect(
|
||||
tracker.shouldSuppressBeforeInput({ data: ' ', inputType: 'insertText', isComposing: false })
|
||||
).toBe(true)
|
||||
expect(
|
||||
tracker.shouldSuppressBeforeInput({ data: '한', inputType: 'insertText', isComposing: false })
|
||||
).toBe(false)
|
||||
expect(
|
||||
consumeNativeOnlyTerminalShortcutCompanion(
|
||||
{ type: 'keydown', key: ' ', code: 'Space' },
|
||||
pending
|
||||
)
|
||||
tracker.shouldSuppressBeforeInput({
|
||||
data: ' ',
|
||||
inputType: 'insertCompositionText',
|
||||
isComposing: true
|
||||
})
|
||||
).toBe(false)
|
||||
expect(pending.has('Space')).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -0,0 +1,119 @@
|
||||
type TerminalNativeOnlyShortcutKeyEvent = {
|
||||
key: string
|
||||
code?: string
|
||||
}
|
||||
|
||||
type TerminalNativeOnlyShortcutCompanionEvent = TerminalNativeOnlyShortcutKeyEvent & {
|
||||
type: string
|
||||
}
|
||||
|
||||
type TerminalNativeOnlyShortcutBeforeInputEvent = {
|
||||
data: string | null
|
||||
inputType: string
|
||||
isComposing: boolean
|
||||
}
|
||||
|
||||
export type TerminalNativeOnlyShortcutTracker = {
|
||||
prepareKeyDown: (event: TerminalNativeOnlyShortcutKeyEvent) => void
|
||||
armKeyDown: (event: TerminalNativeOnlyShortcutKeyEvent) => void
|
||||
consumeCompanion: (event: TerminalNativeOnlyShortcutCompanionEvent) => boolean
|
||||
shouldSuppressBeforeInput: (event: TerminalNativeOnlyShortcutBeforeInputEvent) => boolean
|
||||
clear: () => void
|
||||
}
|
||||
|
||||
/**
|
||||
* Physical-key identity for pairing a native-only keydown with its keypress/keyup.
|
||||
* Why: Chromium keypress can omit `code` and report only `key: " "`; normalize so
|
||||
* keydown (`code: "Space"`) still matches the companion events.
|
||||
*/
|
||||
export function getTerminalShortcutKeyIdentity(event: TerminalNativeOnlyShortcutKeyEvent): string {
|
||||
const code = event.code?.trim()
|
||||
if (code) {
|
||||
return code
|
||||
}
|
||||
// Why: Space is the reported input-source chord; bare " " must not diverge
|
||||
// from code-based identities used on keydown.
|
||||
if (event.key === ' ' || event.key === 'Spacebar' || event.key === 'Space') {
|
||||
return 'Space'
|
||||
}
|
||||
return event.key
|
||||
}
|
||||
|
||||
function getCompanionKeyCandidates(event: TerminalNativeOnlyShortcutKeyEvent): Set<string> {
|
||||
const candidates = new Set<string>([getTerminalShortcutKeyIdentity(event)])
|
||||
if (event.code?.trim()) {
|
||||
candidates.add(event.code.trim())
|
||||
}
|
||||
if (event.key === ' ' || event.key === 'Spacebar' || event.key === 'Space') {
|
||||
candidates.add('Space')
|
||||
}
|
||||
return candidates
|
||||
}
|
||||
|
||||
function consumeCompanion(
|
||||
event: TerminalNativeOnlyShortcutCompanionEvent,
|
||||
pendingKeys: Map<string, string>
|
||||
): boolean {
|
||||
if (event.type !== 'keypress' && event.type !== 'keyup') {
|
||||
return false
|
||||
}
|
||||
// Why: match either normalized identity so keypress without `code` still
|
||||
// pairs with a keydown that used `code: "Space"`.
|
||||
const candidates = getCompanionKeyCandidates(event)
|
||||
let matched: string | null = null
|
||||
for (const candidate of candidates) {
|
||||
if (pendingKeys.has(candidate)) {
|
||||
matched = candidate
|
||||
break
|
||||
}
|
||||
}
|
||||
if (!matched) {
|
||||
return false
|
||||
}
|
||||
if (event.type === 'keyup') {
|
||||
pendingKeys.delete(matched)
|
||||
// Why: drop any alias forms for the same physical key after release.
|
||||
for (const candidate of candidates) {
|
||||
pendingKeys.delete(candidate)
|
||||
}
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
function getExpectedText(key: string): string | null {
|
||||
return key === 'Spacebar' || key === 'Space' ? ' ' : key.length === 1 ? key : null
|
||||
}
|
||||
|
||||
export function createTerminalNativeOnlyShortcutTracker(): TerminalNativeOnlyShortcutTracker {
|
||||
const pendingKeys = new Map<string, string>()
|
||||
|
||||
const shouldSuppressBeforeInput = (
|
||||
event: TerminalNativeOnlyShortcutBeforeInputEvent
|
||||
): boolean => {
|
||||
if (event.inputType !== 'insertText' || event.isComposing) {
|
||||
return false
|
||||
}
|
||||
// Why: an input-source change can also commit IME text before keyup; cancel
|
||||
// only the printable text produced by a configured native-only chord.
|
||||
for (const key of pendingKeys.values()) {
|
||||
if (getExpectedText(key) === event.data) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
return {
|
||||
prepareKeyDown: (event) => {
|
||||
// Why: replace a lost-keyup entry for this key without disarming other
|
||||
// held native-only keys during normal key rollover.
|
||||
pendingKeys.delete(getTerminalShortcutKeyIdentity(event))
|
||||
},
|
||||
armKeyDown: (event) => {
|
||||
pendingKeys.set(getTerminalShortcutKeyIdentity(event), event.key)
|
||||
},
|
||||
consumeCompanion: (event) => consumeCompanion(event, pendingKeys),
|
||||
shouldSuppressBeforeInput,
|
||||
clear: () => pendingKeys.clear()
|
||||
}
|
||||
}
|
||||
@@ -1,7 +1,6 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { createTerminalNativeOnlyShortcutTracker } from './terminal-native-only-shortcut'
|
||||
import {
|
||||
consumeNativeOnlyTerminalShortcutCompanion,
|
||||
getTerminalShortcutKeyIdentity,
|
||||
resolveTerminalShortcutAction,
|
||||
type TerminalShortcutEvent
|
||||
} from './terminal-shortcut-policy'
|
||||
@@ -712,23 +711,12 @@ describe('resolveTerminalShortcutAction', () => {
|
||||
})
|
||||
|
||||
it('suppresses the full companion event sequence for a native-only shortcut', () => {
|
||||
const pendingKeys = new Set([
|
||||
getTerminalShortcutKeyIdentity(event({ key: ' ', code: 'Space', shiftKey: true }))
|
||||
])
|
||||
const tracker = createTerminalNativeOnlyShortcutTracker()
|
||||
tracker.armKeyDown(event({ key: ' ', code: 'Space', shiftKey: true }))
|
||||
|
||||
expect(
|
||||
consumeNativeOnlyTerminalShortcutCompanion(
|
||||
{ type: 'keypress', key: ' ', code: 'Space' },
|
||||
pendingKeys
|
||||
)
|
||||
).toBe(true)
|
||||
expect(
|
||||
consumeNativeOnlyTerminalShortcutCompanion(
|
||||
{ type: 'keyup', key: ' ', code: 'Space' },
|
||||
pendingKeys
|
||||
)
|
||||
).toBe(true)
|
||||
expect(pendingKeys.size).toBe(0)
|
||||
expect(tracker.consumeCompanion({ type: 'keypress', key: ' ', code: 'Space' })).toBe(true)
|
||||
expect(tracker.consumeCompanion({ type: 'keyup', key: ' ', code: 'Space' })).toBe(true)
|
||||
expect(tracker.consumeCompanion({ type: 'keyup', key: ' ', code: 'Space' })).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -11,10 +11,6 @@ export type TerminalShortcutEvent = {
|
||||
repeat?: boolean
|
||||
}
|
||||
|
||||
type TerminalShortcutCompanionEvent = Pick<TerminalShortcutEvent, 'key' | 'code'> & {
|
||||
type: string
|
||||
}
|
||||
|
||||
export type MacOptionAsAlt = 'true' | 'false' | 'left' | 'right'
|
||||
|
||||
// Why: macOS composition replaces event.key for punctuation, so we map
|
||||
@@ -48,62 +44,6 @@ export type TerminalShortcutAction =
|
||||
| { type: 'sendInput'; data: string }
|
||||
| { type: 'switchInputSource' }
|
||||
|
||||
/**
|
||||
* Physical-key identity for pairing a native-only keydown with its keypress/keyup.
|
||||
* Why: Chromium keypress can omit `code` and report only `key: " "`; normalize so
|
||||
* keydown (`code: "Space"`) still matches the companion events.
|
||||
*/
|
||||
export function getTerminalShortcutKeyIdentity(
|
||||
event: Pick<TerminalShortcutEvent, 'key' | 'code'>
|
||||
): string {
|
||||
const code = event.code?.trim()
|
||||
if (code) {
|
||||
return code
|
||||
}
|
||||
// Why: Space is the reported input-source chord; bare " " must not diverge
|
||||
// from code-based identities used on keydown.
|
||||
if (event.key === ' ' || event.key === 'Spacebar' || event.key === 'Space') {
|
||||
return 'Space'
|
||||
}
|
||||
return event.key
|
||||
}
|
||||
|
||||
export function consumeNativeOnlyTerminalShortcutCompanion(
|
||||
event: TerminalShortcutCompanionEvent,
|
||||
pendingKeys: Set<string>
|
||||
): boolean {
|
||||
if (event.type !== 'keypress' && event.type !== 'keyup') {
|
||||
return false
|
||||
}
|
||||
// Why: match either normalized identity so keypress without `code` still
|
||||
// pairs with a keydown that used `code: "Space"`.
|
||||
const candidates = new Set<string>([getTerminalShortcutKeyIdentity(event)])
|
||||
if (event.code?.trim()) {
|
||||
candidates.add(event.code.trim())
|
||||
}
|
||||
if (event.key === ' ' || event.key === 'Spacebar' || event.key === 'Space') {
|
||||
candidates.add('Space')
|
||||
}
|
||||
let matched: string | null = null
|
||||
for (const candidate of candidates) {
|
||||
if (pendingKeys.has(candidate)) {
|
||||
matched = candidate
|
||||
break
|
||||
}
|
||||
}
|
||||
if (!matched) {
|
||||
return false
|
||||
}
|
||||
if (event.type === 'keyup') {
|
||||
pendingKeys.delete(matched)
|
||||
// Why: drop any alias forms for the same physical key after release.
|
||||
for (const candidate of candidates) {
|
||||
pendingKeys.delete(candidate)
|
||||
}
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
/** Kitty keyboard protocol modifier field: 1 + shift(1) + alt(2). */
|
||||
function kittyAltModifiers(shiftKey: boolean): number {
|
||||
return shiftKey ? 4 : 3
|
||||
|
||||
Reference in New Issue
Block a user