diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index dd366f225de..ab491037171 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -615,6 +615,7 @@ jobs: 'tests/e2e/terminal-hangul-wrap-boundary-bytes.spec.ts' \ 'tests/e2e/terminal-ime-exact-byte.spec.ts' \ 'tests/e2e/terminal-korean-composing-chord-order.spec.ts' \ + 'tests/e2e/terminal-korean-endofrow-preedit-cell-span.spec.ts' \ 'tests/e2e/terminal-korean-midline-preedit-occlusion.spec.ts' \ 'tests/e2e/terminal-korean-preedit-visibility.spec.ts' | sort -u)" fi diff --git a/src/renderer/src/components/terminal-pane/terminal-cjk-cursor-cell-placement.test.ts b/src/renderer/src/components/terminal-pane/terminal-cjk-cursor-cell-placement.test.ts index be2ffbed67a..324856d985b 100644 --- a/src/renderer/src/components/terminal-pane/terminal-cjk-cursor-cell-placement.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-cjk-cursor-cell-placement.test.ts @@ -13,6 +13,11 @@ * * The occluding box in that report is the IME preedit overlay, not the cursor — see * `terminal-ime-xterm-trailing-preedit-occlusion.test.ts`. + * + * Claim 2 is pinned against the DOM renderer, which is the only one happy-dom can drive. The + * shipped default on macOS is webgl, where the same guarantee comes from a different mechanism — + * the cursor fill spans `cursorX .. cursorX + cell.getWidth() - 1`, so it covers both cells of a + * wide glyph rather than clipping one. That arm cannot live here. */ import { Terminal } from '@xterm/xterm' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' @@ -20,30 +25,46 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' // Escapes made visible in the issue: hide cursor, home, CUF 2, CUD 24, the text, then two CUPs. const REPORTED_PTY_STREAM = '\x1b[?25l\x1b[H\x1b[2C\x1b[24B가나다라\x1b[30;1H\x1b[25;11H\x1b[?25h' +type Rig = { + container: HTMLElement + terminal: Terminal +} + const openTerminals: Terminal[] = [] -function openTerminal(): Terminal { +function openTerminal(): Rig { const container = document.createElement('div') document.body.appendChild(container) const terminal = new Terminal({ cols: 80, rows: 30 }) terminal.open(container) openTerminals.push(terminal) - return terminal + return { container, terminal } } function write(terminal: Terminal, data: string): Promise { return new Promise((resolve) => terminal.write(data, resolve)) } -/** Awaits the repaint the write triggers, so the row spans under assertion are the rendered ones. */ +/** + * Lets the write settle, drains the frame it already scheduled, then subscribes before forcing the + * one under assertion — so the row spans read below are the rendered ones, and the listener is + * always in place before its trigger rather than racing a frame that already fired. + */ async function writeAwaitingRender(terminal: Terminal, data: string): Promise { await write(terminal, data) - await new Promise((resolve) => { - const rendered = terminal.onRender(() => { - rendered.dispose() + await new Promise((resolve) => window.setTimeout(resolve, 0)) + const rendered = new Promise((resolve) => { + const listener = terminal.onRender(() => { + listener.dispose() resolve() }) }) + terminal.refresh(0, terminal.rows - 1) + await rendered +} + +function cursorCell(container: HTMLElement): Element | null { + return container.querySelector('.xterm-rows .xterm-cursor') } function describeCells(terminal: Terminal, count: number): string[] { @@ -77,7 +98,7 @@ describe('#12729 — the cursor is not displaced by full-width text', () => { }) it('leaves the reported pty stream with the cursor on the requested column', async () => { - const terminal = openTerminal() + const { terminal } = openTerminal() await write(terminal, REPORTED_PTY_STREAM) @@ -103,25 +124,25 @@ describe('#12729 — the cursor is not displaced by full-width text', () => { }) it('paints the cursor after the run, not on the last syllable', async () => { - const terminal = openTerminal() + const { container, terminal } = openTerminal() terminal.focus() await writeAwaitingRender(terminal, '> 가나다라') - const cursorSpan = document.querySelector('.xterm-rows .xterm-cursor') + const cursorSpan = cursorCell(container) expect(cursorSpan?.previousElementSibling?.textContent).toBe('> 가나다라') expect(cursorSpan?.textContent).toBe(' ') }) it('carries the whole syllable when the cursor does sit on a full-width glyph', async () => { - const terminal = openTerminal() + const { container, terminal } = openTerminal() terminal.focus() // CHA 9: back onto 라's starting cell, the placement the report describes. await writeAwaitingRender(terminal, '> 가나다라\x1b[9G') expect(terminal.buffer.active.cursorX).toBe(8) - const cursorSpan = document.querySelector('.xterm-rows .xterm-cursor') + const cursorSpan = cursorCell(container) // The whole syllable is inside the cursor span, so a block cursor cannot clip its right half. expect(cursorSpan?.textContent).toBe('라') expect(cursorSpan?.classList.contains('xterm-cursor-block')).toBe(true) diff --git a/src/renderer/src/components/terminal-pane/terminal-cursor-appearance-precedence.test.ts b/src/renderer/src/components/terminal-pane/terminal-cursor-appearance-precedence.test.ts index 4fdda2b009a..d9bb0a4358b 100644 --- a/src/renderer/src/components/terminal-pane/terminal-cursor-appearance-precedence.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-cursor-appearance-precedence.test.ts @@ -10,6 +10,11 @@ * The one thing that does override the preference is a `DECSCUSR` from the running program, which * every terminal honours and which prompt frameworks emit routinely. That precedence is pinned * here because it is the answer to "my setting does nothing" whenever the overlay is not involved. + * + * Note what this file does NOT establish: composing the opacity into `theme.cursor` is not the same + * as it reaching the screen. The webgl renderer uses the cursor colour as a cell background and + * drops its alpha, so opacity is inert for a block cursor there — the default style on the default + * renderer. That is upstream in the addon, and only the DOM renderer is exercised below. */ import { Terminal } from '@xterm/xterm' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' @@ -24,7 +29,41 @@ function decPrivateCursorStyle(terminal: Terminal): string | undefined { )._core.coreService.decPrivateModes.cursorStyle } -describe('#12729 — cursor style and opacity reach the terminal', () => { +function cursorClasses(container: HTMLElement): string[] { + const cursor = container.querySelector('.xterm-rows .xterm-cursor') + if (!cursor) { + throw new Error('no cursor cell was rendered') + } + return [...cursor.classList] +} + +/** Resolves once the emulator has parsed `data`, so the escape has taken effect before any read. */ +function write(terminal: Terminal, data: string): Promise { + return new Promise((resolve) => terminal.write(data, resolve)) +} + +/** + * Lets the act settle, drains the frame it already scheduled, then subscribes before forcing the + * one under assertion — so the awaited render is guaranteed to post-date the act, and the listener + * is always in place before its trigger runs. + */ +async function actAndAwaitRender( + terminal: Terminal, + act: () => void | Promise +): Promise { + await act() + await new Promise((resolve) => window.setTimeout(resolve, 0)) + const rendered = new Promise((resolve) => { + const listener = terminal.onRender(() => { + listener.dispose() + resolve() + }) + }) + terminal.refresh(0, terminal.rows - 1) + await rendered +} + +describe('#12729 — cursor style and opacity survive the settings path', () => { it('keeps an explicitly chosen bar across the settings write and the next load', () => { // The write path stamps the migration flag alongside the user's choice. const written = normalizeTerminalCursorStyleDefault( @@ -74,19 +113,30 @@ describe('#12729 — DECSCUSR from the program outranks the preference', () => { const terminal = new Terminal({ cols: 40, rows: 6, cursorStyle: 'bar' }) terminal.open(container) openTerminals.push(terminal) + terminal.focus() + await actAndAwaitRender(terminal, () => write(terminal, 'x')) expect(decPrivateCursorStyle(terminal)).toBeUndefined() + expect(cursorClasses(container)).toContain('xterm-cursor-bar') // `CSI 2 SP q` — steady block, what a prompt framework emits on every redraw. - await new Promise((resolve) => terminal.write('\x1b[2 q', resolve)) + await actAndAwaitRender(terminal, () => write(terminal, '\x1b[2 q')) expect(decPrivateCursorStyle(terminal)).toBe('block') + // The rendered class, not the private mode: the two are separate fields, so only what the + // renderer resolved out of them can fail when the precedence itself is dropped. + expect(cursorClasses(container)).toContain('xterm-cursor-block') + expect(cursorClasses(container)).not.toContain('xterm-cursor-bar') // Changing the preference while that is set cannot win it back; only the reset does. - terminal.options.cursorStyle = 'underline' - expect(decPrivateCursorStyle(terminal)).toBe('block') + await actAndAwaitRender(terminal, () => { + terminal.options.cursorStyle = 'underline' + }) + expect(cursorClasses(container)).toContain('xterm-cursor-block') // Orca sends this reset on replay and on the agent-idle path (RESET_TERMINAL_CURSOR_STYLE). - await new Promise((resolve) => terminal.write('\x1b[0 q', resolve)) + await actAndAwaitRender(terminal, () => write(terminal, '\x1b[0 q')) expect(decPrivateCursorStyle(terminal)).toBeUndefined() + // And control lands back on the preference, which moved to underline while DECSCUSR held it. + expect(cursorClasses(container)).toContain('xterm-cursor-underline') }) }) diff --git a/tests/e2e/terminal-korean-endofrow-preedit-cell-span.spec.ts b/tests/e2e/terminal-korean-endofrow-preedit-cell-span.spec.ts new file mode 100644 index 00000000000..1946bfbe640 --- /dev/null +++ b/tests/e2e/terminal-korean-endofrow-preedit-cell-span.spec.ts @@ -0,0 +1,99 @@ +/** + * How many grid cells does an end-of-row preedit overlay actually occupy on screen? + * + * #12729 reported a black block over the last syllable of `가나다라`, and measured it at one cell + * wide (14px against a 14px grid). We answer that report by saying the block is the preedit + * overlay, not the cursor — but an overlay holding one Hangul syllable is shrink-to-fit around a + * full-width glyph, so it should span about two cells, not one. That mismatch is the only measured + * quantity in the report that argues against our explanation, and nothing in the unit suite can + * settle it: `CompositionHelper` sets `maxWidth` but never `width`, so the rendered width comes + * entirely from layout, and happy-dom reports every rect as zero. + * + * So this is the arm that measures it. If it fails, our answer to #12729 is wrong and the report's + * one-cell measurement was right. + * + * The assertion is a floor, not a band. How many cells a syllable occupies is a function of the + * runner's font metrics and fallback — the mid-line spec records a preedit measured at 34.4px over + * an 8.43px grid, which is four cells, not two — so pinning "about two" would pin the machine. The + * claim under test is only that the overlay is materially wider than the single cell the report + * measured, which is what discriminates the overlay from the cursor. + */ +import type { Page } from '@stablyai/playwright-test' +import { expect, test } from './helpers/orca-app' +import { closeTerminalImePaneArena, openTerminalImePaneArena } from './terminal-ime-pane-arena' +import { setImeComposition } from './terminal-ime-cdp-composition' +import { + sampleMidlinePreeditOcclusion, + writeToActiveTerminal, + type MidlinePreeditOcclusionSample +} from './terminal-ime-midline-occlusion-probe' +import { samplePreeditOverlay } from './terminal-ime-preedit-overlay-probe' + +/** Below this the overlay is the one-cell block the report measured; above it, a full-width glyph. */ +const MIN_CELLS_FOR_A_FULL_WIDTH_SYLLABLE = 1.5 + +/** + * Waits for the preedit to reach the overlay at a non-zero size, then samples once — the pane runs + * a live shell that can repaint the row, so only the frame the composition opened on carries the + * state under assertion. + */ +async function sampleOpenComposition(page: Page): Promise { + await expect + .poll( + async () => { + const overlay = await samplePreeditOverlay(page) + return overlay.active && overlay.rect.width > 0 && overlay.text === '가' + }, + { message: 'the preedit never reached the overlay at a non-zero size' } + ) + .toBe(true) + return sampleMidlinePreeditOcclusion(page) +} + +function describeSpan(sample: MidlinePreeditOcclusionSample): string { + const cells = sample.cellWidth > 0 ? sample.overlayRect.width / sample.cellWidth : 0 + return `overlay ${sample.overlayRect.width}px over a ${sample.cellWidth}px cell = ${cells.toFixed(2)} cells, covering columns ${JSON.stringify(sample.coveredColumns)}` +} + +test.describe('Terminal end-of-row Korean preedit cell span', () => { + test('renders a composing syllable wider than the one cell #12729 measured', async ({ + orcaPage + }, testInfo) => { + const arena = await openTerminalImePaneArena(orcaPage) + let completed = false + try { + // 안녕하세요 is ten cells, so the cursor lands at the end of the row with nothing after it — + // the shape #12729 hits, and the one where the overlay carries the preedit alone. + await writeToActiveTerminal(orcaPage, '\x1b[2J\x1b[H안녕하세요') + await setImeComposition(arena.session, '가') + + const sample = await sampleOpenComposition(orcaPage) + expect(sample.rowTailFromCursor, 'text still sits after the cursor').toBe('') + expect(sample.cursorColumn, 'the cursor is not at the end of the committed run').toBe(10) + expect( + sample.cellWidth, + 'the grid reported no cell width to measure against' + ).toBeGreaterThan(0) + + // The load-bearing assertion, and the one no unit arm can make: a preedit overlay holding a + // full-width syllable is wider than a single cell, so the one-cell block in the report + // cannot be an overlay rendering the same way this one does. + expect( + sample.overlayRect.width / sample.cellWidth, + `a composing Hangul syllable must occupy more than one cell — ${describeSpan(sample)}` + ).toBeGreaterThan(MIN_CELLS_FOR_A_FULL_WIDTH_SYLLABLE) + expect( + sample.coveredColumns, + `the overlay covers no grid columns to assert about — ${describeSpan(sample)}` + ).not.toBe(null) + completed = true + } finally { + await closeTerminalImePaneArena( + arena, + testInfo, + 'korean-endofrow-preedit-cell-span', + !completed + ) + } + }) +})