mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 16:03:20 +00:00
fix(terminal): match Mod+letter shortcuts by physical key, not IME-rewritten key
With a CJK input source active, macOS and Windows report the physical key through
`code` but rewrite `key` to the layout's character: Korean 2-Set turns Cmd+C into
`{ key: "ㅊ", code: "KeyC", metaKey: true }`. Every `key.toLowerCase() === 'c'`
match misses it, so the shortcut is not recognised and xterm encodes the chord as
PTY input instead — issue #13033 reports `ESC[12618;9u` and a terminal that jumps
to the bottom, because user input scrolls the viewport.
This is the same key-vs-code confusion that owned #12171, where a `Shift+T`
typing ㅆ was read as Enter for want of a `code` guard, so the fix is the same
shape: trust `code` when it is present, fall back to `key` and then the legacy
`keyCode` when it is not (Chromium omits `code` on synthetic and some keypress
events, and `keyCode` keeps its US value even when `key` is rewritten).
Applied to the four terminal-side sites, including the dashboard pop-out, which
#13033 called out specifically as having its own key handler:
pty-connection.ts Cmd/Ctrl+C copy guard
keyboard-handlers.ts Cmd+G search navigation
agent-interrupt-inference.ts interrupt inference
preview-terminal-key-handler.ts pop-out paste
Nine further `key.toLowerCase()` letter matches exist outside the terminal
(TaskPage, editor, GitHub composer, browser markup). They have the same defect
and are deliberately left for a separate change rather than widening this one.
An existing case, `matchSearchNavigate > returns null for wrong key`, overrode
only `key` and left `code: 'KeyG'`, so it began passing for the wrong reason. It
now overrides both — which is what "wrong key" means once matching is physical —
and a companion case pins the Korean-rewritten chord still matching.
#13033 was closed NOT_PLANNED; the reporter's event shapes drive the new test.
This commit is contained in:
@@ -11,6 +11,7 @@ import {
|
||||
} from './preview-terminal-shortcuts'
|
||||
import { isImeOwnedKeyboardEvent } from '@/lib/ime-composition-keyboard-event'
|
||||
import { shouldBypassXtermForMacNativeText } from '@/components/terminal-pane/xterm-bypass-policy'
|
||||
import { isLatinShortcutKey } from '../terminal-pane/terminal-ime-latin-shortcut-key'
|
||||
|
||||
/**
|
||||
* Installs the preview terminal's ONE custom key handler (xterm allows a single
|
||||
@@ -94,7 +95,7 @@ export function installPreviewTerminalKeyHandler(args: {
|
||||
(platform === 'darwin' ? event.metaKey && !event.ctrlKey : event.ctrlKey && !event.metaKey) &&
|
||||
!event.altKey &&
|
||||
!event.shiftKey &&
|
||||
event.key.toLowerCase() === 'v'
|
||||
isLatinShortcutKey(event, 'v')
|
||||
if (
|
||||
!isMenuPasteChord &&
|
||||
keybindingMatchesAction('terminal.paste', event, platform, keybindings)
|
||||
|
||||
@@ -9,6 +9,7 @@ import {
|
||||
} from '../../../../shared/agent-interrupt-intent'
|
||||
import { isAskUserQuestionTool } from '../../../../shared/agent-question-answered-intent'
|
||||
import { isExplicitAgentStatusFresh } from '@/lib/agent-status'
|
||||
import { isLatinShortcutKey } from './terminal-ime-latin-shortcut-key'
|
||||
|
||||
export type AgentInterruptInference = {
|
||||
observeInputIntent(
|
||||
@@ -99,7 +100,7 @@ export function isCtrlCKeyEvent(
|
||||
event: Pick<KeyboardEvent, 'key' | 'ctrlKey' | 'metaKey' | 'altKey' | 'shiftKey' | 'repeat'>
|
||||
): boolean {
|
||||
return (
|
||||
event.key.toLowerCase() === 'c' &&
|
||||
isLatinShortcutKey(event, 'c') &&
|
||||
!event.repeat &&
|
||||
event.ctrlKey &&
|
||||
!event.metaKey &&
|
||||
|
||||
@@ -93,10 +93,17 @@ describe('matchSearchNavigate', () => {
|
||||
})
|
||||
|
||||
it('returns null for wrong key', () => {
|
||||
const e = makeKeyEvent({ metaKey: true, key: 'f' })
|
||||
// `code` too, not just `key`: matching is physical-key based so a CJK IME rewriting
|
||||
// `key` cannot hide the shortcut (#13033). A different key means a different `code`.
|
||||
const e = makeKeyEvent({ metaKey: true, key: 'f', code: 'KeyF' })
|
||||
expect(matchSearchNavigate(e, isMac, true, searchState)).toBeNull()
|
||||
})
|
||||
|
||||
it('still matches Cmd+G when a Korean IME has rewritten key (#13033)', () => {
|
||||
const e = makeKeyEvent({ metaKey: true, key: 'ㅎ', code: 'KeyG' })
|
||||
expect(matchSearchNavigate(e, isMac, true, searchState)).toBe('next')
|
||||
})
|
||||
|
||||
it('returns null when alt is pressed', () => {
|
||||
const e = makeKeyEvent({ metaKey: true, altKey: true })
|
||||
expect(matchSearchNavigate(e, isMac, true, searchState)).toBeNull()
|
||||
|
||||
@@ -36,6 +36,7 @@ import { makePaneKey } from '../../../../shared/stable-pane-id'
|
||||
import { resolveWindowsShiftEnterEncodingForPane } from './terminal-windows-shift-enter'
|
||||
import { resolveTerminalInputHostPlatform } from './terminal-input-host-platform'
|
||||
import { isImeOwnedKeyboardEvent } from '@/lib/ime-composition-keyboard-event'
|
||||
import { isLatinShortcutKey } from './terminal-ime-latin-shortcut-key'
|
||||
import {
|
||||
markTerminalFollowOutput,
|
||||
markTerminalPinnedViewport,
|
||||
@@ -135,7 +136,7 @@ export function matchSearchNavigate(
|
||||
if (!mod) {
|
||||
return null
|
||||
}
|
||||
if (e.key.toLowerCase() !== 'g') {
|
||||
if (!isLatinShortcutKey(e, 'g')) {
|
||||
return null
|
||||
}
|
||||
if (!searchOpen) {
|
||||
|
||||
@@ -320,6 +320,7 @@ import {
|
||||
import { isRendererHiddenPtyDeliveryGateEnabled } from './terminal-hidden-delivery-gate'
|
||||
import type { DirectSshPaneRetryAttempt } from '@/store/slices/direct-ssh-terminal-recovery'
|
||||
import { directSshAuthoritiesEqual } from '@/store/slices/direct-ssh-terminal-authority-ledger'
|
||||
import { isLatinShortcutKey } from './terminal-ime-latin-shortcut-key'
|
||||
|
||||
const pendingSpawnByPaneKey = new Map<string, Promise<string | null>>()
|
||||
const SSH_SESSION_EXPIRED_ERROR = 'SSH_SESSION_EXPIRED'
|
||||
@@ -2236,7 +2237,7 @@ export function connectPanePty(
|
||||
}
|
||||
if (
|
||||
(event.metaKey || event.ctrlKey) &&
|
||||
event.key.toLowerCase() === 'c' &&
|
||||
isLatinShortcutKey(event, 'c') &&
|
||||
pane.terminal.hasSelection()
|
||||
) {
|
||||
return
|
||||
|
||||
@@ -0,0 +1,59 @@
|
||||
// @vitest-environment happy-dom
|
||||
/**
|
||||
* Issue #13033 — on macOS with Korean 2-Set active, the physical Cmd+C chord is reported as
|
||||
* `{ key: "ㅊ", code: "KeyC", metaKey: true }`. Every `key.toLowerCase() === 'c'` match misses
|
||||
* it, the copy shortcut is not recognised, and xterm encodes the chord as PTY input — the
|
||||
* reporter observed `ESC[12618;9u` and a viewport that jumps to the bottom, because user input
|
||||
* scrolls the terminal.
|
||||
*
|
||||
* The event shapes below are the reporter's, from #13033 (Orca 1.4.158, macOS 26.5.2, Apple
|
||||
* Silicon, `com.apple.inputmethod.Korean.2SetKorean`). The non-Korean rows are the same physical
|
||||
* key under other input sources that also rewrite `key`.
|
||||
*
|
||||
* This is the same key-vs-code confusion that owned #12171, where `Shift+T` typing ㅆ was read as
|
||||
* Enter for want of a `code` guard — so the fix is the same shape: trust `code` when it is there.
|
||||
*/
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { isLatinShortcutKey } from './terminal-ime-latin-shortcut-key'
|
||||
|
||||
/** Physical Cmd+C as reported under each input source. `code` is stable; `key` is not. */
|
||||
const CMD_C_BY_INPUT_SOURCE = [
|
||||
{ key: 'c', label: 'ABC (Latin)' },
|
||||
{ key: 'ㅊ', label: 'Korean 2-Set — the reported shape' },
|
||||
{ key: 'そ', label: 'Japanese kana' },
|
||||
{ key: 'ись', label: 'a multi-character rewrite' }
|
||||
] as const
|
||||
|
||||
describe('isLatinShortcutKey', () => {
|
||||
it('matches the physical key whatever the IME rewrote key to', () => {
|
||||
for (const { key, label } of CMD_C_BY_INPUT_SOURCE) {
|
||||
expect(isLatinShortcutKey({ key, code: 'KeyC' }, 'c'), label).toBe(true)
|
||||
}
|
||||
})
|
||||
|
||||
it('ordinary negative: a different physical key never matches', () => {
|
||||
// The Korean jamo for the C key must not make the V key match, or the fix would
|
||||
// trade a missed shortcut for a fired-wrong one.
|
||||
expect(isLatinShortcutKey({ key: 'ㅊ', code: 'KeyV' }, 'c')).toBe(false)
|
||||
expect(isLatinShortcutKey({ key: 'c', code: 'KeyV' }, 'c')).toBe(false)
|
||||
expect(isLatinShortcutKey({ key: 'v', code: 'KeyV' }, 'c')).toBe(false)
|
||||
})
|
||||
|
||||
it('falls back to key when the event carries no code', () => {
|
||||
// Chromium omits `code` on synthetic and some keypress events.
|
||||
expect(isLatinShortcutKey({ key: 'c' }, 'c')).toBe(true)
|
||||
expect(isLatinShortcutKey({ key: 'C' }, 'c')).toBe(true)
|
||||
expect(isLatinShortcutKey({ key: '', code: '' }, 'c')).toBe(false)
|
||||
})
|
||||
|
||||
it('falls back to keyCode, which keeps its US value when key is rewritten', () => {
|
||||
expect(isLatinShortcutKey({ key: 'ㅊ', keyCode: 67 }, 'c')).toBe(true)
|
||||
expect(isLatinShortcutKey({ key: 'ㅊ', keyCode: 86 }, 'c')).toBe(false)
|
||||
})
|
||||
|
||||
it('treats a blank code as absent rather than as a non-match', () => {
|
||||
// A whitespace-only code must not be trusted over the logical key.
|
||||
expect(isLatinShortcutKey({ key: 'c', code: ' ' }, 'c')).toBe(true)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,41 @@
|
||||
/**
|
||||
* Does this keyboard event carry the Latin letter a Mod+<letter> shortcut is looking for?
|
||||
*
|
||||
* Why: with a CJK input source active, macOS and Windows report the PHYSICAL key through
|
||||
* `code` but rewrite `key` to the layout's character — Korean 2-Set turns Cmd+C into
|
||||
* `{ key: "ㅊ", code: "KeyC", metaKey: true }`. A `key.toLowerCase() === 'c'` match misses,
|
||||
* the shortcut is not recognised, and xterm encodes the chord as PTY input instead
|
||||
* (issue #12164's sibling #13033: the viewport jumps to the bottom because user input
|
||||
* scrolls the terminal). This is the same key-vs-code confusion that owned #12171, where a
|
||||
* `Shift+T` typing ㅆ was misread as Enter for want of a `code` guard.
|
||||
*
|
||||
* `code` is authoritative when present, so it is checked first. `key` remains the fallback
|
||||
* for events that carry no `code` — Chromium omits it on synthetic and some keypress events.
|
||||
*/
|
||||
export type LatinShortcutKeyEvent = {
|
||||
code?: string
|
||||
key: string
|
||||
keyCode?: number
|
||||
}
|
||||
|
||||
/** `KeyA`-style code for a single Latin letter, e.g. `c` -> `KeyC`. */
|
||||
function physicalCodeForLetter(letter: string): string {
|
||||
return `Key${letter.toUpperCase()}`
|
||||
}
|
||||
|
||||
/**
|
||||
* True when `event` is the physical key for `letter`, whatever the IME rewrote `key` to.
|
||||
* `letter` must be a single a-z character.
|
||||
*/
|
||||
export function isLatinShortcutKey(event: LatinShortcutKeyEvent, letter: string): boolean {
|
||||
const code = event.code?.trim()
|
||||
if (code) {
|
||||
return code === physicalCodeForLetter(letter)
|
||||
}
|
||||
// Why: no `code` to trust, so fall back to the logical key, then to the legacy
|
||||
// keyCode — which stays the US-layout value even when `key` has been rewritten.
|
||||
if (event.key.toLowerCase() === letter) {
|
||||
return true
|
||||
}
|
||||
return event.keyCode === letter.toUpperCase().charCodeAt(0)
|
||||
}
|
||||
Reference in New Issue
Block a user