diff --git a/src/renderer/src/components/terminal-pane/use-terminal-scroll-visibility-memory.test.ts b/src/renderer/src/components/terminal-pane/use-terminal-scroll-visibility-memory.test.ts index c30d96a5387..00d1df67849 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-scroll-visibility-memory.test.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-scroll-visibility-memory.test.ts @@ -11,7 +11,8 @@ const mocks = vi.hoisted(() => ({ baseY: 0 })), flushTerminalOutput: vi.fn(), - getTerminalOutputEpoch: vi.fn(() => 1) + getTerminalOutputEpoch: vi.fn(() => 1), + isTerminalScrollRestoreInProgress: vi.fn(() => false) })) const reactRefState = vi.hoisted(() => ({ @@ -65,7 +66,8 @@ vi.mock('@/lib/pane-manager/pane-terminal-output-scheduler', () => ({ vi.mock('@/lib/pane-manager/pane-scroll', () => ({ cancelDeferredScrollRestore: mocks.cancelDeferredScrollRestore, captureScrollState: mocks.captureScrollState, - getTerminalOutputEpoch: mocks.getTerminalOutputEpoch + getTerminalOutputEpoch: mocks.getTerminalOutputEpoch, + isTerminalScrollRestoreInProgress: mocks.isTerminalScrollRestoreInProgress })) describe('useTerminalScrollVisibilityMemory', () => { @@ -149,4 +151,51 @@ describe('useTerminalScrollVisibilityMemory', () => { expect(cancelAnimationFrame).toHaveBeenCalledWith(7) expect(mocks.flushTerminalOutput).not.toHaveBeenCalled() }) + + it('does not remember scroll events caused by a deferred restore', () => { + const onScrollListeners: (() => void)[] = [] + const terminal = { + onScroll: vi.fn((listener: () => void) => { + onScrollListeners.push(listener) + return { dispose: vi.fn() } + }) + } + const manager = { + getPanes: vi.fn(() => [{ id: 1, terminal }]) + } + const userScrolledState = { + bufferType: 'normal', + wasAtBottom: false, + viewportY: 42, + baseY: 100 + } + const restoreBottomState = { + bufferType: 'normal', + wasAtBottom: true, + viewportY: 100, + baseY: 100 + } + mocks.captureScrollState + .mockReturnValueOnce(userScrolledState) + .mockReturnValueOnce(restoreBottomState) + + beginHookRender() + const visibilityMemory = useTerminalScrollVisibilityMemory({ + managerRef: { current: manager as never }, + isVisibleRef: { current: true }, + visibleResumeCompleteRef: { current: true }, + paneCount: 1 + }) + + const listener = onScrollListeners[0] + if (!listener) { + throw new Error('expected onScroll listener to be registered') + } + listener() + mocks.isTerminalScrollRestoreInProgress.mockReturnValueOnce(true) + listener() + + expect(visibilityMemory.captureViewportPositions(true).get(1)).toBe(userScrolledState) + expect(mocks.captureScrollState).toHaveBeenCalledTimes(1) + }) }) diff --git a/src/renderer/src/components/terminal-pane/use-terminal-scroll-visibility-memory.ts b/src/renderer/src/components/terminal-pane/use-terminal-scroll-visibility-memory.ts index 182ef2741e9..c93d04623e9 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-scroll-visibility-memory.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-scroll-visibility-memory.ts @@ -4,7 +4,8 @@ import { flushTerminalOutput } from '@/lib/pane-manager/pane-terminal-output-sch import { cancelDeferredScrollRestore, captureScrollState, - getTerminalOutputEpoch + getTerminalOutputEpoch, + isTerminalScrollRestoreInProgress } from '@/lib/pane-manager/pane-scroll' import type { PaneManager } from '@/lib/pane-manager/pane-manager' import type { ScrollState } from '@/lib/pane-manager/pane-manager-types' @@ -190,7 +191,11 @@ export function useTerminalScrollVisibilityMemory({ disposables.set( pane.id, onScroll.call(pane.terminal, () => { - if (!isVisibleRef.current || suppressScrollTrackingRef.current) { + if ( + !isVisibleRef.current || + suppressScrollTrackingRef.current || + isTerminalScrollRestoreInProgress(pane.terminal) + ) { return } rememberVisibleScrollSnapshot(pane.id, pane.terminal) 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 82c96abc1e4..def9d95fffc 100644 --- a/src/renderer/src/lib/pane-manager/pane-scroll.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-scroll.test.ts @@ -3,6 +3,7 @@ import type { IMarker, Terminal } from '@xterm/xterm' import { captureScrollState, getTerminalOutputEpoch, + isTerminalScrollRestoreInProgress, recordTerminalOutput, restoreScrollState, restoreScrollStateAfterLayout @@ -104,6 +105,27 @@ describe('scroll state', () => { expect(terminal.buffer.active.viewportY).toBe(42) }) + it('marks imperative restore scrolls through the current task', () => { + vi.useFakeTimers() + const terminal = createTerminal({ viewportY: 10, baseY: 100 }) + ;(terminal.scrollToLine as ReturnType).mockImplementation((line: number) => { + expect(isTerminalScrollRestoreInProgress(terminal)).toBe(true) + ;(terminal.buffer.active as { viewportY: number }).viewportY = line + }) + const state: ScrollState = { + bufferType: 'normal', + wasAtBottom: false, + viewportY: 42, + baseY: 100 + } + + restoreScrollState(terminal, state) + + expect(isTerminalScrollRestoreInProgress(terminal)).toBe(true) + vi.advanceTimersByTime(0) + expect(isTerminalScrollRestoreInProgress(terminal)).toBe(false) + }) + it('skips restore when the terminal element is gone (post WebGL teardown)', () => { const terminal = createTerminal({ viewportY: 10, baseY: 100 }) ;(terminal as unknown as { element: HTMLElement | undefined }).element = undefined diff --git a/src/renderer/src/lib/pane-manager/pane-scroll.ts b/src/renderer/src/lib/pane-manager/pane-scroll.ts index 1a974f2072c..d63eb9fe911 100644 --- a/src/renderer/src/lib/pane-manager/pane-scroll.ts +++ b/src/renderer/src/lib/pane-manager/pane-scroll.ts @@ -7,6 +7,7 @@ import { } from './terminal-scroll-restore-debug' const terminalOutputEpochs = new WeakMap() +const terminalScrollRestoreDepths = new WeakMap() const deferredScrollRestores = new WeakMap< Terminal, { @@ -31,6 +32,10 @@ export function getTerminalOutputEpoch(terminal: Terminal): number { return terminalOutputEpochs.get(terminal) ?? 0 } +export function isTerminalScrollRestoreInProgress(terminal: Terminal): boolean { + return (terminalScrollRestoreDepths.get(terminal) ?? 0) > 0 +} + export function cancelDeferredScrollRestore(terminal: Terminal): void { const pending = deferredScrollRestores.get(terminal) if (!pending) { @@ -170,7 +175,7 @@ function restoreScrollStateNow( // window quietly — the next visibility flip re-fits and re-restores. if (state.wasAtBottom) { const before = terminalViewportForDebug(terminal) - if (safeScrollCall(() => terminal.scrollToBottom())) { + if (safeScrollRestoreCall(terminal, () => terminal.scrollToBottom())) { if (options.syncScrollbar) { forceViewportScrollbarSync(terminal) } @@ -197,7 +202,7 @@ function restoreScrollStateNow( // (restoreScrollState, the timeout in // restoreScrollStateAfterLayout, cancelDeferredScrollRestore) own disposal. const before = terminalViewportForDebug(terminal) - if (safeScrollCall(() => terminal.scrollToLine(targetLine))) { + if (safeScrollRestoreCall(terminal, () => terminal.scrollToLine(targetLine))) { if (options.syncScrollbar) { forceViewportScrollbarSync(terminal) } @@ -229,6 +234,26 @@ function safeScrollCall(fn: () => void): boolean { } } +function safeScrollRestoreCall(terminal: Terminal, fn: () => void): boolean { + const depth = terminalScrollRestoreDepths.get(terminal) ?? 0 + terminalScrollRestoreDepths.set(terminal, depth + 1) + try { + return safeScrollCall(fn) + } finally { + // Why: xterm can notify scroll listeners just after the imperative scroll + // returns. Keep the restore marker through the current task so those + // notifications do not become remembered user scroll positions. + setTimeout(() => { + const nextDepth = (terminalScrollRestoreDepths.get(terminal) ?? 1) - 1 + if (nextDepth > 0) { + terminalScrollRestoreDepths.set(terminal, nextDepth) + } else { + terminalScrollRestoreDepths.delete(terminal) + } + }, 0) + } +} + export function releaseScrollStateMarker(state: ScrollState): void { state.firstVisibleLineMarker?.dispose() state.firstVisibleLineMarker = undefined @@ -244,10 +269,10 @@ function forceViewportScrollbarSync(terminal: Terminal): void { return } if (buf.viewportY > 0) { - safeScrollCall(() => terminal.scrollLines(-1)) - safeScrollCall(() => terminal.scrollLines(1)) + safeScrollRestoreCall(terminal, () => terminal.scrollLines(-1)) + safeScrollRestoreCall(terminal, () => terminal.scrollLines(1)) } else if (buf.viewportY < buf.baseY) { - safeScrollCall(() => terminal.scrollLines(1)) - safeScrollCall(() => terminal.scrollLines(-1)) + safeScrollRestoreCall(terminal, () => terminal.scrollLines(1)) + safeScrollRestoreCall(terminal, () => terminal.scrollLines(-1)) } }