From 6caa44eefb2b4b0acd465d803664836119d17fc0 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 28 Oct 2026 11:38:00 -0700 Subject: [PATCH] fix(mobile): keep the terminal reading position when resize snapshots overlap (#31350) * fix(mobile): keep the reading position when resize snapshots overlap A snapshot that overtook a hidden replay took its scroll anchor from the hidden terminal, which sits at the bottom, so the reader jumped down. The newer replay now keeps the anchor the overtaken one would have restored, unless live output, a clear or an alternate screen made the hidden buffer the source. A superseded replay also hands the live terminal back before theme painting reads it. Co-authored-by: Neil Co-authored-by: ogarciarevett * fix(mobile): keep the overtaken anchor when live output arrives during a replay * test(mobile): use T[] for the replay harness completions --------- Co-authored-by: Kelvin Amoaba <97001695+AmoabaKelvin@users.noreply.github.com> Co-authored-by: ogarciarevett --- .../src/terminal/document/document-scope.ts | 3 + .../terminal/document/host-message-router.ts | 1 + mobile/src/terminal/document/surface-swap.ts | 1 + mobile/src/terminal/document/terminal-init.ts | 8 +- .../document/terminal-replay-scroll.test.ts | 168 ++++++++++++++++++ .../terminal-webview-init-surface.test.ts | 12 +- 6 files changed, 191 insertions(+), 2 deletions(-) create mode 100644 mobile/src/terminal/document/terminal-replay-scroll.test.ts diff --git a/mobile/src/terminal/document/document-scope.ts b/mobile/src/terminal/document/document-scope.ts index 2f75d79ba6c..24d39049c5f 100644 --- a/mobile/src/terminal/document/document-scope.ts +++ b/mobile/src/terminal/document/document-scope.ts @@ -99,6 +99,8 @@ export type TerminalDocumentState = { bottomRowBackdrop: string surface: HTMLElement | null pendingTerm: TerminalDocumentTerminal | null + // The anchor the pending replay will restore; null after a clear or an alternate-screen snapshot. + pendingReplayScrollAnchorRows: number | null committedTerm: TerminalDocumentTerminal | null provisionalTerm: TerminalDocumentTerminal | null committedSurface: HTMLElement | null @@ -222,6 +224,7 @@ function createTerminalDocumentState(): TerminalDocumentState { bottomRowBackdrop: '', surface: null, pendingTerm: null, + pendingReplayScrollAnchorRows: null, committedTerm: null, provisionalTerm: null, committedSurface: null, diff --git a/mobile/src/terminal/document/host-message-router.ts b/mobile/src/terminal/document/host-message-router.ts index f746bdfe4eb..e3bc07b0013 100644 --- a/mobile/src/terminal/document/host-message-router.ts +++ b/mobile/src/terminal/document/host-message-router.ts @@ -86,6 +86,7 @@ export function handleMsg(scope: TerminalDocumentScope, msg: TerminalHostMessage } else if (msg.type === 'write') { write(scope, msg.data!) } else if (msg.type === 'clear') { + scope.pendingReplayScrollAnchorRows = null scope.terminalGeneration++ resetWriteQueue(scope) resumeTerminalDataReplyAuthority(scope) diff --git a/mobile/src/terminal/document/surface-swap.ts b/mobile/src/terminal/document/surface-swap.ts index 6e7a42a0af7..f80186bf5de 100644 --- a/mobile/src/terminal/document/surface-swap.ts +++ b/mobile/src/terminal/document/surface-swap.ts @@ -22,6 +22,7 @@ export function beginTerminalSurfaceSwap(scope: TerminalDocumentScope) { } scope.pendingSurface = null scope.pendingTerm = null + scope.term = scope.committedTerm } const swap = { oldTerm: scope.committedTerm, diff --git a/mobile/src/terminal/document/terminal-init.ts b/mobile/src/terminal/document/terminal-init.ts index 8ed1e70ad28..c2594a17403 100644 --- a/mobile/src/terminal/document/terminal-init.ts +++ b/mobile/src/terminal/document/terminal-init.ts @@ -109,7 +109,12 @@ export function init( preserveScroll && scope.term && scope.term.buffer && scope.term.buffer.active ? scope.term.buffer.active : null - const scrollAnchorRows = prevB ? Math.max(0, (prevB.baseY || 0) - (prevB.viewportY || 0)) : -1 + // A hidden replay never moved what the reader sees, so one that overtakes it keeps its anchor. + const supersededAnchorRows = scope.pendingTerm ? scope.pendingReplayScrollAnchorRows : null + const scrollAnchorRows = !preserveScroll + ? -1 + : (supersededAnchorRows ?? + (prevB ? Math.max(0, (prevB.baseY || 0) - (prevB.viewportY || 0)) : -1)) scope.terminalGeneration++ const gen = scope.terminalGeneration const reused = scope.provisionalTerm @@ -143,6 +148,7 @@ export function init( // Mirror only replayed modes after normal-buffer prefixes are discarded. updateMouseModeFromData(scope, replayData) scope.activeAltScreenSnapshot = isAltScreenActive(replayData) + scope.pendingReplayScrollAnchorRows = scope.activeAltScreenSnapshot ? null : scrollAnchorRows scope.initialOscLinks = Array.isArray(nextOscLinks) ? nextOscLinks : [] scope.initialOscLinkRowOffset = 0 scope.initialOscLinkEvictionReady = false diff --git a/mobile/src/terminal/document/terminal-replay-scroll.test.ts b/mobile/src/terminal/document/terminal-replay-scroll.test.ts new file mode 100644 index 00000000000..c987c42261b --- /dev/null +++ b/mobile/src/terminal/document/terminal-replay-scroll.test.ts @@ -0,0 +1,168 @@ +// @vitest-environment happy-dom +import { Terminal } from '@xterm/xterm' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { TERMINAL_DOCUMENT_MARKUP } from '../terminal-webview-html' +import { createTerminalDocumentScope, type TerminalDocumentScope } from './document-scope' +import { startTerminalDocument, stopTerminalDocument } from './create-terminal-document' +import { terminalDocumentDouble } from './document-terminal-double.test-support' +import type { TerminalDocumentTerminal } from './document-terminal-shape' +import { handleMsg } from './host-message-router' +import { afterWritesDrained } from './write-queue' + +const scopes: TerminalDocumentScope[] = [] + +function createReplayHarness() { + const frames: FrameRequestCallback[] = [] + vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => { + frames.push(callback) + return frames.length + }) + vi.stubGlobal('cancelAnimationFrame', () => {}) + document.body.innerHTML = TERMINAL_DOCUMENT_MARKUP + const terminals: Terminal[] = [] + const writeCompletions: (() => void)[] = [] + let deferWrites = false + const scope = createTerminalDocumentScope({ + hasEngine: () => true, + installHostTransport: () => () => {}, + createTerminal: (options) => { + const terminal = new Terminal({ + cols: typeof options.cols === 'number' ? options.cols : 80, + rows: typeof options.rows === 'number' ? options.rows : 24, + scrollback: 5000, + allowProposedApi: true + }) + terminals.push(terminal) + // Real xterm parser and viewport, without a canvas renderer. + return { + ...terminalDocumentDouble().terminal, + get cols() { + return terminal.cols + }, + get rows() { + return terminal.rows + }, + get buffer() { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: xterm's own buffer; only getCell's cell parameter is narrower in the document shape, as in terminal-web-document-mount.ts. + return terminal.buffer as unknown as TerminalDocumentTerminal['buffer'] + }, + write: (data, callback) => + terminal.write(data, () => { + if (callback && deferWrites) { + writeCompletions.push(callback) + } else { + callback?.() + } + }), + clear: () => terminal.clear(), + reset: () => terminal.reset(), + resize: (cols, rows) => terminal.resize(cols, rows), + scrollToLine: (line) => terminal.scrollToLine(line), + scrollLines: (lines) => terminal.scrollLines(lines), + scrollToBottom: () => terminal.scrollToBottom(), + dispose: () => terminal.dispose() + } + }, + createUnicode11Addon: () => null, + createWebglAddon: () => null, + postToHost: () => {} + }) + startTerminalDocument(scope) + scopes.push(scope) + + const snapshot = Array.from({ length: 200 }, (_, index) => `ROW_${index}`).join('\r\n') + return { + scope, + terminals, + replay: (preserveScroll: boolean, cols: number, initialData = snapshot) => { + handleMsg(scope, { + type: 'init', + cols, + rows: 24, + initialData, + preserveScroll + }) + frames.shift()?.(0) + }, + drain: () => new Promise((resolve) => afterWritesDrained(scope, resolve)), + parse: () => new Promise((resolve) => terminals.at(-1)!.write('', resolve)), + holdReplays: (hold: boolean) => { + deferWrites = hold + }, + releaseHeldReplays: () => { + for (const complete of writeCompletions.splice(0).toReversed()) { + complete() + } + } + } +} + +type ReplayHarness = ReturnType + +async function readAt40ThenOvertakeHiddenReplay({ + hiddenReplayData, + duringHiddenReplay +}: { + hiddenReplayData?: string + duringHiddenReplay?: (harness: ReplayHarness) => void +} = {}) { + const harness = createReplayHarness() + harness.replay(false, 80) + await harness.drain() + harness.terminals[0]!.scrollToLine(40) + harness.holdReplays(true) + harness.replay(true, 81, hiddenReplayData) + await harness.parse() + expect(harness.scope.pendingSurface).not.toBeNull() + duringHiddenReplay?.(harness) + harness.holdReplays(false) + harness.replay(true, 82) + await harness.drain() + // The overtaken replay's late completion must not commit it or move the reader. + harness.releaseHeldReplays() + expect(harness.scope.pendingTerm).toBeNull() + expect(document.querySelectorAll('#terminal-container > div')).toHaveLength(1) + return harness.scope.committedTerm!.buffer.active +} + +afterEach(() => { + for (const scope of scopes.splice(0)) { + stopTerminalDocument(scope) + } + vi.unstubAllGlobals() +}) + +describe('mobile terminal replay reading position', () => { + it.each([ + { name: '' }, + { + name: ' that printed live output', + duringHiddenReplay: (harness: ReplayHarness) => + handleMsg(harness.scope, { type: 'write', data: '\r\nLIVE' }) + } + ])( + 'keeps the reading position when a resize snapshot overtakes a hidden replay$name', + async ({ name: _name, ...overtake }) => { + const buffer = await readAt40ThenOvertakeHiddenReplay(overtake) + + expect(buffer.viewportY).toBe(40) + expect(buffer.getLine(40)?.translateToString(true)).toBe('ROW_40') + } + ) + + it.each([ + { + name: 'the terminal was cleared', + duringHiddenReplay: (harness: ReplayHarness) => handleMsg(harness.scope, { type: 'clear' }) + }, + { + name: 'the hidden replay was an alternate screen', + hiddenReplayData: '\x1b[?1049h\x1b[HTUI' + } + ])('follows the bottom when $name', async ({ name: _name, ...overtake }) => { + const buffer = await readAt40ThenOvertakeHiddenReplay(overtake) + + expect(buffer.type).toBe('normal') + expect(buffer.viewportY).toBe(buffer.baseY) + }) +}) diff --git a/mobile/src/terminal/terminal-webview-init-surface.test.ts b/mobile/src/terminal/terminal-webview-init-surface.test.ts index 51bd5227200..3cc9e89dbad 100644 --- a/mobile/src/terminal/terminal-webview-init-surface.test.ts +++ b/mobile/src/terminal/terminal-webview-init-surface.test.ts @@ -20,6 +20,7 @@ type RegisteredWindowListener = { } function makeTerminal(writeCallbacks: Array<() => void>, writes: string[]) { + const dimensions = { css: { cell: { width: 8, height: 15 } } } const terminal = { cols: 80, rows: 24, @@ -27,7 +28,16 @@ function makeTerminal(writeCallbacks: Array<() => void>, writes: string[]) { modes: {}, element: null as HTMLElement | null, disposed: false, - _core: { _renderService: { dimensions: { css: { cell: { width: 8, height: 15 } } } } }, + _core: { + _renderService: { + get dimensions(): typeof dimensions { + if (terminal.disposed) { + throw new Error('Renderer dimensions are unavailable after terminal disposal') + } + return dimensions + } + } + }, buffer: { active: { viewportY: 0,