Skip to content

Keep AltGr text out of terminal application shortcuts - #681

Merged
coneilen merged 2 commits into
mainfrom
coneilen-fix-terminal-mouse-altgr-paste-safety
Oct 10, 2026
Merged

coneilen merged 2 commits into
mainfrom
coneilen-fix-terminal-mouse-altgr-paste-safety

Conversation

@coneilen

@coneilen coneilen commented Oct 10, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

On Windows, AltGr reaches the shell as Ctrl+Alt. TerminalSurface.onKey decided whether a key was an application shortcut from Ctrl and Shift alone, so AltGr+O, AltGr+J and AltGr+comma ran the plain-Ctrl shortcut (open folder, jump, settings) instead of typing the layout's character. This keeps every Ctrl+Alt chord except Ctrl+Alt+Tab out of the terminal application shortcuts, so its text reaches the terminal. It also narrows two code comments that claimed more than is implemented. It is one of a small series that follows up the review of #676, #677 and #679; it touches no mouse, paste or focus code.

Verified against the code before changing it: TerminalKeyEncoding.encodesAltChord already left Ctrl+Alt alone, but isApplicationShortcut (TerminalSurface.zig) took only ctrl and shift, so onKey called the shortcut route first and returned before the WM_CHAR text path mattered.

Changes

  • isApplicationShortcut takes alt and returns false for any Ctrl+Alt chord; onKey passes it. Ctrl+Alt+PageUp/PageDown stay with the window's own accelerators (handled before the key reaches the terminal); a Ctrl+Alt+PageUp that does reach the terminal now goes to the key encoder instead of selecting the previous tab (the bytes the encoder produces for it are not asserted by any test).
  • Ctrl+Tab, Ctrl+Alt+Tab and the Alt-modified Tab route are unchanged: isApplicationShortcut still returns on VK_TAB before it looks at Alt, and onKey routes Alt+Tab separately, so Ctrl+Alt+Tab remains an application shortcut. Windows itself normally takes Alt+Tab before the terminal sees it, and no layout types text on it.
  • TerminalKeyEncoding.zig doc comments no longer claim the encoder consumes Kitty keyboard flags or modifyOtherKeys: they now say only application cursor keys is tested, releases never reach the encoder, and of the keypad keys only numpad Enter is told apart. onKey carries a matching one-line note on key releases. No behavior change from the comment edits.
  • Not changed: Ctrl+S (see Send F6 and F10 to a focused Windows terminal #680), the Tab handling, mouse, paste and focus.

What each test proves

  • AltGr (Ctrl+Alt) never triggers an application shortcut, so its text reaches the terminal (TerminalSurface.zig): calls the production TerminalSurface.onKey callback directly (not the window or accelerator path) with Ctrl+Alt on ten virtual keys modelled on Polish (O, A, L), German (Q, 7, E), French (0, J), Spanish (2) and an AltGr+comma layout, asserts the shortcut callback is not called and nothing is queued, then calls onText with the layout's character and asserts exactly those bytes are queued. The WM_CHAR text is supplied by the test, not produced by a layout.
  • Ctrl+Alt held on every application-shortcut key leaves the key to the terminal: the same for every key isApplicationShortcut accepts (O, J, comma, PageUp, PageDown, Ctrl+Shift+[ and ]), with and without Shift; and plain Ctrl on the same keys still reaches the shortcut route.
  • TerminalSurface.isApplicationShortcut rejects every shortcut key once Alt is held with Ctrl (AltGr): the predicate alone.
  • live terminal keyboard: AltGr characters reach the shell and never run a shell shortcut (App.zig): a real Winghostty surface window; a plain WM_KEYDOWN with Ctrl and Alt set via SetKeyboardState goes through MainWindow.dispatchMessage and the production accelerator table, then WM_CHAR is sent to the surface window; asserts no shortcut ran and the character's UTF-8 bytes arrive in the terminal input queue (6 cases). Limit: it does not prove that the window accelerators declined the chords. Any characters TranslateMessage queued are discarded and the layout character is injected by the test, and the test observes only the surface callback and the input queue, not accelerator or app state. The accelerator table has no Ctrl+Alt entry for these keys (only Ctrl+Alt+PageUp/PageDown), which is read from the table, not asserted by this test.

Which tests were RED before the fix (run against the unfixed onKey): both new TerminalSurface.zig behavior tests and the live test. The predicate test passes on the unfixed onKey, because it calls the already-changed predicate directly; it is a unit test of the new signature, not a RED test.

What is NOT tested

  • No German, French, Polish or Spanish keyboard layout is activated: the test machine's own layout translates nothing as AltGr, so TranslateMessage output is discarded and the layout's character is posted by hand. A real AltGr key sequence, including the fake left Ctrl key event Windows sends first, is not exercised.
  • No conhost, pwsh, ConPTY, zmx or Dev Box was involved; the fixture uses the in-repo fake zmx stand-in and in-process synthetic messages. No physical keyboard.
  • Dead-key and IME paths were not changed; the existing dead-key/AltGr (German AltGr+Q) live test still passes but did not catch this defect because Q is not a shortcut key.
  • Ctrl+Alt+Tab is left as it was.
  • Isolation: the tests ran with process-only USERPROFILE, APPDATA, LOCALAPPDATA, TEMP, TMP and GRAPHCODE_SUPPORT_DIR in a scratch profile; no installed daemon or per-user zmx was used.

Test plan

RED: zig test src\TerminalSurface.zig --test-filter "Ctrl+Alt" (pinned target and link flags, unfixed onKey) -> 0 passed, 2 failed: Polish AltGr vk 0x4f: 1 shortcut calls; expected 0, found 1 (and expected 0, found 12 for every shortcut key); the live App test also failed with AltGr vk 0x4f ran shortcut key 79
GREEN: zig test src\App.zig --test-filter "AltGr" (pinned flags, with the fix) -> All 4 tests passed; zig test src\TerminalSurface.zig --test-filter "Ctrl+Alt" -> the two behavior tests pass
REGRESSION: zig test for TerminalKeys, TerminalVt, TerminalKeyEncoding, TerminalSelection, Clipboard, TerminalSurface and App roots (pinned flags) -> 8, 22, 36, 35, 4, 223 and 1001 tests passed, 0 failed (baseline before the change: TerminalSurface 220, App 997)

Checklist

  • I have read the Contributing Guidelines
  • I have signed off my commits (git commit -s) per the DCO
  • Tests pass locally (make test) - macOS target, not run: this change is Windows-only; the Windows roots above were run instead
  • Code follows the existing style (make check) - macOS lint target, not run: Windows-only change in Zig
  • I added the test/contract before the implementation and observed the intended RED failure

coneilen and others added 2 commits October 9, 2026 23:11
Windows reports AltGr as Ctrl+Alt. TerminalSurface.onKey decided whether a
key was an application shortcut from Ctrl and Shift alone, so AltGr+O, AltGr+J
and AltGr+comma (and Ctrl+Alt+PageUp/PageDown, and Ctrl+Alt+[ or ] with Shift)
ran the Ctrl shortcut instead of typing the layout's character.
isApplicationShortcut now also takes Alt and never matches a Ctrl+Alt chord.

Also narrow comments that claimed more than is implemented: the key encoder
documents that only application cursor keys is exercised, that releases never
reach it, and that only numpad Enter is told apart among keypad keys.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Colin Neilens <coneilen@microsoft.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Colin Neilens <coneilen@microsoft.com>
@coneilen
coneilen merged commit 586e11d into main Oct 10, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant