mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 08:02:28 +00:00
test(terminal): pin the two conditions that keep macOS period substitution out (#15393)
macOS rewrites a double space into ". " and hands the period to the pty.
Orca is not immune to that: a plain Chromium textarea in the Electron
version pinned here does substitute, measured on hardware with real key
events, and spellcheck="false" does not prevent it. What prevents it is
that the forwarder claims a plain space keydown and then empties the
helper textarea, so the text system never sees the word preceding the
space.
Neither half was a decision. Before 01bcc8dca2 the claim predicate
excluded space, letters and digits, the field accumulated, and the
substitution fired on real hardware. That commit widened the predicate
for unrelated reasons - IME commit survival and kitty encoding - and
suppressed this as a side effect it never mentions. The predicate
already declines on modifiers and during composition, so a later
narrowing would return the bug with nothing to catch it.
This does not test the substitution, which a unit environment cannot
produce. It tests the two conditions measured to suppress it. Verified
non-vacuous: narrowing the predicate back toward punctuation fails three
of the four, and removing the blanking fails the fourth.
Refs #11504
This commit is contained in:
@@ -0,0 +1,133 @@
|
||||
// @vitest-environment happy-dom
|
||||
/**
|
||||
* Pins the two properties that keep #11504 from firing, because neither is a decision anyone
|
||||
* made and both are one refactor away from being undone.
|
||||
*
|
||||
* macOS rewrites a double space into ". " and hands the period to the pty. Orca is not immune to
|
||||
* that - a plain Chromium textarea in the Electron version this repo pins does substitute, and
|
||||
* `spellcheck="false"` does not prevent it. What prevents it is that the forwarder claims a plain
|
||||
* space keydown and then empties the helper textarea, so the text system never sees the preceding
|
||||
* word the rule keys on.
|
||||
*
|
||||
* Both halves are needed. Before 01bcc8dca24 the claim predicate excluded space, letters and
|
||||
* digits, the field accumulated, and the substitution fired on real hardware. That commit widened
|
||||
* the predicate for unrelated reasons - IME commit survival and kitty encoding - and suppressed
|
||||
* this as a side effect it never mentions.
|
||||
*
|
||||
* So this file does not test the substitution, which cannot be produced in a unit environment. It
|
||||
* tests the two conditions measured to suppress it, so narrowing either one fails here rather
|
||||
* than in a Korean user's shell.
|
||||
*/
|
||||
import { Terminal } from '@xterm/xterm'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { installTerminalImeNativeTextForwarder } from './terminal-ime-native-text-forwarder'
|
||||
|
||||
function open() {
|
||||
const container = document.createElement('div')
|
||||
document.body.appendChild(container)
|
||||
const terminal = new Terminal()
|
||||
terminal.open(container)
|
||||
const textarea = terminal.textarea!
|
||||
const forwarder = installTerminalImeNativeTextForwarder({
|
||||
terminalElement: terminal.element,
|
||||
isComposing: () => false,
|
||||
sendInput: (data) => terminal.input(data),
|
||||
getKittyKeyboardFlags: () => 0
|
||||
})
|
||||
// Wired the way the pane lifecycle wires it: a claimed key must not also reach xterm's encoder.
|
||||
terminal.attachCustomKeyEventHandler((event) => !forwarder.claimKeyEvent(event))
|
||||
const emitted: string[] = []
|
||||
terminal.onData((d) => emitted.push(d))
|
||||
return { emitted, terminal, textarea, forwarder, container }
|
||||
}
|
||||
|
||||
function makeKeydown(init: {
|
||||
key: string
|
||||
code: string
|
||||
keyCode: number
|
||||
ctrlKey?: boolean
|
||||
}): KeyboardEvent {
|
||||
return new KeyboardEvent('keydown', { ...init, bubbles: true, cancelable: true })
|
||||
}
|
||||
|
||||
function keydown(
|
||||
textarea: HTMLTextAreaElement,
|
||||
init: { key: string; code: string; keyCode: number }
|
||||
) {
|
||||
const event = makeKeydown(init)
|
||||
textarea.dispatchEvent(event)
|
||||
return event
|
||||
}
|
||||
|
||||
function commit(textarea: HTMLTextAreaElement, data: string) {
|
||||
textarea.value += data
|
||||
textarea.dispatchEvent(new InputEvent('input', { data, inputType: 'insertText', bubbles: true }))
|
||||
}
|
||||
|
||||
describe('#11504 suppression rests on claiming space and emptying the field', () => {
|
||||
let panes: ReturnType<typeof open>[] = []
|
||||
|
||||
beforeEach(() => {
|
||||
panes = []
|
||||
// Why: happy-dom has no canvas, and the renderer measures text on open().
|
||||
vi.spyOn(HTMLCanvasElement.prototype, 'getContext').mockReturnValue({
|
||||
measureText: () => ({ width: 10 })
|
||||
} as unknown as CanvasRenderingContext2D)
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
for (const pane of panes) {
|
||||
pane.forwarder.dispose()
|
||||
pane.terminal.dispose()
|
||||
pane.container.remove()
|
||||
}
|
||||
vi.restoreAllMocks()
|
||||
document.body.replaceChildren()
|
||||
})
|
||||
|
||||
function pane() {
|
||||
const created = open()
|
||||
panes.push(created)
|
||||
return created
|
||||
}
|
||||
|
||||
// Why space specifically: it is the character the substitution rule triggers on, and it is the
|
||||
// one the pre-01bcc8dca24 predicate explicitly excluded.
|
||||
it('claims a plain space keydown', () => {
|
||||
const p = pane()
|
||||
expect(p.forwarder.claimKeyEvent(makeKeydown({ key: ' ', code: 'Space', keyCode: 32 }))).toBe(
|
||||
true
|
||||
)
|
||||
})
|
||||
|
||||
it('claims a plain letter keydown', () => {
|
||||
const p = pane()
|
||||
expect(p.forwarder.claimKeyEvent(makeKeydown({ key: 'a', code: 'KeyA', keyCode: 65 }))).toBe(
|
||||
true
|
||||
)
|
||||
})
|
||||
|
||||
// The second half: a claimed key must leave the field empty, or the text system reads a word
|
||||
// before the space and substitutes.
|
||||
it('leaves the helper textarea empty after a claimed commit, so no word precedes the space', () => {
|
||||
const p = pane()
|
||||
keydown(p.textarea, { key: 'a', code: 'KeyA', keyCode: 65 })
|
||||
commit(p.textarea, 'a')
|
||||
keydown(p.textarea, { key: 'b', code: 'KeyB', keyCode: 66 })
|
||||
commit(p.textarea, 'b')
|
||||
keydown(p.textarea, { key: ' ', code: 'Space', keyCode: 32 })
|
||||
commit(p.textarea, ' ')
|
||||
|
||||
expect(p.emitted.join('')).toBe('ab ')
|
||||
expect(p.textarea.value).toBe('')
|
||||
})
|
||||
|
||||
// A control chord is deliberately not claimed - that path belongs to xterm's encoder - so this
|
||||
// asserts the exclusions that exist on purpose still hold.
|
||||
it('does not claim a control chord', () => {
|
||||
const p = pane()
|
||||
expect(
|
||||
p.forwarder.claimKeyEvent(makeKeydown({ key: 'c', code: 'KeyC', keyCode: 67, ctrlKey: true }))
|
||||
).toBe(false)
|
||||
})
|
||||
})
|
||||
@@ -110,6 +110,12 @@ function isNativeTextKeydown(event: ImeNativeTextKeyEvent, compositionActive: bo
|
||||
!event.ctrlKey &&
|
||||
!event.altKey &&
|
||||
!event.metaKey &&
|
||||
// Space and letters must stay eligible. Claiming them is what blanks the helper textarea
|
||||
// before macOS can read a preceding word, which is the only thing suppressing #11504 - the
|
||||
// system rewriting a double space into ". " and handing the period to the pty. That
|
||||
// suppression is a side effect of this predicate rather than a decision, so narrowing it
|
||||
// back toward punctuation would return the bug. Pinned by
|
||||
// terminal-ime-forwarder-space-claim.test.ts.
|
||||
event.key.length === 1 &&
|
||||
// Composing keystrokes already belong to xterm's composition helper.
|
||||
event.isComposing !== true &&
|
||||
@@ -349,7 +355,10 @@ export function installTerminalImeNativeTextForwarder(args: {
|
||||
settleCommit(commit, null)
|
||||
}
|
||||
event.stopImmediatePropagation()
|
||||
// Clear the helper textarea so the committed text doesn't accumulate.
|
||||
// Clear the helper textarea so the committed text doesn't accumulate. Also load-bearing:
|
||||
// macOS decides an automatic period substitution from the characters already in the field,
|
||||
// so emptying it is what keeps #11504 from firing. Measured - with this line removed and
|
||||
// everything else held constant, `hi` + two spaces reaches the pty as `hi. `.
|
||||
if (event.target instanceof HTMLTextAreaElement) {
|
||||
event.target.value = ''
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user