diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts index 057757f8046..2578d657569 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts @@ -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() + 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 }) diff --git a/src/renderer/src/components/terminal-pane/repro-8299-shift-space-input-source.test.ts b/src/renderer/src/components/terminal-pane/repro-8299-shift-space-input-source.test.ts index b6561a92e99..3bdfb801ae7 100644 --- a/src/renderer/src/components/terminal-pane/repro-8299-shift-space-input-source.test.ts +++ b/src/renderer/src/components/terminal-pane/repro-8299-shift-space-input-source.test.ts @@ -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) }) }) diff --git a/src/renderer/src/components/terminal-pane/terminal-native-only-shortcut.ts b/src/renderer/src/components/terminal-pane/terminal-native-only-shortcut.ts new file mode 100644 index 00000000000..12d04a0abe8 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-native-only-shortcut.ts @@ -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 { + const candidates = new Set([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 +): 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() + + 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() + } +} diff --git a/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.test.ts b/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.test.ts index 9665ca2969f..2736d5e0797 100644 --- a/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.test.ts @@ -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) }) }) diff --git a/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts b/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts index 9e2101ce834..976cfa9bb7a 100644 --- a/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts +++ b/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts @@ -11,10 +11,6 @@ export type TerminalShortcutEvent = { repeat?: boolean } -type TerminalShortcutCompanionEvent = Pick & { - 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 -): 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 -): 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([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