From 8761c76337060a07c412fc78649eb2d9cc506794 Mon Sep 17 00:00:00 2001 From: brennanb2025 Date: Sat, 27 Jun 2026 21:54:33 -0700 Subject: [PATCH] Fix Ctrl+C copy in Markdown Source (Monaco) mode on non-Latin layouts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Shortcut matching trusted the produced logical event.key for letter, digit, and punctuation chords, with a physical-code fallback gated to only empty/Dead/Unidentified keys. On non-Latin layouts (Cyrillic, Greek, ABNT2, etc.) a physical letter key reports a non-Latin character (e.g. physical C -> 'с'), so Ctrl/Meta chords like copy never matched and could not be re-bound in Settings. Add a non-mac physical-code fallback that activates only when no logical shortcut token exists, a real primary modifier (Ctrl or Meta) is held, and the event is not AltGr (Ctrl+Alt) text composition. Latin layouts (including Dvorak remaps) keep matching by logical key unchanged. Fixes #6274 --- src/shared/keybindings.test.ts | 65 ++++++++++++++++++++++++++++++ src/shared/keybindings.ts | 72 +++++++++++++++++++++++++++++----- 2 files changed, 127 insertions(+), 10 deletions(-) diff --git a/src/shared/keybindings.test.ts b/src/shared/keybindings.test.ts index e69917db03e..6dcfcbba44f 100644 --- a/src/shared/keybindings.test.ts +++ b/src/shared/keybindings.test.ts @@ -833,6 +833,71 @@ describe('keybindings', () => { }) }) + it('matches letter shortcuts on non-Latin layouts via the physical code (issue #6274)', () => { + // Cyrillic ЙЦУКЕН: physical C produces the logical key 'с' (Cyrillic es, + // U+0441) while code stays 'KeyC'. The produced character is not a Latin + // shortcut letter, so the chord must still match through the physical code. + const cyrillicCtrlC = { + key: 'с', + code: 'KeyC', + control: true, + meta: false, + alt: false, + shift: false + } + expect(keybindingMatchesAction('browser.grabElement', cyrillicCtrlC, 'win32')).toBe(true) + expect(keybindingMatchesAction('browser.grabElement', cyrillicCtrlC, 'linux')).toBe(true) + + // Ctrl+Shift+C on the same layout (terminal copy) must match too. + expect( + keybindingMatchesAction( + 'terminal.copySelection', + { ...cyrillicCtrlC, shift: true }, + 'win32' + ) + ).toBe(true) + + // Greek layout: physical P produces 'π' (U+03C0); Ctrl+P must still match. + expect( + keybindingMatchesAction( + 'worktree.quickOpen', + { key: 'π', code: 'KeyP', control: true, meta: false, alt: false, shift: false }, + 'win32' + ) + ).toBe(true) + + // The fallback must not steal a different physical key: Ctrl+V (physical V, + // Cyrillic 'м') is not Ctrl+C, so grabElement must stay unmatched. + expect( + keybindingMatchesAction( + 'browser.grabElement', + { key: 'м', code: 'KeyV', control: true, meta: false, alt: false, shift: false }, + 'win32' + ) + ).toBe(false) + }) + + it('does not let non-Latin physical fallback hijack AltGr text input (issue #6274)', () => { + // Windows/Linux AltGr arrives as Ctrl+Alt. A composed character typed via + // AltGr (e.g. AltGr+C) must remain text input, never an app shortcut. + // editor.copyContext is Mod+Alt+C, so the modifier state otherwise matches — + // only the AltGr key gating may keep this from firing. + expect( + keybindingMatchesAction( + 'editor.copyContext', + { + key: '¢', + code: 'KeyC', + control: true, + meta: false, + alt: true, + shift: false + }, + 'win32' + ) + ).toBe(false) + }) + it('uses shifted punctuation aliases only while Shift is pressed', () => { const shiftedComma = { key: '<', diff --git a/src/shared/keybindings.ts b/src/shared/keybindings.ts index 6d4c3e64fb7..e5f649c4f97 100644 --- a/src/shared/keybindings.ts +++ b/src/shared/keybindings.ts @@ -1483,6 +1483,51 @@ function canUsePhysicalCodeFallback(input: KeybindingInput): boolean { return PHYSICAL_CODE_FALLBACK_KEYS.has(input.key ?? '') } +function isLatinShortcutKey(key: string): boolean { + // Why: A-Z / 0-9 are the only single chars a Latin shortcut token names; any + // other produced character (Cyrillic с, Greek π, ...) cannot be a deliberate + // remap of a Latin chord, so it is safe to fall back to the physical code. + if (key.length !== 1) { + return false + } + const upper = key.toUpperCase() + return (upper >= 'A' && upper <= 'Z') || (key >= '0' && key <= '9') +} + +function shouldUseNonLatinShortcutPhysicalFallback( + input: KeybindingInput, + platform: NodeJS.Platform +): boolean { + // Why: non-Latin layouts (Cyrillic, Greek, ...) report a non-Latin logical key + // for physical letter keys (issue #6274), so Ctrl/Meta shortcuts never match. + // Fall back to the physical code, but only when no logical shortcut token + // exists, a real primary modifier (Ctrl or Meta) is held, and this is not an + // AltGr (Ctrl+Alt) text-composition event. macOS is excluded — it already has + // dedicated Option-composed fallbacks and Cmd shortcuts are layout-stable. + if (getKeybindingPlatform(platform) === 'darwin') { + return false + } + const hasPrimaryModifier = hasModifier(input, 'control') || hasModifier(input, 'meta') + if (!hasPrimaryModifier) { + return false + } + // AltGr surfaces as Ctrl+Alt on Windows/Linux; treat it as text, not a chord. + if (hasModifier(input, 'control') && hasModifier(input, 'alt')) { + return false + } + if (logicalKeyTokenFromInput(input) !== null) { + return false + } + const key = input.key ?? '' + return key !== '' && !MODIFIER_KEYS.has(key) && !isLatinShortcutKey(key) +} + +function canFallBackToPhysicalCode(input: KeybindingInput, platform: NodeJS.Platform): boolean { + return ( + canUsePhysicalCodeFallback(input) || shouldUseNonLatinShortcutPhysicalFallback(input, platform) + ) +} + function physicalCodeKeyTokenFromInput(input: KeybindingInput): string | null { const code = input.code ?? '' if (code.startsWith('Key') && code.length === 4) { @@ -1534,7 +1579,8 @@ function keyTokenFromInput(input: KeybindingInput, platform: NodeJS.Platform): s } if ( !canUsePhysicalCodeFallback(input) && - !shouldUseMacOptionComposedCaptureFallback(input, platform) + !shouldUseMacOptionComposedCaptureFallback(input, platform) && + !shouldUseNonLatinShortcutPhysicalFallback(input, platform) ) { return null } @@ -1764,18 +1810,22 @@ function letterKeyMatches( return logicalKey === letter.toUpperCase() } return ( - (canUsePhysicalCodeFallback(input) || + (canFallBackToPhysicalCode(input, platform) || shouldUseMacOptionLetterPhysicalFallback(parsed, input, platform)) && input.code === `Key${letter.toUpperCase()}` ) } -function digitKeyMatches(input: KeybindingInput, digit: string): boolean { +function digitKeyMatches( + input: KeybindingInput, + digit: string, + platform: NodeJS.Platform +): boolean { const logicalKey = logicalKeyTokenFromInput(input) if (logicalKey && logicalKey.length === 1 && logicalKey >= '0' && logicalKey <= '9') { return logicalKey === digit } - return canUsePhysicalCodeFallback(input) && input.code === `Digit${digit}` + return canFallBackToPhysicalCode(input, platform) && input.code === `Digit${digit}` } function isPunctuationKeyToken(token: string | null): token is string { @@ -1823,7 +1873,7 @@ function keyMatches( return letterKeyMatches(input, parsedKey, parsed, platform) } if (parsedKey.length === 1 && parsedKey >= '0' && parsedKey <= '9') { - return digitKeyMatches(input, parsedKey) + return digitKeyMatches(input, parsedKey, platform) } if (parsedKey === 'NumpadAdd' || parsedKey === 'NumpadSubtract') { @@ -1844,7 +1894,7 @@ function keyMatches( return semanticKey === parsedKey } return ( - (canUsePhysicalCodeFallback(input) || + (canFallBackToPhysicalCode(input, platform) || shouldUseMacOptionPunctuationPhysicalFallback(parsed, input, platform)) && physicalPunctuationKey(input) === parsedKey ) @@ -1854,7 +1904,9 @@ function keyMatches( if (logicalKey !== null) { return logicalKey === parsedKey } - return canUsePhysicalCodeFallback(input) && physicalCodeKeyTokenFromInput(input) === parsedKey + return ( + canFallBackToPhysicalCode(input, platform) && physicalCodeKeyTokenFromInput(input) === parsedKey + ) } function resolveModifierToken( @@ -1928,10 +1980,10 @@ export function keybindingMatchesAction( ) } -function digitFromInput(input: KeybindingInput): string | null { +function digitFromInput(input: KeybindingInput, platform: NodeJS.Platform): string | null { for (let value = 1; value <= 9; value++) { const digit = String(value) - if (digitKeyMatches(input, digit)) { + if (digitKeyMatches(input, digit, platform)) { return digit } } @@ -1954,7 +2006,7 @@ export function matchKeybindingDigitIndex( if (!definition || !keybindingIsActiveInContext(definition, options)) { return null } - const digit = digitFromInput(input) + const digit = digitFromInput(input, platform) if (!digit) { return null }