From d89fdd0ab8a0713d836790e01bebc7de7eb20db7 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 17 May 2026 20:52:28 -0700 Subject: [PATCH] Fix shifted Russian input in kitty terminal mode Fixes shifted non-ASCII keyboard input when kitty keyboard reporting is active, including keyup release handling. --- .../use-terminal-pane-lifecycle.ts | 16 +-- .../terminal-pane/xterm-bypass-policy.test.ts | 128 ++++++++++++------ .../terminal-pane/xterm-bypass-policy.ts | 39 +++++- tests/e2e/terminal-shortcuts.spec.ts | 101 ++++++++++++++ 4 files changed, 232 insertions(+), 52 deletions(-) diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts index 9200ffa3709..55f59a1fe5d 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts @@ -33,7 +33,7 @@ import { } from './terminal-appearance' import { parseOsc52 } from './osc52-clipboard' import { parseOsc7 } from './parse-osc7' -import { shouldBypassXtermKeydown } from './xterm-bypass-policy' +import { shouldBypassXtermKeyboardEvent } from './xterm-bypass-policy' import type { PaneCwdMap } from './resolve-split-cwd' import { installMouseHideWhileTyping } from './mouse-hide-while-typing' import type { EffectiveMacOptionAsAlt } from '@/lib/keyboard-layout/detect-option-as-alt' @@ -472,20 +472,18 @@ export function useTerminalPaneLifecycle({ }) osc7DisposablesRef.current.set(pane.id, osc7Disposable) - // Why: let clipboard chords bypass xterm's kitty CSI-u encoder. + // Why: let host-handled keys bypass xterm's kitty CSI-u encoder. // With vtExtensions.kittyKeyboard on, a CLI that activates progressive // enhancement (Codex does, Claude Code does not) makes xterm encode // Cmd+C as a CSI-u sequence with cancel=true, which preventDefaults // the keydown and suppresses Chromium's native copy event — so the - // selection never reaches the clipboard. Returning false here short- - // circuits xterm's _keyDown before the encoder runs, letting the - // browser copy pipeline and Electron menu accelerators fire normally. + // selection never reaches the clipboard. The same hook also bypasses + // matching keyups so kitty release sequences do not leak after a + // bypassed press. Returning false here short-circuits xterm before the + // encoder runs, letting the browser and Electron paths fire normally. // See xterm-bypass-policy.ts for the rule derivation (Ghostty/VS Code). pane.terminal.attachCustomKeyEventHandler((e) => { - if (e.type !== 'keydown') { - return true - } - return !shouldBypassXtermKeydown(e, { + return !shouldBypassXtermKeyboardEvent(e, { isMac: navigator.userAgent.includes('Mac'), hasSelection: pane.terminal.hasSelection() }) diff --git a/src/renderer/src/components/terminal-pane/xterm-bypass-policy.test.ts b/src/renderer/src/components/terminal-pane/xterm-bypass-policy.test.ts index 35ce7d3a009..f7ccaa057ff 100644 --- a/src/renderer/src/components/terminal-pane/xterm-bypass-policy.test.ts +++ b/src/renderer/src/components/terminal-pane/xterm-bypass-policy.test.ts @@ -1,8 +1,9 @@ import { describe, expect, it } from 'vitest' -import { shouldBypassXtermKeydown, type XtermBypassEvent } from './xterm-bypass-policy' +import { shouldBypassXtermKeyboardEvent, type XtermBypassEvent } from './xterm-bypass-policy' function event(overrides: Partial): XtermBypassEvent { return { + type: 'keydown', key: '', code: '', defaultPrevented: false, @@ -14,7 +15,7 @@ function event(overrides: Partial): XtermBypassEvent { } } -describe('shouldBypassXtermKeydown — macOS', () => { +describe('shouldBypassXtermKeyboardEvent — macOS', () => { const opts = { isMac: true, hasSelection: true } const noSel = { isMac: true, hasSelection: false } @@ -22,15 +23,15 @@ describe('shouldBypassXtermKeydown — macOS', () => { // Why: this is the whole point of the policy. When kitty progressive // enhancement is on, the default xterm path CSI-u encodes Cmd+C and // preventDefaults the keydown, suppressing the browser copy event. - expect(shouldBypassXtermKeydown(event({ key: 'c', code: 'KeyC', metaKey: true }), opts)).toBe( - true - ) + expect( + shouldBypassXtermKeyboardEvent(event({ key: 'c', code: 'KeyC', metaKey: true }), opts) + ).toBe(true) }) it('bubbles Cmd+C even with no selection (no-op copy is harmless on macOS)', () => { - expect(shouldBypassXtermKeydown(event({ key: 'c', code: 'KeyC', metaKey: true }), noSel)).toBe( - true - ) + expect( + shouldBypassXtermKeyboardEvent(event({ key: 'c', code: 'KeyC', metaKey: true }), noSel) + ).toBe(true) }) it('does NOT bubble other Cmd chords — Orca window handlers intercept them before xterm', () => { @@ -47,7 +48,7 @@ describe('shouldBypassXtermKeydown — macOS', () => { event({ key: 't', code: 'KeyT', metaKey: true }) ] for (const e of cases) { - expect(shouldBypassXtermKeydown(e, opts)).toBe(false) + expect(shouldBypassXtermKeyboardEvent(e, opts)).toBe(false) } }) @@ -56,13 +57,13 @@ describe('shouldBypassXtermKeydown — macOS', () => { // propagation. VS Code returns false for resolved Meta keybindings for the // same kitty reason: app shortcuts must not also become terminal input. expect( - shouldBypassXtermKeydown( + shouldBypassXtermKeyboardEvent( event({ key: 'b', code: 'KeyB', defaultPrevented: true, metaKey: true }), opts ) ).toBe(true) expect( - shouldBypassXtermKeydown( + shouldBypassXtermKeyboardEvent( event({ key: 'ArrowLeft', code: 'ArrowLeft', @@ -77,7 +78,7 @@ describe('shouldBypassXtermKeydown — macOS', () => { it('does not bubble Cmd+Shift+C — already intercepted in keyboard-handlers.ts', () => { expect( - shouldBypassXtermKeydown( + shouldBypassXtermKeyboardEvent( event({ key: 'C', code: 'KeyC', metaKey: true, shiftKey: true }), opts ) @@ -86,17 +87,17 @@ describe('shouldBypassXtermKeydown — macOS', () => { it('does not bubble Ctrl chords — those must reach the shell', () => { // Ctrl+C is SIGINT, Ctrl+D is EOF, etc. — xterm must see them. - expect(shouldBypassXtermKeydown(event({ key: 'c', code: 'KeyC', ctrlKey: true }), opts)).toBe( - false - ) - expect(shouldBypassXtermKeydown(event({ key: 'd', code: 'KeyD', ctrlKey: true }), opts)).toBe( - false - ) + expect( + shouldBypassXtermKeyboardEvent(event({ key: 'c', code: 'KeyC', ctrlKey: true }), opts) + ).toBe(false) + expect( + shouldBypassXtermKeyboardEvent(event({ key: 'd', code: 'KeyD', ctrlKey: true }), opts) + ).toBe(false) }) it('does not bubble Cmd+Ctrl combos (unusual; defer to xterm)', () => { expect( - shouldBypassXtermKeydown( + shouldBypassXtermKeyboardEvent( event({ key: 'c', code: 'KeyC', metaKey: true, ctrlKey: true }), opts ) @@ -105,7 +106,7 @@ describe('shouldBypassXtermKeydown — macOS', () => { it('does not bubble already-handled Ctrl chords on macOS', () => { expect( - shouldBypassXtermKeydown( + shouldBypassXtermKeyboardEvent( event({ key: 'c', code: 'KeyC', defaultPrevented: true, ctrlKey: true }), opts ) @@ -113,17 +114,47 @@ describe('shouldBypassXtermKeydown — macOS', () => { }) it('does not bubble plain letters — those are normal input', () => { - expect(shouldBypassXtermKeydown(event({ key: 'c', code: 'KeyC' }), opts)).toBe(false) + expect(shouldBypassXtermKeyboardEvent(event({ key: 'c', code: 'KeyC' }), opts)).toBe(false) + }) + + it('bubbles Shift+non-ASCII printable text so the active keyboard layout wins', () => { + expect( + shouldBypassXtermKeyboardEvent(event({ key: 'Ф', code: 'KeyA', shiftKey: true }), opts) + ).toBe(true) + }) + + it('bubbles Shift+non-ASCII keyup so kitty does not emit a Latin release sequence', () => { + expect( + shouldBypassXtermKeyboardEvent( + event({ type: 'keyup', key: 'Ф', code: 'KeyA', shiftKey: true }), + opts + ) + ).toBe(true) + }) + + it('does not bubble Shift+non-ASCII keypress because that carries the layout text', () => { + expect( + shouldBypassXtermKeyboardEvent( + event({ type: 'keypress', key: 'Ф', code: 'KeyA', shiftKey: true }), + opts + ) + ).toBe(false) + }) + + it('does not bubble Shift+Latin printable text', () => { + expect( + shouldBypassXtermKeyboardEvent(event({ key: 'A', code: 'KeyA', shiftKey: true }), opts) + ).toBe(false) }) }) -describe('shouldBypassXtermKeydown — Windows/Linux', () => { +describe('shouldBypassXtermKeyboardEvent — Windows/Linux', () => { const withSel = { isMac: false, hasSelection: true } const noSel = { isMac: false, hasSelection: false } it('bubbles Ctrl+Shift+C (standard terminal copy on Linux/Windows)', () => { expect( - shouldBypassXtermKeydown( + shouldBypassXtermKeyboardEvent( event({ key: 'C', code: 'KeyC', ctrlKey: true, shiftKey: true }), noSel ) @@ -134,19 +165,19 @@ describe('shouldBypassXtermKeydown — Windows/Linux', () => { // Why: bare Ctrl+C without a selection must reach the shell as SIGINT. // With a selection, terminals like Windows Terminal copy instead. expect( - shouldBypassXtermKeydown(event({ key: 'c', code: 'KeyC', ctrlKey: true }), withSel) + shouldBypassXtermKeyboardEvent(event({ key: 'c', code: 'KeyC', ctrlKey: true }), withSel) ).toBe(true) - expect(shouldBypassXtermKeydown(event({ key: 'c', code: 'KeyC', ctrlKey: true }), noSel)).toBe( - false - ) + expect( + shouldBypassXtermKeyboardEvent(event({ key: 'c', code: 'KeyC', ctrlKey: true }), noSel) + ).toBe(false) }) it('bubbles Ctrl+V and Ctrl+Shift+V for paste', () => { - expect(shouldBypassXtermKeydown(event({ key: 'v', code: 'KeyV', ctrlKey: true }), noSel)).toBe( - true - ) expect( - shouldBypassXtermKeydown( + shouldBypassXtermKeyboardEvent(event({ key: 'v', code: 'KeyV', ctrlKey: true }), noSel) + ).toBe(true) + expect( + shouldBypassXtermKeyboardEvent( event({ key: 'V', code: 'KeyV', ctrlKey: true, shiftKey: true }), noSel ) @@ -155,7 +186,10 @@ describe('shouldBypassXtermKeydown — Windows/Linux', () => { it('bubbles Shift+Insert (X11/Linux paste convention)', () => { expect( - shouldBypassXtermKeydown(event({ key: 'Insert', code: 'Insert', shiftKey: true }), noSel) + shouldBypassXtermKeyboardEvent( + event({ key: 'Insert', code: 'Insert', shiftKey: true }), + noSel + ) ).toBe(true) }) @@ -163,7 +197,7 @@ describe('shouldBypassXtermKeydown — Windows/Linux', () => { // Ctrl+A, Ctrl+E, Ctrl+U, Ctrl+R, Ctrl+L — all readline-critical. for (const keyCode of ['a', 'e', 'u', 'r', 'l']) { expect( - shouldBypassXtermKeydown( + shouldBypassXtermKeyboardEvent( event({ key: keyCode, code: `Key${keyCode.toUpperCase()}`, ctrlKey: true }), noSel ) @@ -173,13 +207,13 @@ describe('shouldBypassXtermKeydown — Windows/Linux', () => { it('bubbles already-handled Ctrl app shortcuts so kitty does not also write to shell', () => { expect( - shouldBypassXtermKeydown( + shouldBypassXtermKeyboardEvent( event({ key: 'b', code: 'KeyB', defaultPrevented: true, ctrlKey: true }), noSel ) ).toBe(true) expect( - shouldBypassXtermKeydown( + shouldBypassXtermKeyboardEvent( event({ key: 'ArrowLeft', code: 'ArrowLeft', @@ -193,12 +227,28 @@ describe('shouldBypassXtermKeydown — Windows/Linux', () => { }) it('does not bubble plain letters', () => { - expect(shouldBypassXtermKeydown(event({ key: 'c', code: 'KeyC' }), noSel)).toBe(false) + expect(shouldBypassXtermKeyboardEvent(event({ key: 'c', code: 'KeyC' }), noSel)).toBe(false) + }) + + it('bubbles Shift+non-ASCII printable text so the active keyboard layout wins', () => { + expect( + shouldBypassXtermKeyboardEvent(event({ key: 'Ф', code: 'KeyA', shiftKey: true }), noSel) + ).toBe(true) + expect( + shouldBypassXtermKeyboardEvent( + event({ type: 'keyup', key: 'Ф', code: 'KeyA', shiftKey: true }), + noSel + ) + ).toBe(true) + }) + + it('does not bubble unshifted non-ASCII printable text', () => { + expect(shouldBypassXtermKeyboardEvent(event({ key: 'ф', code: 'KeyA' }), noSel)).toBe(false) }) it('does not bubble Cmd chords on non-Mac (Super+C has no clipboard meaning there)', () => { - expect(shouldBypassXtermKeydown(event({ key: 'c', code: 'KeyC', metaKey: true }), noSel)).toBe( - false - ) + expect( + shouldBypassXtermKeyboardEvent(event({ key: 'c', code: 'KeyC', metaKey: true }), noSel) + ).toBe(false) }) }) diff --git a/src/renderer/src/components/terminal-pane/xterm-bypass-policy.ts b/src/renderer/src/components/terminal-pane/xterm-bypass-policy.ts index 790281cc702..dde53fb2741 100644 --- a/src/renderer/src/components/terminal-pane/xterm-bypass-policy.ts +++ b/src/renderer/src/components/terminal-pane/xterm-bypass-policy.ts @@ -18,6 +18,7 @@ // this exact bug) both converge on the same pattern. export type XtermBypassEvent = { + type: string key: string code?: string defaultPrevented?: boolean @@ -35,15 +36,32 @@ export type XtermBypassOptions = { hasSelection: boolean } +function isSingleNonAsciiPrintableText(key: string): boolean { + const chars = Array.from(key) + if (chars.length !== 1) { + return false + } + const codePoint = chars[0].codePointAt(0) + return codePoint !== undefined && codePoint >= 0x80 +} + +function isXtermHandledKeyEvent(type: string): boolean { + return type === 'keydown' || type === 'keyup' +} + /** - * Decide whether a chord should bypass xterm's keydown handler so the native - * browser pipeline (Chromium `copy` event, Electron menu accelerators) can - * handle it instead of the kitty CSI-u encoder swallowing it. + * Decide whether a chord should bypass xterm's key handlers so the native + * browser pipeline (Chromium `copy` event, Electron menu accelerators) or + * layout-aware text event can handle it instead of the kitty CSI-u encoder. */ -export function shouldBypassXtermKeydown( +export function shouldBypassXtermKeyboardEvent( event: XtermBypassEvent, options: XtermBypassOptions ): boolean { + if (!isXtermHandledKeyEvent(event.type)) { + return false + } + const { isMac, hasSelection } = options const platformModifierHeld = isMac ? event.metaKey && !event.ctrlKey @@ -56,6 +74,19 @@ export function shouldBypassXtermKeydown( return true } + if ( + event.shiftKey && + !event.ctrlKey && + !event.metaKey && + !event.altKey && + isSingleNonAsciiPrintableText(event.key) + ) { + // Why: xterm's kitty encoder derives shifted key codes from physical + // `code` (KeyA -> Latin "a"). Bypass keydown so Chromium emits layout text + // via keypress, and bypass keyup so xterm doesn't leak the release CSI-u. + return true + } + if (isMac) { // Narrow Ghostty rule to Cmd+C only: Ghostty bubbles every Cmd chord on // macOS, but Orca's window-level handlers (keyboard-handlers.ts, diff --git a/tests/e2e/terminal-shortcuts.spec.ts b/tests/e2e/terminal-shortcuts.spec.ts index 9f53ac55724..f667005094c 100644 --- a/tests/e2e/terminal-shortcuts.spec.ts +++ b/tests/e2e/terminal-shortcuts.spec.ts @@ -74,6 +74,81 @@ async function focusActiveTerminal(page: Page): Promise { }) } +async function enableKittyKeyboardReporting(page: Page, flags: number): Promise { + await page.evaluate(async (flags) => { + const state = window.__store?.getState() + const worktreeId = state?.activeWorktreeId + const tabId = + state?.activeTabType === 'terminal' + ? state.activeTabId + : worktreeId + ? (state?.activeTabIdByWorktree?.[worktreeId] ?? null) + : null + const manager = tabId ? window.__paneManagers?.get(tabId) : null + const pane = manager?.getActivePane?.() ?? manager?.getPanes?.()[0] ?? null + if (!pane) { + throw new Error('No active terminal pane for kitty keyboard setup') + } + await new Promise((resolve) => { + pane.terminal.write(`\x1b[=${flags}u`, resolve) + }) + }, flags) +} + +async function pressShiftedRussianLayoutKey(page: Page): Promise<{ + keydownDefaultPrevented: boolean + keypressSent: boolean + keyupSent: boolean +}> { + return page.evaluate(() => { + const textarea = document.querySelector('.xterm-helper-textarea') as HTMLTextAreaElement | null + if (!textarea) { + throw new Error('No xterm helper textarea to receive keyboard input') + } + textarea.focus() + + const keydown = new KeyboardEvent('keydown', { + key: 'Ф', + code: 'KeyA', + shiftKey: true, + bubbles: true, + cancelable: true + }) + Object.defineProperty(keydown, 'keyCode', { get: () => 65 }) + Object.defineProperty(keydown, 'which', { get: () => 65 }) + textarea.dispatchEvent(keydown) + + if (keydown.defaultPrevented) { + return { keydownDefaultPrevented: true, keypressSent: false, keyupSent: false } + } + + const keypress = new KeyboardEvent('keypress', { + key: 'Ф', + code: 'KeyA', + shiftKey: true, + bubbles: true, + cancelable: true + }) + Object.defineProperty(keypress, 'keyCode', { get: () => 1060 }) + Object.defineProperty(keypress, 'charCode', { get: () => 1060 }) + Object.defineProperty(keypress, 'which', { get: () => 1060 }) + textarea.dispatchEvent(keypress) + + const keyup = new KeyboardEvent('keyup', { + key: 'Ф', + code: 'KeyA', + shiftKey: true, + bubbles: true, + cancelable: true + }) + Object.defineProperty(keyup, 'keyCode', { get: () => 65 }) + Object.defineProperty(keyup, 'which', { get: () => 65 }) + textarea.dispatchEvent(keyup) + + return { keydownDefaultPrevented: false, keypressSent: true, keyupSent: true } + }) +} + // Why: handleRequestClosePane pops a "Close Terminal?" dialog when the pane // reports a running child process. Under E2E, a freshly split pane's // proc.process is briefly unset so the check returns true spuriously. Click @@ -271,4 +346,30 @@ test.describe('Terminal Shortcuts', () => { timeout: 3_000 }) }) + + test('Shift with Russian layout text reaches the PTY as Cyrillic under kitty keyboard reporting', async ({ + orcaPage, + electronApp + }) => { + await installMainProcessPtyWriteSpy(electronApp) + await enableKittyKeyboardReporting(orcaPage, 31) + await clearPtyWriteLog(electronApp) + + const dispatch = await pressShiftedRussianLayoutKey(orcaPage) + + expect(dispatch).toEqual({ + keydownDefaultPrevented: false, + keypressSent: true, + keyupSent: true + }) + await expect + .poll(async () => (await getPtyWrites(electronApp)).includes('Ф'), { + timeout: 5_000, + message: 'Shift+Russian layout text did not reach the PTY as Cyrillic' + }) + .toBe(true) + const writes = await getPtyWrites(electronApp) + expect(writes).not.toContain('\x1b[97:1060;2;1060u') + expect(writes).not.toContain('\x1b[97:1060;2:3u') + }) })