From 5f799cd4fa54129d75c7fcfb12f440717f97f380 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 1 Aug 2026 22:15:24 -0700 Subject: [PATCH] Separate the terminal IME platform checks The candidate-key guards and the standalone 229 policy were reading one flag, so tightening either one moved both. Derive them together instead. Co-authored-by: Orca --- .../terminal-ime-platform.test.ts | 47 +++++++++++++++++++ .../terminal-pane/terminal-ime-platform.ts | 31 ++++++++++++ .../use-terminal-pane-lifecycle.ts | 13 ++--- .../xterm-bypass-policy-non-mac.test.ts | 26 ++++++++++ .../terminal-pane/xterm-bypass-policy.ts | 16 +++++-- 5 files changed, 121 insertions(+), 12 deletions(-) create mode 100644 src/renderer/src/components/terminal-pane/terminal-ime-platform.test.ts create mode 100644 src/renderer/src/components/terminal-pane/terminal-ime-platform.ts diff --git a/src/renderer/src/components/terminal-pane/terminal-ime-platform.test.ts b/src/renderer/src/components/terminal-pane/terminal-ime-platform.test.ts new file mode 100644 index 00000000000..5b98ed646e9 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-ime-platform.test.ts @@ -0,0 +1,47 @@ +import { describe, expect, it } from 'vitest' +import { resolveTerminalImePlatform } from './terminal-ime-platform' + +const DESKTOP_LINUX = + 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/140.0.0.0 Safari/537.36' +const CHROME_OS = + 'Mozilla/5.0 (X11; CrOS x86_64 14541.0.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/140.0.0.0 Safari/537.36' +const ANDROID = + 'Mozilla/5.0 (Linux; Android 14; Pixel 8) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/140.0.0.0 Mobile Safari/537.36' +const MACOS = + 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/140.0.0.0 Safari/537.36' +const WINDOWS = + 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/140.0.0.0 Safari/537.36' + +describe('resolveTerminalImePlatform', () => { + it('treats desktop Linux as both Chromium-family and ibus/fcitx-bearing', () => { + expect(resolveTerminalImePlatform(DESKTOP_LINUX)).toEqual({ + isMac: false, + isLinux: true, + isDesktopLinux: true + }) + }) + + // The split exists for these two: their UA says Linux, so they share the 229 behavior, + // but neither runs the input methods the candidate-key guards are written against. + it.each([ + ['ChromeOS', CHROME_OS], + ['Android', ANDROID] + ])('keeps %s in the Chromium family but out of the desktop guards', (_name, userAgent) => { + expect(resolveTerminalImePlatform(userAgent)).toEqual({ + isMac: false, + isLinux: true, + isDesktopLinux: false + }) + }) + + it.each([ + ['macOS', MACOS, true], + ['Windows', WINDOWS, false] + ])('reports %s as neither Linux flavour', (_name, userAgent, isMac) => { + expect(resolveTerminalImePlatform(userAgent)).toEqual({ + isMac, + isLinux: false, + isDesktopLinux: false + }) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/terminal-ime-platform.ts b/src/renderer/src/components/terminal-pane/terminal-ime-platform.ts new file mode 100644 index 00000000000..ea7765e937f --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-ime-platform.ts @@ -0,0 +1,31 @@ +export type TerminalImePlatform = { + isMac: boolean + /** Chromium on a Linux kernel — ChromeOS and Android included. */ + isLinux: boolean + /** Desktop Linux only — the platforms actually running ibus or fcitx. */ + isDesktopLinux: boolean +} + +// ChromeOS reports `X11; CrOS x86_64`, with no "Linux" anywhere in it, so a plain +// substring test puts it on the Windows side of every policy below. +const LINUX_KERNEL_PLATFORM = /Linux|CrOS/ +const NON_DESKTOP_LINUX_PLATFORM = /Android|CrOS/ + +/** + * Splits the two platform questions the terminal's IME policy asks, which are not the + * same question and had been sharing one answer. + * + * Standalone keydown 229 is Chromium-family behavior, so ChromeOS and Android belong + * with desktop Linux. The candidate-key guards read ibus/fcitx specifics that neither + * shares, so they are scoped tighter. Deriving both here keeps one user agent from + * getting two different platform verdicts in two different files. + */ +export function resolveTerminalImePlatform(userAgent: string): TerminalImePlatform { + const isMac = userAgent.includes('Mac') + const isLinux = !isMac && LINUX_KERNEL_PLATFORM.test(userAgent) + return { + isMac, + isLinux, + isDesktopLinux: isLinux && !NON_DESKTOP_LINUX_PLATFORM.test(userAgent) + } +} 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 92f877819dc..5e13b7b33c0 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 @@ -79,6 +79,7 @@ import { parseOsc7 } from './parse-osc7' import { guardParserHandler } from './terminal-parser-handler-guard' import { resolveTerminalJisYenInput } from './terminal-jis-yen-input' import { installTerminalImeCompositionTracker } from './terminal-ime-composition-tracker' +import { resolveTerminalImePlatform } from './terminal-ime-platform' import { installTerminalImeLinuxCandidateState } from './terminal-ime-linux-candidate-state' import { armTerminalImePendingCandidateKeyRelease, @@ -887,13 +888,8 @@ export function useTerminalPaneLifecycle({ let pendingTerminalInterruptKeyup = false const pendingTerminalImeCandidateKeyReleases = createTerminalImePendingCandidateKeyReleases() - const isMac = navigator.userAgent.includes('Mac') - // Why: Android/ChromeOS UAs also contain "Linux"; scope the fcitx candidate-key policy to desktop Linux. - const isLinux = - !isMac && - navigator.userAgent.includes('Linux') && - !/Android|CrOS/.test(navigator.userAgent) - const linuxImeCandidateState = isLinux + const { isMac, isLinux, isDesktopLinux } = resolveTerminalImePlatform(navigator.userAgent) + const linuxImeCandidateState = isDesktopLinux ? installTerminalImeLinuxCandidateState(pane.terminal.element) : null const macNativeTextInputSourceTracker = isMac ? getMacNativeTextInputSourceTracker() : null @@ -943,7 +939,8 @@ export function useTerminalPaneLifecycle({ linuxOrphanCandidateDigitGuardActive: linuxCandidateClassification.candidateDigitGuardActive, isMac, - isLinux + isLinux, + isDesktopLinux } if (shouldSuppressTerminalImeKeyboardEvent(e, imeKeyboardOptions)) { // Why: clear before arm so a fresh keydown drops any stale pending release before arming its own. diff --git a/src/renderer/src/components/terminal-pane/xterm-bypass-policy-non-mac.test.ts b/src/renderer/src/components/terminal-pane/xterm-bypass-policy-non-mac.test.ts index 2124ce1ad05..9365f3592a6 100644 --- a/src/renderer/src/components/terminal-pane/xterm-bypass-policy-non-mac.test.ts +++ b/src/renderer/src/components/terminal-pane/xterm-bypass-policy-non-mac.test.ts @@ -215,6 +215,32 @@ describe('shouldSuppressTerminalImeKeyboardEvent — Windows/Linux', () => { ).toBe(false) }) + // A Chromebook's UA contains "Linux", so it is the same Chromium/229 platform, but it + // is not running ibus/fcitx. The two policies need different answers for it. + const chromeOsIdle = { ...linuxIdle, isDesktopLinux: false } + + it('lets standalone ChromeOS 229 keydowns reach xterm, as on desktop Linux', () => { + expect( + shouldSuppressTerminalImeKeyboardEvent( + event({ key: 'Process', code: 'KeyN', keyCode: 229 }), + chromeOsIdle + ) + ).toBe(false) + }) + + it('does not apply the fcitx candidate-key guards on ChromeOS', () => { + const guarded = { ...chromeOsIdle, candidateKeyGuardActive: true } + expect( + shouldPreventDefaultTerminalImeCandidateKey(event({ key: '1', code: 'Digit1' }), guarded) + ).toBe(false) + expect( + shouldPreventDefaultTerminalImeCandidateKey(event({ key: '1', code: 'Digit1' }), { + ...guarded, + isDesktopLinux: true + }) + ).toBe(true) + }) + it('suppresses Linux 229 keydowns while the composition tracker is active', () => { expect( shouldSuppressTerminalImeKeyboardEvent( 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 faf08fb4bff..dbbaf1fa3eb 100644 --- a/src/renderer/src/components/terminal-pane/xterm-bypass-policy.ts +++ b/src/renderer/src/components/terminal-pane/xterm-bypass-policy.ts @@ -54,7 +54,12 @@ export type XtermImeKeyboardOptions = { // Required Linux/Windows split: Linux passes standalone 229 keydowns like // macOS; the Windows-only suppression guards its preedit-diff race (preedit // can hit the textarea before compositionstart and be flushed by the diff). + // Chromium-family, so ChromeOS and Android count — they share that behavior. isLinux: boolean + /** Desktop Linux only. The candidate-key guards read ibus/fcitx behavior that + * a Chromebook's IME does not share, so they are scoped tighter than the 229 + * policy above. Defaults to `isLinux` for callers on a known desktop. */ + isDesktopLinux?: boolean } export const TERMINAL_INTERRUPT_INPUT = '\x03' @@ -114,12 +119,15 @@ export function shouldSuppressTerminalImeKeyboardEvent( pendingCandidateKeyReleaseActive, linuxOrphanCandidateDigitGuardActive = false, isMac, - isLinux + isLinux, + isDesktopLinux = isLinux } = options const suppressOrphanCandidateDigit = - isLinux && linuxOrphanCandidateDigitGuardActive && isTerminalImeCandidateDigitKeyEvent(event) + isDesktopLinux && + linuxOrphanCandidateDigitGuardActive && + isTerminalImeCandidateDigitKeyEvent(event) const suppressCandidateKey = - isLinux && + isDesktopLinux && (pendingCandidateKeyReleaseActive || (candidateKeyGuardActive && isTerminalImeCandidateSelectionKeyEvent(event)) || suppressOrphanCandidateDigit) @@ -162,7 +170,7 @@ export function shouldPreventDefaultTerminalImeCandidateKey( // the leaked selector to the PTY. return ( event.type === 'keydown' && - options.isLinux && + (options.isDesktopLinux ?? options.isLinux) && ((options.candidateKeyGuardActive && isTerminalImeCandidateSelectionKeyEvent(event)) || (options.linuxOrphanCandidateDigitGuardActive === true && isTerminalImeCandidateDigitKeyEvent(event)))