fix(terminal): decline a text substitution instead of dropping the keystroke

macOS rewrites a double space into ". " while the user is typing at a
shell prompt. Terminal.app, iTerm2, Ghostty and Warp all ignore this;
Orca is the only one that reacted to it, and it reacted by losing the
keystroke entirely. The pty received "ab " where every other terminal
sends "ab  ".

forwardCommittedText retired the pending commit on any non-insertText
input, so the deleteContentBackward half settled the claim with nothing
and the replacing insertText that followed found no claim and returned.
Zero bytes for the whole keystroke, and xterm never saw the keydown
either because the forwarder had already claimed it.

A deletion no longer retires the claim, and a keystroke the text system
rewrote delivers the character the key produced rather than the
replacement. Two spaces stay two spaces, "--" stays "--", and a text
replacement like "omw " stays "omw ".

The discriminator is structural rather than a list of characters: a
one-key-one-character substitution from an input source never deletes
first. So layout substitution keeps working untouched - the
backslash-position case, full-width digits, telex - while the
text-editing rules that rewrite what is already on the wire are
declined. That distinction is what the file previously lacked, and it
conflated the two.

Declining is the whole fix, so the retraction accounting an earlier pass
added is gone with the module that held it. The forwarder ends up
smaller than it started.

autocorrect, autocapitalize and spellcheck are already off on the helper
textarea and do not prevent this; they govern the browser's own
correction, not the system replacement service. A native terminal
escapes by not being a text view, which a browser-based one cannot do.

Coverage for the boundary is restored headlessly, as CDP-synthesised
arms on the normal shards asserting bytes the pty child receives.

