diff --git a/src/renderer/src/components/terminal-pane/expand-collapse.ts b/src/renderer/src/components/terminal-pane/expand-collapse.ts index 83ad95e9c..a8b499154 100644 --- a/src/renderer/src/components/terminal-pane/expand-collapse.ts +++ b/src/renderer/src/components/terminal-pane/expand-collapse.ts @@ -96,10 +96,8 @@ export function createExpandCollapseActions(state: ExpandCollapseState) { // Why: expand/collapse flips inline display/flex styles on ancestor panes // synchronously. The rAF here lets layout settle so FitAddon's // proposeDimensions reads the final rects, not the pre-toggle ones. - // xterm preserves viewportY natively across resize (see - // scroll-reflow.test.ts "reference: undisturbed"), so a bare fit() is - // enough — the content-hash capture/restore we used to do here jumped to - // the wrong duplicate scrollback line in long sessions. + // safeFit owns scroll preservation; content matching here jumped to the + // wrong duplicate scrollback line in long sessions. const refreshPaneSizes = (focusActive: boolean): void => { requestAnimationFrame(() => { const manager = state.managerRef.current diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.test.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.test.ts index 2124e20a0..44d1a6a9e 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.test.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.test.ts @@ -3,12 +3,12 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { useTerminalPaneGlobalEffects } from './use-terminal-pane-global-effects' const mocks = vi.hoisted(() => ({ - captureScrollViewportPosition: vi.fn(), + captureScrollState: vi.fn(), fitAndFocusPanes: vi.fn(), fitPanes: vi.fn(), flushTerminalOutput: vi.fn(), handleTerminalFileDrop: vi.fn(), - restoreScrollViewportPosition: vi.fn() + restoreScrollState: vi.fn() })) vi.mock('react', async (importOriginal) => { @@ -32,8 +32,8 @@ vi.mock('@/lib/pane-manager/pane-terminal-output-scheduler', () => ({ })) vi.mock('@/lib/pane-manager/pane-scroll', () => ({ - captureScrollViewportPosition: mocks.captureScrollViewportPosition, - restoreScrollViewportPosition: mocks.restoreScrollViewportPosition + captureScrollState: mocks.captureScrollState, + restoreScrollState: mocks.restoreScrollState })) vi.mock('./terminal-drop-handler', () => ({ @@ -135,11 +135,11 @@ describe('useTerminalPaneGlobalEffects', () => { mocks.flushTerminalOutput.mockImplementation((terminal: { name: string }) => { order.push(`flush:${terminal.name}`) }) - mocks.captureScrollViewportPosition.mockImplementation((terminal: { name: string }) => { + mocks.captureScrollState.mockImplementation((terminal: { name: string }) => { order.push(`capture:${terminal.name}`) return { terminalName: terminal.name } }) - mocks.restoreScrollViewportPosition.mockImplementation((terminal: { name: string }) => { + mocks.restoreScrollState.mockImplementation((terminal: { name: string }) => { order.push(`restore:${terminal.name}`) }) mocks.fitAndFocusPanes.mockImplementation(() => order.push('fit-focus')) diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts index ed3f4ca8d..028a20987 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts @@ -15,10 +15,7 @@ import { flushTerminalOutput } from '@/lib/pane-manager/pane-terminal-output-sch import { handleFocusTerminalPaneDetail } from './focus-terminal-pane-event' import { surfaceStaleAgentRow } from './stale-agent-row' import { useAppStore } from '@/store' -import { - captureScrollViewportPosition, - restoreScrollViewportPosition -} from '@/lib/pane-manager/pane-scroll' +import { captureScrollState, restoreScrollState } from '@/lib/pane-manager/pane-scroll' type UseTerminalPaneGlobalEffectsArgs = { tabId: string @@ -68,9 +65,7 @@ export function useTerminalPaneGlobalEffects({ // restore path avoids content matching so duplicate agent log lines do // not jump to the wrong history entry. const viewportPositions = new Map( - manager - .getPanes() - .map((pane) => [pane.id, captureScrollViewportPosition(pane.terminal)] as const) + manager.getPanes().map((pane) => [pane.id, captureScrollState(pane.terminal)] as const) ) // Why: background PTY output is throttled while a pane is not focused; // flush it before fitting so newly visible terminals paint current state. @@ -93,7 +88,7 @@ export function useTerminalPaneGlobalEffects({ for (const pane of manager.getPanes()) { const position = viewportPositions.get(pane.id) if (position) { - restoreScrollViewportPosition(pane.terminal, position) + restoreScrollState(pane.terminal, position) } } } else if (wasVisibleRef.current) { @@ -172,10 +167,7 @@ export function useTerminalPaneGlobalEffects({ // useLayoutEffect (pre-paint, same frame as the width change) so the // terminal fits synchronously with the new container size, eliminating the // ~16ms "old cols, new container width" flash that a deferred - // ResizeObserver rAF would otherwise produce. xterm's terminal.resize() - // natively preserves viewportY across reflows (verified in - // scroll-reflow.test.ts "reference: undisturbed"), so a bare fitAllPanes() - // is all we need — no capture/restore dance. The subsequent per-pane + // ResizeObserver rAF would otherwise produce. The subsequent per-pane // ResizeObserver rAF and the 150ms debounced global fit become no-ops // because proposeDimensions() will match current cols/rows (early-return // branch in safeFit). Listener is global (not gated on isVisible/isActive) diff --git a/src/renderer/src/constants/terminal.ts b/src/renderer/src/constants/terminal.ts index aecf27a13..a6c0cabe2 100644 --- a/src/renderer/src/constants/terminal.ts +++ b/src/renderer/src/constants/terminal.ts @@ -12,11 +12,8 @@ export const BACKGROUND_MOUNT_TERMINAL_WORKTREE_EVENT = 'orca-background-mount-t // terminal fit synchronously before paint — so the new width and the // reflowed terminal land on the same frame with no visible transient. // -// Continuous drags (sidebar-width drag, tab-group split drag) don't need -// this: the per-pane ResizeObserver rAF path is fine on its own because -// xterm's terminal.resize() natively preserves viewportY across reflows -// (verified in scroll-reflow.test.ts "reference: undisturbed"). This is -// how Superset and VSCode handle the same case. +// Continuous drags (sidebar-width drag, tab-group split drag) use the +// per-pane ResizeObserver path instead. export const SYNC_FIT_PANES_EVENT = 'orca-sync-fit-panes' export type ToggleTerminalPaneExpandDetail = { diff --git a/src/renderer/src/lib/pane-manager/pane-fit-resize-observer.test.ts b/src/renderer/src/lib/pane-manager/pane-fit-resize-observer.test.ts index acd20726e..73ab8c9b4 100644 --- a/src/renderer/src/lib/pane-manager/pane-fit-resize-observer.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-fit-resize-observer.test.ts @@ -67,9 +67,8 @@ function createPane(): ManagedPaneInternal { pendingSplitScrollState: { bufferType: 'normal', wasAtBottom: true, - firstVisibleLineContent: '', viewportY: 0, - totalLines: 24 + baseY: 0 } satisfies ScrollState } } diff --git a/src/renderer/src/lib/pane-manager/pane-manager-types.ts b/src/renderer/src/lib/pane-manager/pane-manager-types.ts index e2aa6d8a5..c22de15cd 100644 --- a/src/renderer/src/lib/pane-manager/pane-manager-types.ts +++ b/src/renderer/src/lib/pane-manager/pane-manager-types.ts @@ -81,9 +81,8 @@ export type ManagedPane = { export type ScrollState = { bufferType: 'normal' | 'alternate' wasAtBottom: boolean - firstVisibleLineContent: string viewportY: number - totalLines: number + baseY: number } export type ManagedPaneInternal = { @@ -108,11 +107,8 @@ export type ManagedPaneInternal = { webLinksAddon: WebLinksAddon // Stored so disposePane() can remove it and avoid a memory leak. compositionHandler: (() => void) | null - // Why: during splitPane, multiple async operations (rAFs, ResizeObserver - // debounce, WebGL context loss) may independently attempt scroll - // restoration. This field acts as a lock: when set, safeFit and other - // intermediate fit paths skip their own scroll restoration, deferring to - // the splitPane's final authoritative restore. + // Why: splitPane reparents DOM; its delayed restore owns scroll until the + // browser settles, so intermediate fits must not compete with it. pendingSplitScrollState: ScrollState | null debugLabel: string | null } & ManagedPane diff --git a/src/renderer/src/lib/pane-manager/pane-scroll.test.ts b/src/renderer/src/lib/pane-manager/pane-scroll.test.ts index becc43540..a1beb6563 100644 --- a/src/renderer/src/lib/pane-manager/pane-scroll.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-scroll.test.ts @@ -1,16 +1,11 @@ import { describe, expect, it, vi } from 'vitest' import type { Terminal } from '@xterm/xterm' -import { - captureScrollViewportPosition, - restoreScrollViewportPosition, - type ScrollViewportPosition -} from './pane-scroll' +import { captureScrollState, restoreScrollState } from './pane-scroll' +import type { ScrollState } from './pane-manager-types' function createTerminal(args: { viewportY: number baseY: number - cols: number - rows: number type?: 'normal' | 'alternate' }): Terminal { const active = { @@ -19,8 +14,6 @@ function createTerminal(args: { baseY: args.baseY } return { - cols: args.cols, - rows: args.rows, buffer: { active }, scrollToBottom: vi.fn(() => { active.viewportY = active.baseY @@ -34,100 +27,73 @@ function createTerminal(args: { } as unknown as Terminal } -describe('scroll viewport position', () => { +describe('scroll state', () => { it('captures the numeric viewport position', () => { - const terminal = createTerminal({ viewportY: 42, baseY: 100, cols: 120, rows: 32 }) + const terminal = createTerminal({ viewportY: 42, baseY: 100 }) - expect(captureScrollViewportPosition(terminal)).toEqual({ + expect(captureScrollState(terminal)).toEqual({ bufferType: 'normal', wasAtBottom: false, viewportY: 42, - baseY: 100, - cols: 120, - rows: 32 + baseY: 100 }) }) - it('restores the same viewport line when the terminal grid did not reflow', () => { - const terminal = createTerminal({ viewportY: 10, baseY: 100, cols: 120, rows: 32 }) - const state: ScrollViewportPosition = { + it('restores the captured viewport line', () => { + const terminal = createTerminal({ viewportY: 10, baseY: 100 }) + const state: ScrollState = { bufferType: 'normal', wasAtBottom: false, viewportY: 42, - baseY: 100, - cols: 120, - rows: 32 + baseY: 100 } - restoreScrollViewportPosition(terminal, state) + restoreScrollState(terminal, state) expect(terminal.scrollToLine).toHaveBeenCalledWith(42) expect(terminal.buffer.active.viewportY).toBe(42) }) it('clamps the restored viewport line to the current buffer bottom', () => { - const terminal = createTerminal({ viewportY: 10, baseY: 30, cols: 120, rows: 32 }) - const state: ScrollViewportPosition = { + const terminal = createTerminal({ viewportY: 10, baseY: 30 }) + const state: ScrollState = { bufferType: 'normal', wasAtBottom: false, viewportY: 42, - baseY: 100, - cols: 120, - rows: 32 + baseY: 100 } - restoreScrollViewportPosition(terminal, state) + restoreScrollState(terminal, state) expect(terminal.scrollToLine).toHaveBeenCalledWith(30) expect(terminal.buffer.active.viewportY).toBe(30) }) - it('does not restore across normal and alternate buffers', () => { - const terminal = createTerminal({ viewportY: 10, baseY: 100, cols: 120, rows: 32 }) - const state: ScrollViewportPosition = { - bufferType: 'alternate', - wasAtBottom: false, - viewportY: 42, - baseY: 100, - cols: 120, - rows: 32 - } - - restoreScrollViewportPosition(terminal, state) - - expect(terminal.scrollToLine).not.toHaveBeenCalled() - expect(terminal.buffer.active.viewportY).toBe(10) - }) - it('scrolls to the current bottom when the pane was previously at bottom', () => { - const terminal = createTerminal({ viewportY: 10, baseY: 250, cols: 120, rows: 32 }) - const state: ScrollViewportPosition = { + const terminal = createTerminal({ viewportY: 10, baseY: 250 }) + const state: ScrollState = { bufferType: 'normal', wasAtBottom: true, viewportY: 100, - baseY: 100, - cols: 120, - rows: 32 + baseY: 100 } - restoreScrollViewportPosition(terminal, state) + restoreScrollState(terminal, state) expect(terminal.scrollToBottom).toHaveBeenCalledTimes(1) expect(terminal.buffer.active.viewportY).toBe(250) }) - it('does not numerically restore a non-bottom viewport after a grid reflow', () => { - const terminal = createTerminal({ viewportY: 10, baseY: 100, cols: 80, rows: 32 }) - const state: ScrollViewportPosition = { - bufferType: 'normal', + it('does not restore across normal and alternate buffers', () => { + const terminal = createTerminal({ viewportY: 10, baseY: 100 }) + const state: ScrollState = { + bufferType: 'alternate', wasAtBottom: false, viewportY: 42, - baseY: 100, - cols: 120, - rows: 32 + baseY: 100 } - restoreScrollViewportPosition(terminal, state) + restoreScrollState(terminal, state) expect(terminal.scrollToLine).not.toHaveBeenCalled() expect(terminal.buffer.active.viewportY).toBe(10) diff --git a/src/renderer/src/lib/pane-manager/pane-scroll.ts b/src/renderer/src/lib/pane-manager/pane-scroll.ts index b2345a327..d4d1bb449 100644 --- a/src/renderer/src/lib/pane-manager/pane-scroll.ts +++ b/src/renderer/src/lib/pane-manager/pane-scroll.ts @@ -1,105 +1,19 @@ import type { Terminal } from '@xterm/xterm' import type { ScrollState } from './pane-manager-types' -export type ScrollViewportPosition = { - bufferType: 'normal' | 'alternate' - wasAtBottom: boolean - viewportY: number - baseY: number - cols: number - rows: number -} - -// --------------------------------------------------------------------------- -// Scroll restoration after reflow -// --------------------------------------------------------------------------- - -// Why: xterm.js does NOT adjust viewportY for partially-scrolled buffers -// during resize/reflow. Line N before reflow shows different content than -// line N after reflow when wrapping changes (e.g. 80→40 cols makes each -// line wrap to 2 rows). To preserve the user's scroll position, we find -// the buffer line whose content matches what was at the top of the viewport -// before the reflow, then scroll to it. -// -// Why hintRatio: terminals frequently contain duplicate short lines (shell -// prompts, repeated log prefixes). A prefix-only search returns the first -// match which may be far from the actual scroll position. The proportional -// hint (viewportY / totalLines before reflow) disambiguates by preferring -// the match closest to the expected position in the reflowed buffer. -export function findLineByContent(terminal: Terminal, content: string, hintRatio?: number): number { - if (!content) { - return -1 - } - const buf = terminal.buffer.active - const totalLines = buf.baseY + terminal.rows - const prefix = content.substring(0, Math.min(content.length, 40)) - if (!prefix) { - return -1 - } - - const hintLine = hintRatio !== undefined ? Math.round(hintRatio * totalLines) : -1 - - let bestMatch = -1 - let bestDistance = Infinity - - for (let i = 0; i < totalLines; i++) { - const line = buf.getLine(i)?.translateToString(true)?.trimEnd() ?? '' - if (line.startsWith(prefix)) { - if (hintLine < 0) { - return i - } - const distance = Math.abs(i - hintLine) - if (distance < bestDistance) { - bestDistance = distance - bestMatch = i - } - } - } - return bestMatch -} - export function captureScrollState(terminal: Terminal): ScrollState { - const buf = terminal.buffer.active - const bufferType = buf.type - const viewportY = buf.viewportY - const wasAtBottom = viewportY >= buf.baseY - const firstVisibleLineContent = buf.getLine(viewportY)?.translateToString(true)?.trimEnd() ?? '' - const totalLines = buf.baseY + terminal.rows - return { bufferType, wasAtBottom, firstVisibleLineContent, viewportY, totalLines } -} - -export function restoreScrollState(terminal: Terminal, state: ScrollState): void { - if (state.wasAtBottom) { - terminal.scrollToBottom() - forceViewportScrollbarSync(terminal) - return - } - const hintRatio = state.totalLines > 0 ? state.viewportY / state.totalLines : undefined - const target = findLineByContent(terminal, state.firstVisibleLineContent, hintRatio) - if (target >= 0) { - terminal.scrollToLine(target) - forceViewportScrollbarSync(terminal) - } -} - -export function captureScrollViewportPosition(terminal: Terminal): ScrollViewportPosition { const buf = terminal.buffer.active return { bufferType: buf.type, wasAtBottom: buf.viewportY >= buf.baseY, viewportY: buf.viewportY, - baseY: buf.baseY, - cols: terminal.cols, - rows: terminal.rows + baseY: buf.baseY } } -export function restoreScrollViewportPosition( - terminal: Terminal, - state: ScrollViewportPosition -): void { +export function restoreScrollState(terminal: Terminal, state: ScrollState): void { const buf = terminal.buffer.active - if (buf.type !== state.bufferType) { + if (state.bufferType === 'alternate' || buf.type !== state.bufferType) { return } @@ -109,24 +23,12 @@ export function restoreScrollViewportPosition( return } - if (terminal.cols !== state.cols || terminal.rows !== state.rows) { - return - } - - // Why: worktree switches can resume WebGL with the same terminal grid but - // stale viewport internals. Restore by numeric viewport only when the grid - // did not reflow, avoiding duplicate-content matching in long agent logs. terminal.scrollToLine(Math.min(state.viewportY, buf.baseY)) forceViewportScrollbarSync(terminal) } -// Why: xterm 6's Viewport._sync() updates scrollDimensions after resize but -// skips the scrollPosition update when ydisp matches _latestYDisp (a stale -// internal value). This leaves the scrollbar thumb at a wrong position even -// though the rendered content is correct. A scroll jiggle (-1/+1) in the -// same JS turn forces _sync() to fire with a differing ydisp, which triggers -// setScrollPosition and syncs the scrollbar. No paint occurs between the two -// synchronous calls so the intermediate state is never visible. +// Why: xterm 6 can leave its scrollbar thumb stale when ydisp is unchanged. +// A synchronous one-line jiggle updates the scrollbar without a visible paint. function forceViewportScrollbarSync(terminal: Terminal): void { const buf = terminal.buffer.active if (buf.viewportY > 0) { diff --git a/src/renderer/src/lib/pane-manager/pane-split-close.test.ts b/src/renderer/src/lib/pane-manager/pane-split-close.test.ts index 93ef7198d..75623ce6c 100644 --- a/src/renderer/src/lib/pane-manager/pane-split-close.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-split-close.test.ts @@ -72,9 +72,8 @@ function createScrollState(viewportY: number): ScrollState { return { bufferType: 'normal', wasAtBottom: false, - firstVisibleLineContent: `line-${viewportY}`, viewportY, - totalLines: 100 + baseY: 100 } } diff --git a/src/renderer/src/lib/pane-manager/pane-split-scroll.test.ts b/src/renderer/src/lib/pane-manager/pane-split-scroll.test.ts index 4ff8c5eb8..95d56eb01 100644 --- a/src/renderer/src/lib/pane-manager/pane-split-scroll.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-split-scroll.test.ts @@ -13,9 +13,8 @@ import { scheduleSplitScrollRestore } from './pane-split-scroll' const scrollState = { bufferType: 'normal', wasAtBottom: true, - firstVisibleLineContent: '', viewportY: 0, - totalLines: 24 + baseY: 0 } satisfies ScrollState const alternateScrollState = { diff --git a/src/renderer/src/lib/pane-manager/pane-split-scroll.ts b/src/renderer/src/lib/pane-manager/pane-split-scroll.ts index b8b00747c..fa2a25669 100644 --- a/src/renderer/src/lib/pane-manager/pane-split-scroll.ts +++ b/src/renderer/src/lib/pane-manager/pane-split-scroll.ts @@ -47,62 +47,6 @@ function restoreCapturedScrollState( refreshAfterReparent(pane) } -function logPaneHealth(pane: ManagedPaneInternal, phase: string): void { - const canvases = pane.container.querySelectorAll('canvas') - const canvasInfo = Array.from(canvases).map((c) => { - const gl = c.getContext('webgl2') ?? c.getContext('webgl') - return { - w: c.width, - h: c.height, - inDOM: c.isConnected, - ctxLost: gl ? gl.isContextLost() : 'no-ctx' - } - }) - const content = pane.serializeAddon?.serialize?.() ?? '' - // oxlint-disable-next-line no-control-regex - const stripped = content.replace(/[\s\x00-\x1f]/g, '') - const info = { - phase, - paneId: pane.id, - webgl: !!pane.webglAddon, - webglDeferred: pane.webglAttachmentDeferred, - webglDisabled: pane.webglDisabledAfterContextLoss, - canvases: canvasInfo, - contentLen: stripped.length, - bufferLines: pane.terminal.buffer.active.length - } - const hasBufferData = pane.terminal.buffer.active.length > pane.terminal.rows - if (stripped.length === 0 && hasBufferData) { - console.error( - '[split-diag] DEAD TERMINAL — pane', - pane.id, - pane.debugLabel ?? '', - 'has buffer data but no rendered content at', - phase, - info - ) - } else if (stripped.length === 0) { - console.log( - '[split-diag] pane', - pane.id, - pane.debugLabel ?? '', - 'no content yet at', - phase, - '(PTY likely still spawning)' - ) - } else { - console.log( - '[split-diag] pane', - pane.id, - pane.debugLabel ?? '', - 'healthy at', - phase, - '— content:', - stripped.length - ) - } -} - // Why: reparenting a terminal container during split resets the viewport // scroll position (browser clears scrollTop on DOM move). This schedules a // two-phase restore: an early double-rAF (~32ms) to minimise the visible @@ -178,15 +122,4 @@ export function scheduleSplitScrollRestore( } restoreCapturedScrollState(live, scrollState, reattachWebgl) }, 200) - - setTimeout(() => { - if (isDestroyed()) { - return - } - const live = getPaneById(paneId) - // Skip suspended panes — they have no WebGL/content by design. - if (live && !live.webglAttachmentDeferred) { - logPaneHealth(live, '1s-health-check') - } - }, 1000) } diff --git a/src/renderer/src/lib/pane-manager/pane-tree-ops.test.ts b/src/renderer/src/lib/pane-manager/pane-tree-ops.test.ts index 577dd8e02..6ef216840 100644 --- a/src/renderer/src/lib/pane-manager/pane-tree-ops.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-tree-ops.test.ts @@ -49,14 +49,22 @@ function createPane({ refresh: vi.fn(), buffer: { active: { + type: 'normal', viewportY: 0, baseY: 0, getLine: vi.fn(() => ({ translateToString: () => '' })) } }, scrollToBottom: vi.fn(), - scrollToLine: vi.fn(), - scrollLines: vi.fn() + scrollToLine: vi.fn((line: number) => { + terminal.buffer.active.viewportY = line + }), + scrollLines: vi.fn((delta: number) => { + terminal.buffer.active.viewportY = Math.max( + 0, + Math.min(terminal.buffer.active.baseY, terminal.buffer.active.viewportY + delta) + ) + }) } return { @@ -104,6 +112,26 @@ describe('safeFit', () => { expect(pane.fitAddon.fit).not.toHaveBeenCalled() }) + it('does not restore scroll for no-op drag-frame refits', () => { + const pane = createPane({ + proposedCols: 120, + proposedRows: 32, + terminalCols: 120, + terminalRows: 32 + }) + const activeBuffer = pane.terminal.buffer.active as { viewportY: number; baseY: number } + activeBuffer.viewportY = 42 + activeBuffer.baseY = 100 + + safeFit(pane) + + expect(pane.fitAddon.fit).not.toHaveBeenCalled() + expect(pane.terminal.scrollToLine).not.toHaveBeenCalled() + expect(pane.terminal.scrollToBottom).not.toHaveBeenCalled() + expect(pane.terminal.scrollLines).not.toHaveBeenCalled() + expect(activeBuffer.viewportY).toBe(42) + }) + it('still refits when the proposed grid dimensions changed', () => { const pane = createPane({ proposedCols: 100, @@ -117,6 +145,27 @@ describe('safeFit', () => { expect(pane.fitAddon.fit).toHaveBeenCalledTimes(1) }) + it('restores the viewport if fit clobbers it during resize', () => { + const pane = createPane({ + proposedCols: 100, + proposedRows: 32, + terminalCols: 120, + terminalRows: 32 + }) + const activeBuffer = pane.terminal.buffer.active as { viewportY: number; baseY: number } + activeBuffer.viewportY = 42 + activeBuffer.baseY = 100 + vi.mocked(pane.fitAddon.fit).mockImplementation(() => { + activeBuffer.viewportY = 0 + }) + + safeFit(pane) + + expect(pane.fitAddon.fit).toHaveBeenCalledTimes(1) + expect(pane.terminal.scrollToLine).toHaveBeenCalledWith(42) + expect(activeBuffer.viewportY).toBe(42) + }) + it('still refits when a split-scroll lock is active and the grid changed', () => { const pane = createPane({ proposedCols: 100, @@ -127,9 +176,8 @@ describe('safeFit', () => { pane.pendingSplitScrollState = { bufferType: 'normal', wasAtBottom: true, - firstVisibleLineContent: '', viewportY: 0, - totalLines: 32 + baseY: 0 } satisfies ScrollState safeFit(pane) diff --git a/src/renderer/src/lib/pane-manager/pane-tree-ops.ts b/src/renderer/src/lib/pane-manager/pane-tree-ops.ts index cdb97e47f..377d7fee3 100644 --- a/src/renderer/src/lib/pane-manager/pane-tree-ops.ts +++ b/src/renderer/src/lib/pane-manager/pane-tree-ops.ts @@ -2,13 +2,15 @@ import type { DropZone, ManagedPane, ManagedPaneInternal, - PaneStyleOptions + PaneStyleOptions, + ScrollState } from './pane-manager-types' import { createDivider } from './pane-divider' import { getFitOverrideForPty } from './mobile-fit-overrides' import { disposeWebgl, attachWebgl } from './pane-webgl-renderer' +import { captureScrollState, restoreScrollState } from './pane-scroll' -export { findLineByContent, captureScrollState, restoreScrollState } from './pane-scroll' +export { captureScrollState, restoreScrollState } from './pane-scroll' // --------------------------------------------------------------------------- // Split-tree manipulation: detach, insert, promote sibling @@ -30,17 +32,16 @@ function getProposedDimensions(pane: ManagedPane): { cols: number; rows: number } } -// Why: xterm's terminal.resize() (called by fitAddon.fit()) natively preserves -// viewportY across reflows — see scroll-reflow.test.ts "reference: undisturbed". -// A plain fit() is all we need during sidebar drags, divider drags, and window -// resizes. This matches how Superset and VSCode handle the same cases. -// -// pendingSplitScrollState is the one case where fit() alone isn't enough: -// wrapInSplit() reparents the container, which makes the browser reset -// scrollTop to 0 asynchronously. splitPane captures the pre-split state and -// scheduleSplitScrollRestore owns the authoritative restore on a timer, so -// safeFit here just fits and lets the scheduled restore do its job. +function captureScrollStateForFit(pane: ManagedPane): ScrollState | null { + // Why: split reparent has its own delayed restore; restoring here can fight that timer. + return 'pendingSplitScrollState' in pane && (pane as ManagedPaneInternal).pendingSplitScrollState + ? null + : captureScrollState(pane.terminal) +} + export function safeFit(pane: ManagedPane): void { + let scrollState: ScrollState | null = null + let shouldRestoreScroll = false try { // Why: when a mobile client has resized this PTY to phone dimensions, // the desktop must keep xterm at those dimensions instead of fitting to @@ -51,6 +52,8 @@ export function safeFit(pane: ManagedPane): void { const override = ptyId ? getFitOverrideForPty(ptyId) : null if (override) { if (pane.terminal.cols !== override.cols || pane.terminal.rows !== override.rows) { + scrollState = captureScrollStateForFit(pane) + shouldRestoreScroll = true pane.terminal.resize(override.cols, override.rows) } return @@ -63,9 +66,15 @@ export function safeFit(pane: ManagedPane): void { // churn, which was causing visible terminal blinking while resizing. return } + scrollState = captureScrollStateForFit(pane) + shouldRestoreScroll = true pane.fitAddon.fit() } catch { // Container may not have dimensions yet + } finally { + if (shouldRestoreScroll && scrollState) { + restoreScrollState(pane.terminal, scrollState) + } } } diff --git a/src/renderer/src/lib/pane-manager/scroll-reflow.test.ts b/src/renderer/src/lib/pane-manager/scroll-reflow.test.ts deleted file mode 100644 index 7ee5021fc..000000000 --- a/src/renderer/src/lib/pane-manager/scroll-reflow.test.ts +++ /dev/null @@ -1,305 +0,0 @@ -import { describe, expect, it } from 'vitest' -import { Terminal } from '@xterm/headless' -import type { Terminal as XtermTerminal } from '@xterm/xterm' -import { findLineByContent } from './pane-tree-ops' - -/** - * These tests verify what xterm.js actually does to viewportY (ydisp) - * during terminal.resize() when column count changes cause line reflow. - * Understanding this behavior is critical for preserving scroll position - * when splitting terminal panes (which narrows the terminal). - */ - -function writeSync(term: Terminal, data: string): Promise { - return new Promise((resolve) => term.write(data, resolve)) -} - -async function createTerminalWithContentAsync( - cols: number, - rows: number, - scrollback: number, - lineCount: number -): Promise { - const term = new Terminal({ cols, rows, scrollback, allowProposedApi: true }) - // headless terminal doesn't need open() - for (let i = 0; i < lineCount; i++) { - const line = `L${String(i).padStart(3, '0')}${'x'.repeat(cols - 4)}` - await writeSync(term, `${line}\r\n`) - } - return term -} - -describe('xterm.js scroll position during reflow', () => { - it('reports buffer state correctly', async () => { - const term = await createTerminalWithContentAsync(80, 24, 1000, 100) - const buf = term.buffer.active - // 100 lines of content, 24 visible rows - expect(buf.baseY).toBeGreaterThan(0) - // By default after writing, terminal should be at bottom - expect(buf.viewportY).toBe(buf.baseY) - term.dispose() - }) - - it('scrollToLine sets viewportY', async () => { - const term = await createTerminalWithContentAsync(80, 24, 1000, 100) - term.scrollToLine(10) - expect(term.buffer.active.viewportY).toBe(10) - term.dispose() - }) - - describe('resize from wider to narrower (split scenario)', () => { - it('when at bottom: resize adjusts viewportY to stay at bottom', async () => { - const term = await createTerminalWithContentAsync(80, 24, 1000, 100) - const buf = term.buffer.active - - expect(buf.viewportY).toBe(buf.baseY) - const oldBaseY = buf.baseY - - // Simulate split: narrow from 80 to 40 cols - term.resize(40, 24) - - // After narrowing, lines wrap → more total lines → baseY increases - expect(buf.baseY).toBeGreaterThanOrEqual(oldBaseY) - // Does xterm keep us at the bottom? - console.log( - `[at-bottom] old baseY=${oldBaseY}, new baseY=${buf.baseY}, ` + - `viewportY=${buf.viewportY}, at-bottom=${buf.viewportY >= buf.baseY}` - ) - - term.dispose() - }) - - it('when scrolled up: captures how xterm adjusts viewportY during reflow', async () => { - const term = await createTerminalWithContentAsync(80, 24, 1000, 100) - const buf = term.buffer.active - - // Scroll to line 30 - term.scrollToLine(30) - expect(buf.viewportY).toBe(30) - - const oldViewportY = buf.viewportY - const oldBaseY = buf.baseY - - // Simulate split: narrow from 80 to 40 cols - term.resize(40, 24) - - console.log( - `[scrolled-up] old viewportY=${oldViewportY}, old baseY=${oldBaseY}, ` + - `new viewportY=${buf.viewportY}, new baseY=${buf.baseY}` - ) - - term.dispose() - }) - - it('when scrolled up: does scrollToLine before resize help?', async () => { - const term = await createTerminalWithContentAsync(80, 24, 1000, 100) - const buf = term.buffer.active - - // Scroll to line 30 - term.scrollToLine(30) - expect(buf.viewportY).toBe(30) - - const savedViewportY = buf.viewportY - const oldBaseY = buf.baseY - - // Simulate browser clobbering scroll to 0 - term.scrollToLine(0) - expect(buf.viewportY).toBe(0) - - // Strategy A: restore scrollToLine BEFORE resize - term.scrollToLine(savedViewportY) - expect(buf.viewportY).toBe(savedViewportY) - - term.resize(40, 24) - - console.log( - `[strategy-A: restore-before-resize] saved=${savedViewportY}, ` + - `old baseY=${oldBaseY}, new viewportY=${buf.viewportY}, new baseY=${buf.baseY}` - ) - - term.dispose() - }) - - it('when scrolled up: does scrollToLine after resize work?', async () => { - const term = await createTerminalWithContentAsync(80, 24, 1000, 100) - const buf = term.buffer.active - - // Scroll to line 30 - term.scrollToLine(30) - const savedViewportY = buf.viewportY - const oldBaseY = buf.baseY - - // Simulate browser clobbering scroll to 0 - term.scrollToLine(0) - - // Strategy B: resize first (from clobbered state), then restore - term.resize(40, 24) - const postResizeViewportY = buf.viewportY - const newBaseY = buf.baseY - - // Now try to restore with the old viewportY - term.scrollToLine(savedViewportY) - - console.log( - `[strategy-B: restore-after-resize] saved=${savedViewportY}, ` + - `old baseY=${oldBaseY}, post-resize viewportY=${postResizeViewportY}, ` + - `new baseY=${newBaseY}, final viewportY=${buf.viewportY}` - ) - - term.dispose() - }) - - it('ratio-based restoration after resize', async () => { - const term = await createTerminalWithContentAsync(80, 24, 1000, 100) - const buf = term.buffer.active - - term.scrollToLine(30) - const savedViewportY = buf.viewportY - const oldBaseY = buf.baseY - const ratio = oldBaseY > 0 ? savedViewportY / oldBaseY : 0 - - // Simulate browser clobbering scroll to 0 - term.scrollToLine(0) - - // Resize (from clobbered state) - term.resize(40, 24) - const newBaseY = buf.baseY - - // Strategy C: restore using ratio - const targetLine = Math.round(ratio * newBaseY) - term.scrollToLine(targetLine) - - console.log( - `[strategy-C: ratio] saved=${savedViewportY}, old baseY=${oldBaseY}, ` + - `ratio=${ratio.toFixed(3)}, new baseY=${newBaseY}, ` + - `target=${targetLine}, final viewportY=${buf.viewportY}` - ) - - term.dispose() - }) - - it('getline-based: find first visible line content and match after reflow', async () => { - const term = await createTerminalWithContentAsync(80, 24, 1000, 100) - const buf = term.buffer.active - - term.scrollToLine(30) - const savedViewportY = buf.viewportY - - // Capture the text of the first visible line - const firstVisibleLine = buf.getLine(savedViewportY)?.translateToString(true) ?? '' - const firstVisiblePrefix = firstVisibleLine.substring(0, 10) - - // Simulate browser clobbering scroll to 0 - term.scrollToLine(0) - - // Resize (from clobbered state) - term.resize(40, 24) - const newBaseY = buf.baseY - - // Strategy D: scan for the line with matching content - let matchLine = -1 - for (let i = 0; i <= newBaseY + 24; i++) { - const line = buf.getLine(i)?.translateToString(true) ?? '' - if (line.startsWith(firstVisiblePrefix)) { - matchLine = i - break - } - } - - if (matchLine >= 0) { - term.scrollToLine(matchLine) - } - - console.log( - `[strategy-D: content-match] looking for "${firstVisiblePrefix}", ` + - `found at line ${matchLine}, viewportY=${buf.viewportY}, ` + - `saved was ${savedViewportY}, new baseY=${newBaseY}` - ) - - term.dispose() - }) - - it('distance-from-bottom preservation', async () => { - const term = await createTerminalWithContentAsync(80, 24, 1000, 100) - const buf = term.buffer.active - - term.scrollToLine(30) - const savedViewportY = buf.viewportY - const oldBaseY = buf.baseY - const distFromBottom = oldBaseY - savedViewportY - - // Simulate browser clobbering scroll to 0 - term.scrollToLine(0) - - // Resize (from clobbered state) - term.resize(40, 24) - const newBaseY = buf.baseY - - // Strategy E: preserve distance from bottom - const targetLine = Math.max(0, newBaseY - distFromBottom) - term.scrollToLine(targetLine) - - console.log( - `[strategy-E: dist-from-bottom] saved=${savedViewportY}, ` + - `old baseY=${oldBaseY}, distFromBottom=${distFromBottom}, ` + - `new baseY=${newBaseY}, target=${targetLine}, ` + - `final viewportY=${buf.viewportY}` - ) - - term.dispose() - }) - }) - - describe('reference: what does undisturbed resize do?', () => { - it('resize without any scroll clobbering (ideal reference)', async () => { - const term = await createTerminalWithContentAsync(80, 24, 1000, 100) - const buf = term.buffer.active - - term.scrollToLine(30) - const savedViewportY = buf.viewportY - const oldBaseY = buf.baseY - - // DON'T clobber scroll — just resize directly - term.resize(40, 24) - - console.log( - `[reference: undisturbed] saved=${savedViewportY}, old baseY=${oldBaseY}, ` + - `new viewportY=${buf.viewportY}, new baseY=${buf.baseY}` - ) - - // This is the gold standard — what xterm does natively - term.dispose() - }) - }) - - describe('findLineByContent after reflow', () => { - it('finds the correct line after narrowing', async () => { - const term = await createTerminalWithContentAsync(80, 24, 1000, 100) - const buf = term.buffer.active - - term.scrollToLine(30) - const firstVisibleContent = buf.getLine(30)?.translateToString(true)?.trimEnd() ?? '' - - term.resize(40, 24) - - // findLineByContent should locate the same content after reflow - const target = findLineByContent(term as unknown as XtermTerminal, firstVisibleContent) - expect(target).toBeGreaterThan(0) - - // After scrolling to target, the first visible line should contain - // the same prefix as before the reflow - term.scrollToLine(target) - const afterContent = buf.getLine(target)?.translateToString(true)?.trimEnd() ?? '' - expect(afterContent.startsWith(firstVisibleContent.substring(0, 10))).toBe(true) - - term.dispose() - }) - - it('returns -1 for empty content', async () => { - const term = await createTerminalWithContentAsync(80, 24, 1000, 10) - const target = findLineByContent(term as unknown as XtermTerminal, '') - expect(target).toBe(-1) - term.dispose() - }) - }) -})