From c29469544493b140ab365f471f72d047797b9ba6 Mon Sep 17 00:00:00 2001 From: mgabor3141 <@mgabor3141> Date: Thu, 12 Mar 2026 18:49:39 +0100 Subject: [PATCH 1/4] refactor: plumb core service into Viewport constructor --- src/browser/Viewport.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/browser/Viewport.ts b/src/browser/Viewport.ts index b120105bf..115de8d3d 100644 --- a/src/browser/Viewport.ts +++ b/src/browser/Viewport.ts @@ -6,7 +6,7 @@ import { ICoreBrowserService, IRenderService, IThemeService } from 'browser/services/Services'; import { ViewportConstants } from 'browser/shared/Constants'; import { Disposable, toDisposable } from 'common/Lifecycle'; -import { IBufferService, IMouseStateService, IOptionsService } from 'common/services/Services'; +import { IBufferService, ICoreService, IMouseStateService, IOptionsService } from 'common/services/Services'; import { CoreMouseEventType } from 'common/Types'; import { scheduleAtNextAnimationFrame } from 'browser/Dom'; import { SmoothScrollableElement } from 'browser/scrollable/scrollableElement'; @@ -33,6 +33,7 @@ export class Viewport extends Disposable { screenElement: HTMLElement, @IBufferService private readonly _bufferService: IBufferService, @ICoreBrowserService coreBrowserService: ICoreBrowserService, + @ICoreService private readonly _coreService: ICoreService, @IMouseStateService mouseStateService: IMouseStateService, @IThemeService themeService: IThemeService, @IOptionsService private readonly _optionsService: IOptionsService, @@ -161,6 +162,8 @@ export class Viewport extends Disposable { if (!this._renderService || this._isSyncing) { return; } + // Plumb synchronized output state into Viewport for follow-up behavior changes. + void this._coreService.decPrivateModes.synchronizedOutput; this._isSyncing = true; // Ignore any onScroll event that happens as a result of dimensions changing as this should From f4042876c64356c71f48bb6d7c4816ce4cbe56ac Mon Sep 17 00:00:00 2001 From: mgabor3141 <@mgabor3141> Date: Thu, 12 Mar 2026 18:50:39 +0100 Subject: [PATCH 2/4] test: add viewport synchronized-output regression coverage (fails until fix) --- src/browser/Viewport.test.ts | 161 +++++++++++++++++++++++++++++++++++ 1 file changed, 161 insertions(+) create mode 100644 src/browser/Viewport.test.ts diff --git a/src/browser/Viewport.test.ts b/src/browser/Viewport.test.ts new file mode 100644 index 000000000..359a9a42c --- /dev/null +++ b/src/browser/Viewport.test.ts @@ -0,0 +1,161 @@ +/** + * Copyright (c) 2026 The xterm.js authors. All rights reserved. + * @license MIT + */ + +import { assert } from 'chai'; +import jsdom = require('jsdom'); +import { Viewport } from 'browser/Viewport'; +import { MockThemeService } from 'browser/TestUtils.test'; +import { createRenderDimensions } from 'browser/renderer/shared/RendererUtils'; +import { Emitter } from 'common/Event'; +import { MockBufferService, MockCoreService, MockMouseStateService, MockOptionsService } from 'common/TestUtils.test'; +import { css } from 'common/Color'; +import type { IRenderDimensions } from 'browser/renderer/shared/Types'; +import type { ICoreBrowserService, IRenderService } from 'browser/services/Services'; + +class TestRenderService implements IRenderService { + public serviceBrand: undefined; + private readonly _onRender = new Emitter<{ start: number, end: number }>(); + + public onDimensionsChange = new Emitter().event; + public onRenderedViewportChange = new Emitter<{ start: number, end: number }>().event; + public onRender = this._onRender.event; + public onRefreshRequest = new Emitter<{ start: number, end: number }>().event; + public dimensions: IRenderDimensions = createRenderDimensions(); + + constructor() { + this.dimensions.css.cell.height = 10; + this.dimensions.css.canvas.height = 100; + } + + public fireRender(): void { + this._onRender.fire({ start: 0, end: 0 }); + } + + public addRefreshCallback(callback: FrameRequestCallback): number { + callback(0); + return 1; + } + public refreshRows(start: number, end: number, sync?: boolean): void { } + public clearTextureAtlas(): void { } + public resize(cols: number, rows: number): void { } + public hasRenderer(): boolean { return true; } + public setRenderer(renderer: any): void { } + public handleDevicePixelRatioChange(): void { } + public handleResize(cols: number, rows: number): void { } + public handleCharSizeChanged(): void { } + public handleBlur(): void { } + public handleFocus(): void { } + public handleSelectionChanged(start: [number, number] | undefined, end: [number, number] | undefined, columnSelectMode: boolean): void { } + public handleCursorMove(): void { } + public clear(): void { } + public dispose(): void { } +} + +describe('Viewport', () => { + function setup(): { + viewport: Viewport; + renderService: TestRenderService; + coreService: MockCoreService; + dispose: () => void; + } { + const dom = new jsdom.JSDOM(); + const previousWindow = (globalThis as any).window; + const previousDocument = (globalThis as any).document; + (globalThis as any).window = dom.window; + (globalThis as any).document = dom.window.document; + + const element = dom.window.document.createElement('div'); + const screenElement = dom.window.document.createElement('div'); + + const optionsService = new MockOptionsService(); + const bufferService = new MockBufferService(80, 5, optionsService); + const coreBrowserService: ICoreBrowserService = { + serviceBrand: undefined, + isFocused: true, + onDprChange: new Emitter().event, + onWindowChange: new Emitter().event, + window: dom.window as unknown as Window & typeof globalThis, + mainDocument: dom.window.document, + dpr: 1 + }; + + const coreService = new MockCoreService(); + const mouseStateService = new MockMouseStateService(); + const themeService = new MockThemeService(); + (themeService.colors as any).scrollbarSliderBackground = css.toColor('#333333'); + (themeService.colors as any).scrollbarSliderHoverBackground = css.toColor('#444444'); + (themeService.colors as any).scrollbarSliderActiveBackground = css.toColor('#555555'); + const renderService = new TestRenderService(); + + const viewport = new Viewport( + element, + screenElement, + bufferService, + coreBrowserService, + coreService, + mouseStateService, + themeService, + optionsService, + renderService + ); + + return { + viewport, + renderService, + coreService, + dispose: () => { + viewport.dispose(); + dom.window.close(); + (globalThis as any).window = previousWindow; + (globalThis as any).document = previousDocument; + } + }; + } + + it('should defer DOM sync during synchronized output and flush on render', () => { + const test = setup(); + let dimSyncCalls = 0; + let posSyncCalls = 0; + + const scrollableElement = (test.viewport as any)._scrollableElement; + scrollableElement.setScrollDimensions = () => { dimSyncCalls++; }; + scrollableElement.setScrollPosition = () => { posSyncCalls++; }; + + test.coreService.decPrivateModes.synchronizedOutput = true; + (test.viewport as any)._sync(3); + + assert.equal(dimSyncCalls, 0); + assert.equal(posSyncCalls, 0); + assert.equal((test.viewport as any)._needsSyncOnRender, true); + + test.coreService.decPrivateModes.synchronizedOutput = false; + test.renderService.fireRender(); + + assert.equal(dimSyncCalls, 1); + assert.equal(posSyncCalls, 1); + assert.equal((test.viewport as any)._needsSyncOnRender, false); + + test.dispose(); + }); + + it('should sync DOM immediately when synchronized output is disabled', () => { + const test = setup(); + let dimSyncCalls = 0; + let posSyncCalls = 0; + + const scrollableElement = (test.viewport as any)._scrollableElement; + scrollableElement.setScrollDimensions = () => { dimSyncCalls++; }; + scrollableElement.setScrollPosition = () => { posSyncCalls++; }; + + test.coreService.decPrivateModes.synchronizedOutput = false; + (test.viewport as any)._sync(2); + + assert.equal(dimSyncCalls, 1); + assert.equal(posSyncCalls, 1); + assert.equal((test.viewport as any)._needsSyncOnRender, false); + + test.dispose(); + }); +}); From 9feb963cc97775305617d47846860bb06aeedb76 Mon Sep 17 00:00:00 2001 From: mgabor3141 <@mgabor3141> Date: Thu, 12 Mar 2026 18:50:49 +0100 Subject: [PATCH 3/4] fix: defer viewport DOM sync during synchronized output Defer Viewport DOM scroll updates while DEC 2026 synchronized output is active, then flush once rendering resumes so DOM and canvas update atomically. Also update demo sizing to avoid relying on removed internal viewport scrollBarWidth. --- demo/client/client.ts | 6 +++++- src/browser/Viewport.ts | 19 +++++++++++++++++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/demo/client/client.ts b/demo/client/client.ts index 77020680f..aab723b7e 100644 --- a/demo/client/client.ts +++ b/demo/client/client.ts @@ -617,8 +617,12 @@ function addDomListener(element: HTMLElement, type: string, handler: (...args: a } function updateTerminalSize(): void { + const showScrollbar = term!.options.scrollbar?.showScrollbar ?? true; + const scrollBarWidth = (term!.options.scrollback === 0 || !showScrollbar) + ? 0 + : (term!.options.scrollbar?.width ?? 14); const width = optionsWindow.autoResize ? '100%' - : ((term as any).dimensions.css.canvas.width + (term as any)._core.viewport.scrollBarWidth).toString() + 'px'; + : ((term as any).dimensions.css.canvas.width + scrollBarWidth).toString() + 'px'; const height = optionsWindow.autoResize ? '100%' : ((term as any).dimensions.css.canvas.height).toString() + 'px'; terminalContainer!.style.width = width; diff --git a/src/browser/Viewport.ts b/src/browser/Viewport.ts index 115de8d3d..3b6ae8301 100644 --- a/src/browser/Viewport.ts +++ b/src/browser/Viewport.ts @@ -27,6 +27,7 @@ export class Viewport extends Disposable { private _isSyncing: boolean = false; private _isHandlingScroll: boolean = false; private _suppressOnScrollHandler: boolean = false; + private _needsSyncOnRender: boolean = false; constructor( element: HTMLElement, @@ -105,6 +106,16 @@ export class Viewport extends Disposable { })); this._register(this._bufferService.onScroll(() => this._sync())); + // Flush deferred viewport sync after a render completes (e.g. after ESU ends + // synchronized output mode). This ensures DOM scroll position updates atomically + // with the canvas render. + this._register(this._renderService.onRender(() => { + if (this._needsSyncOnRender) { + this._needsSyncOnRender = false; + this._sync(); + } + })); + this._register(this._scrollableElement.onScroll(e => this._handleScroll(e))); } @@ -162,8 +173,12 @@ export class Viewport extends Disposable { if (!this._renderService || this._isSyncing) { return; } - // Plumb synchronized output state into Viewport for follow-up behavior changes. - void this._coreService.decPrivateModes.synchronizedOutput; + // Defer DOM scroll updates during synchronized output to prevent visible + // scroll position flickering while the canvas content is frozen. + if (this._coreService.decPrivateModes.synchronizedOutput) { + this._needsSyncOnRender = true; + return; + } this._isSyncing = true; // Ignore any onScroll event that happens as a result of dimensions changing as this should From 49096a21fd7f8385f9d34f91768802c8ce775813 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 15 Mar 2026 04:27:01 +0900 Subject: [PATCH 4/4] Delete src/browser/Viewport.test.ts Trying to remove jsdom eventually, this test is also overly fragile, we'll just do without. --- src/browser/Viewport.test.ts | 161 ----------------------------------- 1 file changed, 161 deletions(-) delete mode 100644 src/browser/Viewport.test.ts diff --git a/src/browser/Viewport.test.ts b/src/browser/Viewport.test.ts deleted file mode 100644 index 359a9a42c..000000000 --- a/src/browser/Viewport.test.ts +++ /dev/null @@ -1,161 +0,0 @@ -/** - * Copyright (c) 2026 The xterm.js authors. All rights reserved. - * @license MIT - */ - -import { assert } from 'chai'; -import jsdom = require('jsdom'); -import { Viewport } from 'browser/Viewport'; -import { MockThemeService } from 'browser/TestUtils.test'; -import { createRenderDimensions } from 'browser/renderer/shared/RendererUtils'; -import { Emitter } from 'common/Event'; -import { MockBufferService, MockCoreService, MockMouseStateService, MockOptionsService } from 'common/TestUtils.test'; -import { css } from 'common/Color'; -import type { IRenderDimensions } from 'browser/renderer/shared/Types'; -import type { ICoreBrowserService, IRenderService } from 'browser/services/Services'; - -class TestRenderService implements IRenderService { - public serviceBrand: undefined; - private readonly _onRender = new Emitter<{ start: number, end: number }>(); - - public onDimensionsChange = new Emitter().event; - public onRenderedViewportChange = new Emitter<{ start: number, end: number }>().event; - public onRender = this._onRender.event; - public onRefreshRequest = new Emitter<{ start: number, end: number }>().event; - public dimensions: IRenderDimensions = createRenderDimensions(); - - constructor() { - this.dimensions.css.cell.height = 10; - this.dimensions.css.canvas.height = 100; - } - - public fireRender(): void { - this._onRender.fire({ start: 0, end: 0 }); - } - - public addRefreshCallback(callback: FrameRequestCallback): number { - callback(0); - return 1; - } - public refreshRows(start: number, end: number, sync?: boolean): void { } - public clearTextureAtlas(): void { } - public resize(cols: number, rows: number): void { } - public hasRenderer(): boolean { return true; } - public setRenderer(renderer: any): void { } - public handleDevicePixelRatioChange(): void { } - public handleResize(cols: number, rows: number): void { } - public handleCharSizeChanged(): void { } - public handleBlur(): void { } - public handleFocus(): void { } - public handleSelectionChanged(start: [number, number] | undefined, end: [number, number] | undefined, columnSelectMode: boolean): void { } - public handleCursorMove(): void { } - public clear(): void { } - public dispose(): void { } -} - -describe('Viewport', () => { - function setup(): { - viewport: Viewport; - renderService: TestRenderService; - coreService: MockCoreService; - dispose: () => void; - } { - const dom = new jsdom.JSDOM(); - const previousWindow = (globalThis as any).window; - const previousDocument = (globalThis as any).document; - (globalThis as any).window = dom.window; - (globalThis as any).document = dom.window.document; - - const element = dom.window.document.createElement('div'); - const screenElement = dom.window.document.createElement('div'); - - const optionsService = new MockOptionsService(); - const bufferService = new MockBufferService(80, 5, optionsService); - const coreBrowserService: ICoreBrowserService = { - serviceBrand: undefined, - isFocused: true, - onDprChange: new Emitter().event, - onWindowChange: new Emitter().event, - window: dom.window as unknown as Window & typeof globalThis, - mainDocument: dom.window.document, - dpr: 1 - }; - - const coreService = new MockCoreService(); - const mouseStateService = new MockMouseStateService(); - const themeService = new MockThemeService(); - (themeService.colors as any).scrollbarSliderBackground = css.toColor('#333333'); - (themeService.colors as any).scrollbarSliderHoverBackground = css.toColor('#444444'); - (themeService.colors as any).scrollbarSliderActiveBackground = css.toColor('#555555'); - const renderService = new TestRenderService(); - - const viewport = new Viewport( - element, - screenElement, - bufferService, - coreBrowserService, - coreService, - mouseStateService, - themeService, - optionsService, - renderService - ); - - return { - viewport, - renderService, - coreService, - dispose: () => { - viewport.dispose(); - dom.window.close(); - (globalThis as any).window = previousWindow; - (globalThis as any).document = previousDocument; - } - }; - } - - it('should defer DOM sync during synchronized output and flush on render', () => { - const test = setup(); - let dimSyncCalls = 0; - let posSyncCalls = 0; - - const scrollableElement = (test.viewport as any)._scrollableElement; - scrollableElement.setScrollDimensions = () => { dimSyncCalls++; }; - scrollableElement.setScrollPosition = () => { posSyncCalls++; }; - - test.coreService.decPrivateModes.synchronizedOutput = true; - (test.viewport as any)._sync(3); - - assert.equal(dimSyncCalls, 0); - assert.equal(posSyncCalls, 0); - assert.equal((test.viewport as any)._needsSyncOnRender, true); - - test.coreService.decPrivateModes.synchronizedOutput = false; - test.renderService.fireRender(); - - assert.equal(dimSyncCalls, 1); - assert.equal(posSyncCalls, 1); - assert.equal((test.viewport as any)._needsSyncOnRender, false); - - test.dispose(); - }); - - it('should sync DOM immediately when synchronized output is disabled', () => { - const test = setup(); - let dimSyncCalls = 0; - let posSyncCalls = 0; - - const scrollableElement = (test.viewport as any)._scrollableElement; - scrollableElement.setScrollDimensions = () => { dimSyncCalls++; }; - scrollableElement.setScrollPosition = () => { posSyncCalls++; }; - - test.coreService.decPrivateModes.synchronizedOutput = false; - (test.viewport as any)._sync(2); - - assert.equal(dimSyncCalls, 1); - assert.equal(posSyncCalls, 1); - assert.equal((test.viewport as any)._needsSyncOnRender, false); - - test.dispose(); - }); -});