Refs #11504
This commit is contained in:
Neil
2026-08-18 02:09:25 -07:00
parent 96d88dd5fe
commit 2bf7d44f65
5 changed files with 490 additions and 3 deletions
+2
View File
@@ -616,6 +616,8 @@ jobs:
'tests/e2e/terminal-ime-exact-byte.spec.ts' \
'tests/e2e/terminal-korean-composing-chord-order.spec.ts' \
'tests/e2e/terminal-korean-midline-preedit-occlusion.spec.ts' \
'tests/e2e/terminal-macos-system-key-remap.spec.ts' \
'tests/e2e/terminal-macos-text-substitution.spec.ts' \
'tests/e2e/terminal-korean-preedit-visibility.spec.ts' | sort -u)"
fi
TEST_FILES_JSON="$(printf '%s\n' "$TEST_FILES" | jq --raw-input --slurp --compact-output 'split("\n") | map(select(length > 0))')"
@@ -31,6 +31,25 @@ type PendingCommit = {
press: ClaimedKeyPress
/** Non-null when `keyup` preceded `insertText` — an order some macOS IMEs use. */
keyup: ImeReleaseKeyEvent | null
/**
* Set when the text system deleted text it had already committed before
* delivering this keystroke's own — the shape of a text-*editing* convenience
* like macOS automatic period substitution rewriting `" "` into `". "`, or the
* dash, quote and text-replacement rules in the same family.
*
* That is a different thing from an input source mapping one physical key to
* one different character, which is what this forwarder exists to carry.
* Nothing here honours the editing kind: no reference terminal does, because
* none of them is a text view, so the substitution never reaches them and two
* spaces stay two spaces. A browser-based terminal cannot opt out that way —
* `autocorrect` and `spellcheck` on the helper textarea govern the browser's
* own correction, not the system text service — so the rewrite is declined
* here instead and the key sends what it produced.
*
* The discriminator is structural, not a character table: a
* one-key-one-character substitution never deletes first.
*/
rewritten: boolean
}
/**
@@ -265,7 +284,8 @@ export function installTerminalImeNativeTextForwarder(args: {
capsLock: event.getModifierState?.('CapsLock') === true,
numLock: event.getModifierState?.('NumLock') === true
},
keyup: null
keyup: null,
rewritten: false
}
return true
}
@@ -328,6 +348,15 @@ export function installTerminalImeNativeTextForwarder(args: {
if (!commit) {
return
}
if (event.inputType.startsWith('delete')) {
// Why the claim stays pending: this deletion is the text system rewriting
// what it already committed, and the `insertText` carrying the rewrite is
// still inbound. Retiring here sent nothing and left that insert with no
// claim to attach to, so the whole keystroke produced no bytes at all.
commit.rewritten = true
event.stopImmediatePropagation()
return
}
pendingCommit = null
if (event.inputType !== 'insertText') {
// A non-text input takes the press over; it delivered nothing, so only the
@@ -336,14 +365,17 @@ export function installTerminalImeNativeTextForwarder(args: {
return
}
if (event.data) {
// A rewrite is declined, so what goes out is what the key produced rather
// than what the text system substituted for it.
const deliveredText = commit.rewritten ? commit.press.key : event.data
// Read the mutable flags EXACTLY once, here: kitty state can change
// between keydown and commit, and the release must describe the same
// negotiation the press was encoded under.
const encoding = encodeImeCommitForKitty(commit.press, args.getKittyKeyboardFlags?.() ?? 0, {
committedText: event.data,
committedText: deliveredText,
layoutCharacterForCode: getLayoutCharacterForCode
})
args.sendInput(encoding.report ?? event.data)
args.sendInput(encoding.report ?? deliveredText)
settleCommit(commit, encoding.release)
} else {
settleCommit(commit, null)
@@ -0,0 +1,240 @@
// @vitest-environment happy-dom
/**
* A text system that rewrites text the terminal has already sent.
*
* Every substitution the rest of the suite covers is decided before or during the keystroke that
* produces it — `,` commits as `,`, a `DefaultKeyBinding.dict` remap commits a backquote. One
* keydown, one character, no rewrite of anything already on the wire. That is an *input source*
* substitution and carrying it is what this forwarder exists for.
*
* macOS automatic period substitution is a different thing wearing the same clothes. It is a text
* *editing* convenience: type a second space and the system deletes the first one and inserts
* `". "` in its place. It reaches a browser text field — the helper textarea is one, and
* `autocorrect`/`spellcheck` do not govern the system text service — but it reaches no reference
* terminal, because none of them is a text view. Four were checked on hardware and all four
* deliver two spaces. The dash, smart-quote and text-replacement rules are the same family.
*
* So the rewrite is declined, and the keystroke sends what the key itself produced. The bug this
* pins is that it used to send *nothing at all*: the deletion arrives as an input event whose type
* is not `insertText`, which retired the claim without emitting, and the replacing `insertText`
* then found no claim to attach to. Two spaces became one.
*
* The discriminator is structural rather than a character table — a one-key-one-character
* substitution never deletes first — so nothing here reads which characters were involved.
*/ import { Terminal } from '@xterm/xterm'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { installTerminalImeNativeTextForwarder } from './terminal-ime-native-text-forwarder'
import { shouldBypassXtermKeyboardEvent } from './xterm-bypass-policy'
/** What a cooked tty leaves on the line after the erases the terminal sent are applied. */
function resolveLine(sent: string): string {
const line: string[] = []
for (const character of sent) {
if (character === '\x7f' || character === '\b') {
line.pop()
continue
}
line.push(character)
}
return line.join('')
}
function open(kittyKeyboardFlags = 0): {
sent: () => string
textarea: HTMLTextAreaElement
dispose: () => void
} {
const container = document.createElement('div')
document.body.appendChild(container)
const terminal = new Terminal()
terminal.open(container)
const forwarder = installTerminalImeNativeTextForwarder({
terminalElement: terminal.element,
isComposing: () => false,
sendInput: (data) => terminal.input(data),
getKittyKeyboardFlags: () => kittyKeyboardFlags
})
terminal.attachCustomKeyEventHandler((event) => {
if (forwarder.claimKeyEvent(event)) {
return false
}
return !shouldBypassXtermKeyboardEvent(event, {
isMac: true,
hasSelection: false,
kittyKeyboardFlags
})
})
const emitted: string[] = []
terminal.onData((data) => emitted.push(data))
return {
sent: () => emitted.join(''),
textarea: terminal.textarea!,
dispose: () => {
forwarder.dispose()
terminal.dispose()
}
}
}
function key(
textarea: HTMLTextAreaElement,
type: string,
init: { key: string; code: string; keyCode: number }
): KeyboardEvent {
const event = new KeyboardEvent(type, {
key: init.key,
code: init.code,
bubbles: true,
cancelable: true
})
Object.defineProperty(event, 'keyCode', { value: init.keyCode })
Object.defineProperty(event, 'charCode', { value: type === 'keypress' ? init.keyCode : 0 })
textarea.dispatchEvent(event)
return event
}
function inputEvent(textarea: HTMLTextAreaElement, inputType: string, data: string | null): void {
const event = new InputEvent('input', { bubbles: true })
Object.defineProperty(event, 'inputType', { value: inputType })
Object.defineProperty(event, 'data', { value: data })
Object.defineProperty(event, 'composed', { value: true })
textarea.dispatchEvent(event)
}
/** A printable key the text system commits verbatim, in the order Chromium delivers it. */
function pressPrintable(
textarea: HTMLTextAreaElement,
character: string,
code: string,
keyCode: number
): void {
const keydown = key(textarea, 'keydown', { key: character, code, keyCode })
if (!keydown.defaultPrevented) {
key(textarea, 'keypress', {
key: character,
code,
keyCode: character.charCodeAt(0)
})
}
textarea.value += character
inputEvent(textarea, 'insertText', character)
key(textarea, 'keyup', { key: character, code, keyCode })
}
/**
* The substituting keystroke: a keydown the text system swallows, then a keyless rewrite of text
* the terminal has already sent.
*/
function pressReplacing(
textarea: HTMLTextAreaElement,
press: { key: string; code: string; keyCode: number },
replacement: { deleteCount: number; insertedText: string }
): void {
key(textarea, 'keydown', press)
const retained = Array.from(textarea.value)
retained.splice(retained.length - replacement.deleteCount, replacement.deleteCount)
textarea.value = retained.join('')
inputEvent(textarea, 'deleteContentBackward', null)
textarea.value += replacement.insertedText
inputEvent(textarea, 'insertText', replacement.insertedText)
key(textarea, 'keyup', press)
}
describe('a text substitution decided after the terminal already sent the text', () => {
beforeEach(() => {
vi.spyOn(HTMLCanvasElement.prototype, 'getContext').mockReturnValue({
measureText: () => ({ width: 10 })
} as unknown as CanvasRenderingContext2D)
})
afterEach(() => {
vi.restoreAllMocks()
document.body.replaceChildren()
})
// Guards the two expectations below from passing vacuously: if this harness stopped reaching the
// forwarder's listeners at all, `it.fails` would still be satisfied and report nothing.
it('delivers a non-replacing commit through the same harness', () => {
const pane = open()
try {
pressPrintable(pane.textarea, 'a', 'KeyA', 65)
pressPrintable(pane.textarea, 'b', 'KeyB', 66)
expect(resolveLine(pane.sent())).toBe('ab')
} finally {
pane.dispose()
}
})
// The retraction machinery itself, exercised where the textarea still holds the text being
// rewritten. It does not in the two cases below, which is the whole reason they still fail: the
// forwarder blanks the textarea after every commit, so by the time a rewrite arrives the diff
// has no left side to measure against.
// The declining rule itself. Two spaces in, two spaces out, and no period invented — which is
// what every reference terminal does with these keystrokes.
it('declines a rewrite and sends what the key produced', () => {
const pane = open()
try {
pressPrintable(pane.textarea, 'a', 'KeyA', 65)
pressPrintable(pane.textarea, 'b', 'KeyB', 66)
pressPrintable(pane.textarea, ' ', 'Space', 32)
pressReplacing(
pane.textarea,
{ key: ' ', code: 'Space', keyCode: 32 },
{ deleteCount: 1, insertedText: '. ' }
)
expect(resolveLine(pane.sent())).toBe('ab ')
} finally {
pane.dispose()
}
})
// The regression that motivated all of this: before the fix the deletion retired the claim and
// the keystroke produced no bytes, so the user lost the space entirely.
it('does not swallow the keystroke the rewrite was attached to', () => {
const pane = open()
try {
pressReplacing(
pane.textarea,
{ key: ' ', code: 'Space', keyCode: 32 },
{ deleteCount: 0, insertedText: '. ' }
)
expect(pane.sent()).not.toBe('')
} finally {
pane.dispose()
}
})
// Same family, same rule: `--` must stay `--` rather than becoming an em dash.
it('declines a dash substitution', () => {
const pane = open()
try {
pressPrintable(pane.textarea, '-', 'Minus', 189)
pressReplacing(
pane.textarea,
{ key: '-', code: 'Minus', keyCode: 189 },
{ deleteCount: 1, insertedText: '—' }
)
expect(resolveLine(pane.sent())).toBe('--')
} finally {
pane.dispose()
}
})
// Bit 3 asks for every printable key as a CSI-u report. The declining rule has to hold there
// too: the report identity is the physical key, and the substituted character must not ride
// along as associated text.
it('declines a rewrite on a pane that negotiated kitty bit 3', () => {
const pane = open(8)
try {
pressReplacing(
pane.textarea,
{ key: ' ', code: 'Space', keyCode: 32 },
{ deleteCount: 1, insertedText: '. ' }
)
const sent = pane.sent()
expect(sent).not.toBe('')
expect(sent).not.toContain('.')
} finally {
pane.dispose()
}
})
})
+48
View File
@@ -173,3 +173,51 @@ export async function dispatchResumedCompositionUpdate(page: Page, data: string)
)
}, data)
}
/**
* Rewrites text the terminal has already committed, the way a text system that decides *after the
* fact* does: the previous characters are removed and replaced, with no key event of their own.
*
* Two producers share this shape. macOS automatic period substitution rewrites a trailing `" "`
* into `". "` on the second space, and a touch-iOS Korean source rewrites the syllable in place on
* every jamo (`ㅎ` -> `하` -> `한`) while firing no composition event at all. In both the deletion
* carries no `keydown`, so nothing that decides on key events can see it coming.
*
* SYNTHESISED rather than driven through CDP, and the reason is structural: `Input.insertText`
* only inserts at the caret and there is no CDP command that produces a keyless
* `deleteContentBackward`. Dispatching a real Backspace would work, but it would also produce a
* Backspace *keydown* the real substitution never sends — which is the whole difficulty. Same
* escape hatch, and same justification, as `dispatchResumedCompositionUpdate` above.
*
* The textarea value is mutated before each event because the deferred textarea diff reads it, so
* a spec that dispatched the events alone would exercise only half the path.
*/
export async function dispatchRetroactiveTextReplacement(
page: Page,
replacement: { deleteCount: number | 'all'; insertedText: string }
): Promise<void> {
await page.evaluate((step: { deleteCount: number | 'all'; insertedText: string }) => {
const textarea = document.querySelector<HTMLTextAreaElement>('.xterm-helper-textarea:focus')
if (!textarea) {
throw new Error('xterm helper textarea is not focused')
}
const fire = (type: string, inputType: string, data: string | null): void => {
textarea.dispatchEvent(
new InputEvent(type, { bubbles: true, composed: true, data, inputType })
)
}
// 'all' is for a source that replaces its whole in-progress region rather than a fixed count,
// which is what an in-place syllable rewrite does and what keeps this robust to whether the
// keystroke's own character was inserted before the rewrite ran.
const retained = Array.from(textarea.value)
const deleteCount = step.deleteCount === 'all' ? retained.length : step.deleteCount
retained.splice(retained.length - deleteCount, deleteCount)
textarea.value = retained.join('')
fire('beforeinput', 'deleteContentBackward', null)
fire('input', 'deleteContentBackward', null)
textarea.value += step.insertedText
textarea.setSelectionRange(textarea.value.length, textarea.value.length)
fire('beforeinput', 'insertText', step.insertedText)
fire('input', 'insertText', step.insertedText)
}, replacement)
}
@@ -0,0 +1,165 @@
/**
* Headless end-to-end coverage for macOS system text substitution reaching the terminal.
*
* This restores, as CDP-synthesised specs that run in the normal headless project, coverage two
* `@headful` macOS-only specs used to carry before they were removed as collateral in
* 17cfc968cf5. Those two depended on an accessibility grant, a live system input source and a
* Swift helper, and one was gated behind an env var, so CI had no lane that ran either.
*
* They also covered less than their names suggest. One typed a period and a space under the Latin
* source and a Hangul word plus a space under the Korean source and asserted that **nothing extra**
* appeared — recon for a report that automatic period substitution corrupts terminal input, which
* recorded the report's stated trigger not reproducing. The other explicitly "asserts
* preconditions, not outcomes". Neither was a regression test.
*
* Two things are pinned here, and they are the same guarantee stated twice:
*
* 1. Typing that looks like it might attract a substitution does not attract one. A Hangul commit
* followed by a word-boundary space, and a Latin period followed by a space, each reach the pty
* as exactly what was typed.
* 2. A substitution that really is delivered is **declined**. macOS automatic period substitution
* rewrites a trailing `" "` into `". "` on the second space, and it does reach a browser text
* field — the helper textarea is one. It reaches no reference terminal: four were checked on
* hardware and all four deliver two spaces, because none of them is a text view. So the
* terminal has to decline it explicitly, and the regression guarded is that the substituting
* keystroke used to produce no bytes at all, turning two spaces into one.
*
* Assertions are on the bytes the pty child receives, through the tty line discipline, because
* that is the line the user ends up with.
*/ import { expect, test } from './helpers/orca-app'
import { closeTerminalImePaneArena, openTerminalImePaneArena } from './terminal-ime-pane-arena'
import { readTerminalImeBoundaryTrace } from './terminal-ime-boundary-probe'
import {
commitImeText,
dispatchImeProcessKey,
dispatchImeRewrittenPrintableKey,
dispatchPlainEnter,
dispatchRetroactiveTextReplacement,
setImeComposition,
type ImeKeyIdentity
} from './terminal-ime-cdp-composition'
import {
createTerminalImeByteReader,
removeTerminalImeByteReader,
startTerminalImeByteReader,
waitForTerminalImeBytes
} from './terminal-ime-byte-reader'
import { applyImePlatformPolicy, expectImePlatformPolicy } from './terminal-ime-platform-policy'
const SPACE: ImeKeyIdentity = { key: ' ', code: 'Space', keyCode: 32 }
const PERIOD: ImeKeyIdentity = { key: '.', code: 'Period', keyCode: 190 }
/** macOS virtual key codes the removed spec typed, in the identities Chromium reports for them. */
const KEY_A: ImeKeyIdentity = { key: 'a', code: 'KeyA', keyCode: 65 }
const KEY_B: ImeKeyIdentity = { key: 'b', code: 'KeyB', keyCode: 66 }
test.describe('Terminal macOS text substitution', () => {
test('sends a Hangul commit and its word-boundary space with nothing added', async ({
orcaPage,
testRepoPath
}, testInfo) => {
// The removed spec's Korean arm: 2-Set d+k composes 아, then Space separates the word. It ran
// with NSAutomaticPeriodSubstitutionEnabled on and recorded `아 ` — one space, no period. The
// guard is that the terminal adds nothing of its own at a word boundary.
await applyImePlatformPolicy(orcaPage, 'mac')
await expectImePlatformPolicy(orcaPage, 'mac')
const arena = await openTerminalImePaneArena(orcaPage)
const reader = createTerminalImeByteReader(testRepoPath, 1)
let completed = false
try {
await startTerminalImeByteReader(orcaPage, arena.ptyId, reader)
for (const frame of ['ㅇ', '아']) {
await dispatchImeProcessKey(arena.session, { key: 'Process', code: 'KeyD' })
await setImeComposition(arena.session, frame)
}
await commitImeText(arena.session, '아')
await orcaPage.waitForTimeout(60)
await dispatchImeRewrittenPrintableKey(arena.session, SPACE)
await orcaPage.waitForTimeout(60)
await dispatchPlainEnter(arena.session)
expect((await readTerminalImeBoundaryTrace(orcaPage)).onData.join('')).toBe('아 \r')
expect(await waitForTerminalImeBytes(orcaPage, reader)).toEqual([
Buffer.from('아 \n').toString('hex')
])
completed = true
} finally {
await closeTerminalImePaneArena(arena, testInfo, 'hangul-word-boundary-space', !completed)
removeTerminalImeByteReader(reader)
}
})
test('sends a Latin period and space with nothing added', async ({
orcaPage,
testRepoPath
}, testInfo) => {
// The removed spec's ABC arm. A period immediately before a space is the position automatic
// period substitution acts on, so this is the paired control: with no second space there is
// nothing to substitute and the terminal must send exactly the two characters typed.
await applyImePlatformPolicy(orcaPage, 'mac')
const arena = await openTerminalImePaneArena(orcaPage)
const reader = createTerminalImeByteReader(testRepoPath, 1)
let completed = false
try {
await startTerminalImeByteReader(orcaPage, arena.ptyId, reader)
for (const identity of [PERIOD, SPACE]) {
await dispatchImeRewrittenPrintableKey(arena.session, identity)
await orcaPage.waitForTimeout(60)
}
await dispatchPlainEnter(arena.session)
expect((await readTerminalImeBoundaryTrace(orcaPage)).onData.join('')).toBe('. \r')
expect(await waitForTerminalImeBytes(orcaPage, reader)).toEqual([
Buffer.from('. \n').toString('hex')
])
completed = true
} finally {
await closeTerminalImePaneArena(arena, testInfo, 'latin-period-space', !completed)
removeTerminalImeByteReader(reader)
}
})
test('declines an automatic period substitution and sends both spaces', async ({
orcaPage,
testRepoPath
}, testInfo) => {
// Declined, not honoured. Four reference terminals were checked on hardware and none of them
// applies this substitution — they are not text views, so it never reaches them and two spaces
// stay two spaces. A browser-based terminal does receive it, so it has to decline explicitly.
// The regression guarded here is that the substituting keystroke used to produce no bytes at
// all, turning two spaces into one.
await applyImePlatformPolicy(orcaPage, 'mac')
const arena = await openTerminalImePaneArena(orcaPage)
const reader = createTerminalImeByteReader(testRepoPath, 1)
let completed = false
try {
await startTerminalImeByteReader(orcaPage, arena.ptyId, reader)
for (const identity of [KEY_A, KEY_B, SPACE]) {
await dispatchImeRewrittenPrintableKey(arena.session, identity)
await orcaPage.waitForTimeout(60)
}
// The second Space produces no character of its own: the text system swallows it and
// rewrites the trailing space into `. `.
await arena.session.send('Input.dispatchKeyEvent', {
type: 'rawKeyDown',
key: ' ',
code: 'Space',
windowsVirtualKeyCode: 32,
nativeVirtualKeyCode: 32,
text: '',
unmodifiedText: ''
})
await dispatchRetroactiveTextReplacement(orcaPage, { deleteCount: 1, insertedText: '. ' })
await orcaPage.waitForTimeout(60)
await dispatchPlainEnter(arena.session)
expect(await waitForTerminalImeBytes(orcaPage, reader)).toEqual([
Buffer.from('ab \n').toString('hex')
])
completed = true
} finally {
await closeTerminalImePaneArena(arena, testInfo, 'automatic-period-substitution', !completed)
removeTerminalImeByteReader(reader)
}
})
})