diff --git a/packages/layout-engine/painters/dom/src/renderer.ts b/packages/layout-engine/painters/dom/src/renderer.ts index 4a6fe8c81e..0bfafda773 100644 --- a/packages/layout-engine/painters/dom/src/renderer.ts +++ b/packages/layout-engine/painters/dom/src/renderer.ts @@ -1,76 +1,89 @@ import type { + CustomGeometryData, + DrawingBlock, + DrawingFragment, + DrawingGeometry, + DropCapDescriptor, + FieldAnnotationRun, FlowBlock, + FlowMode, + FlowRunLink, Fragment, + GradientFill, + ImageBlock, + ImageDrawing, + ImageFragment, + ImageRun, Layout, + Line, + LineSegment, + ListBlock, + ListItemFragment, + ListMeasure, Measure, Page, PageMargins, ParaFragment, - ImageFragment, - DrawingFragment, - Run, - TextRun, - ImageRun, - FieldAnnotationRun, - Line, - LineSegment, - ParagraphBlock, - ParagraphMeasure, - ImageBlock, - ImageDrawing, ParagraphAttrs, + ParagraphBlock, ParagraphBorder, - ListItemFragment, - ListBlock, - ListMeasure, + ParagraphMeasure, + PositionedDrawingGeometry, + PositionMapping, + Run, + SdtMetadata, + ShapeGroupChild, + ShapeGroupDrawing, + ShapeTextContent, + SolidFillWithAlpha, + TableAttrs, TableBlock, + TableCellAttrs, TableFragment, + TextRun, TrackedChangeKind, TrackedChangesMode, - SdtMetadata, - DrawingBlock, VectorShapeDrawing, - ShapeGroupDrawing, - ShapeGroupChild, - DrawingGeometry, - PositionedDrawingGeometry, VectorShapeStyle, - FlowRunLink, - GradientFill, - SolidFillWithAlpha, - ShapeTextContent, - DropCapDescriptor, - TableAttrs, - TableCellAttrs, - PositionMapping, - FlowMode, - CustomGeometryData, } from '@superdoc/contracts'; import { calculateJustifySpacing, computeLinePmRange, shouldApplyJustify, SPACE_CHARS } from '@superdoc/contracts'; +import { toCssFontFamily } from '@superdoc/font-utils'; import { getPresetShapeSvg } from '@superdoc/preset-geometry'; -import { applyGradientToSVG, applyAlphaToSVG, validateHexColor } from './svg-utils.js'; +import { encodeTooltip, sanitizeHref } from '@superdoc/url-validation'; +import { DOM_CLASS_NAMES } from './constants.js'; +import { + getRunBooleanProp, + getRunNumberProp, + getRunStringProp, + getRunUnderlineColor, + getRunUnderlineStyle, + hashCellBorders, + hashParagraphBorders, + hashTableBorders, +} from './paragraph-hash-utils.js'; +import { assertFragmentPmPositions, assertPmPositions } from './pm-position-validation.js'; +import { createRulerElement, ensureRulerStyles, generateRulerDefinitionFromPx } from './ruler/index.js'; import { CLASS_NAMES, containerStyles, containerStylesHorizontal, - spreadStyles, - fragmentStyles, - lineStyles, - pageStyles, - ensurePrintStyles, - ensureLinkStyles, - ensureTrackChangeStyles, - ensureSdtContainerStyles, ensureFieldAnnotationStyles, ensureImageSelectionStyles, + ensureLinkStyles, ensureNativeSelectionStyles, + ensurePrintStyles, + ensureSdtContainerStyles, + ensureTrackChangeStyles, + fragmentStyles, + lineStyles, + pageStyles, + spreadStyles, type PageStyles, } from './styles.js'; -import { DOM_CLASS_NAMES } from './constants.js'; -import { sanitizeHref, encodeTooltip } from '@superdoc/url-validation'; +import { applyAlphaToSVG, applyGradientToSVG, validateHexColor } from './svg-utils.js'; import { renderTableFragment as renderTableFragmentElement } from './table/renderTableFragment.js'; -import { assertPmPositions, assertFragmentPmPositions } from './pm-position-validation.js'; import { applyImageClipPath } from './utils/image-clip-path.js'; +import { computeTabWidth } from './utils/marker-helpers.js'; import { applySdtContainerStyling, getSdtContainerKey, @@ -78,19 +91,6 @@ import { type SdtBoundaryOptions, } from './utils/sdt-helpers.js'; import { SdtGroupedHover } from './utils/sdt-hover.js'; -import { computeTabWidth } from './utils/marker-helpers.js'; -import { generateRulerDefinitionFromPx, createRulerElement, ensureRulerStyles } from './ruler/index.js'; -import { toCssFontFamily } from '@superdoc/font-utils'; -import { - hashParagraphBorders, - hashTableBorders, - hashCellBorders, - getRunStringProp, - getRunNumberProp, - getRunBooleanProp, - getRunUnderlineStyle, - getRunUnderlineColor, -} from './paragraph-hash-utils.js'; /** * Minimal type for WordParagraphLayoutOutput marker data used in rendering. @@ -1041,6 +1041,12 @@ export class DomPainter { * wrapper div that owns scrolling rather than the window. */ private scrollContainer: HTMLElement | null = null; + /** + * Cached offset (in px) from the scroll container's content top to the mount's top. + * Used for stable scrollY calculation that avoids feedback loops from spacer DOM mutations. + * Invalidated when the mount, scroll container, or zoom changes. + */ + private scrollContainerMountOffset: number | null = null; private sdtHover = new SdtGroupedHover(); /** The currently active/selected comment ID for highlighting */ private activeCommentId: string | null = null; @@ -1114,6 +1120,7 @@ export class DomPainter { const next = typeof zoom === 'number' && Number.isFinite(zoom) && zoom > 0 ? zoom : 1; if (next !== this.zoomFactor) { this.zoomFactor = next; + this.scrollContainerMountOffset = null; // Invalidate on zoom change if (this.virtualEnabled && this.mount) { this.updateVirtualWindow(); } @@ -1137,6 +1144,7 @@ export class DomPainter { public setScrollContainer(el: HTMLElement | null): void { if (el !== this.scrollContainer) { this.scrollContainer = el; + this.scrollContainerMountOffset = null; // Invalidate cached offset if (this.virtualEnabled && this.mount) { this.updateVirtualWindow(); } @@ -1576,6 +1584,14 @@ export class DomPainter { this.virtualPagesEl.style.alignItems = 'center'; this.virtualPagesEl.style.width = '100%'; this.virtualPagesEl.style.gap = `${this.virtualGap}px`; + // Prevent the browser from using this container as a scroll anchor. + // When the top spacer grows during virtual window shifts, this element + // moves down. If the browser anchors on it, scroll anchoring adjusts + // scrollTop to compensate, which fires a new scroll event with a higher + // scrollY, triggering another window shift — a positive feedback loop. + // With this set, the browser anchors on page elements (children) instead, + // which stay at stable positions regardless of spacer changes. + this.virtualPagesEl.style.overflowAnchor = 'none'; mount.appendChild(this.topSpacerEl); mount.appendChild(this.virtualPagesEl); @@ -1589,6 +1605,10 @@ export class DomPainter { element.style.width = '1px'; element.style.height = '0px'; element.style.flex = '0 0 auto'; + // Prevent Chrome's scroll anchoring from using spacers as anchor nodes. + // When spacer heights change during virtual window shifts, scroll anchoring + // could adjust scrollTop and trigger cascading scroll events. + element.style.overflowAnchor = 'none'; element.setAttribute('data-virtual-spacer', type); } @@ -1618,6 +1638,7 @@ export class DomPainter { win.removeEventListener('resize', this.onResizeHandler); } this.onResizeHandler = () => { + this.scrollContainerMountOffset = null; // Recompute on resize this.updateVirtualWindow(); }; win.addEventListener('resize', this.onResizeHandler); @@ -1682,6 +1703,7 @@ export class DomPainter { if (!this.mount || !this.topSpacerEl || !this.bottomSpacerEl || !this.virtualPagesEl || !this.currentLayout) return; const layout = this.currentLayout; const N = layout.pages.length; + if (N === 0) { this.mount.innerHTML = ''; this.processedLayoutVersion = this.layoutVersion; @@ -1700,13 +1722,18 @@ export class DomPainter { scrollY = Math.max(0, this.mount.scrollTop - paddingTop); } else if (this.scrollContainer) { // Intermediate scroll ancestor (e.g., a wrapper div with overflow-y: auto). - // Compute scrollY relative to the scroll container, not the browser viewport. - // Using (scrollContainer.rect.top - mount.rect.top) instead of just (-mount.rect.top) - // eliminates the offset caused by the scroll container's distance from the viewport top - // (e.g., a toolbar/header above the scroll wrapper). - const mountRect = this.mount.getBoundingClientRect(); - const containerRect = this.scrollContainer.getBoundingClientRect(); - scrollY = Math.max(0, (containerRect.top - mountRect.top) / zoom - paddingTop); + // Use scrollContainer.scrollTop with a cached mount offset instead of + // getBoundingClientRect(). Rects are affected by spacer DOM mutations + // which can cause cascading scroll events and runaway scrolling. + // + // mountOffset = distance from scroll container's content top to mount's top. + // Computed once and cached; invalidated on mount/container/zoom change. + if (this.scrollContainerMountOffset == null) { + const mountRect = this.mount.getBoundingClientRect(); + const containerRect = this.scrollContainer.getBoundingClientRect(); + this.scrollContainerMountOffset = mountRect.top - containerRect.top + this.scrollContainer.scrollTop; + } + scrollY = Math.max(0, (this.scrollContainer.scrollTop - this.scrollContainerMountOffset) / zoom - paddingTop); } else { const rect = this.mount.getBoundingClientRect(); // rect.top is in screen space (affected by CSS transform: scale). @@ -2210,6 +2237,7 @@ export class DomPainter { this.onScrollHandler = null; this.onWindowScrollHandler = null; this.onResizeHandler = null; + this.scrollContainerMountOffset = null; this.sdtHover.destroy(); this.layoutVersion = 0; this.processedLayoutVersion = -1; diff --git a/packages/layout-engine/painters/dom/src/virtualization.test.ts b/packages/layout-engine/painters/dom/src/virtualization.test.ts index 1fa4388b85..f126ed22a9 100644 --- a/packages/layout-engine/painters/dom/src/virtualization.test.ts +++ b/packages/layout-engine/painters/dom/src/virtualization.test.ts @@ -487,19 +487,19 @@ describe('DomPainter virtualization (vertical)', () => { const pagesWithout = mount.querySelectorAll('.superdoc-page'); const indicesWithout = Array.from(pagesWithout).map((p) => Number((p as HTMLElement).dataset.pageIndex)); - // WITH scroll container: scrollY = (100 - (-4900)) / 0.75 = 6666 - // That's different from the without case — but the key point is: - // the offset from the viewport (toolbarHeight) is eliminated from the calculation. - // scrollY = (containerRect.top - mountRect.top) / zoom = (100 - (-4900)) / 0.75 = 6666 + // WITH scroll container: uses scrollTop-based calculation. + // offset = mountRect.top - containerRect.top + scrollTop = -4900 - 100 + 5000 = 0 + // scrollY = (5000 - 0) / 0.75 = 6666 + Object.defineProperty(scrollWrapper, 'scrollTop', { value: scrollTop, writable: true, configurable: true }); painter.setScrollContainer!(scrollWrapper); painter.onScroll!(); const pagesWith = mount.querySelectorAll('.superdoc-page'); const indicesWith = Array.from(pagesWith).map((p) => Number((p as HTMLElement).dataset.pageIndex)); - // Now simulate at scroll=0 to see the offset difference clearly. - // Without container: mount.rect.top = 100, scrollY = max(0, -100/0.75) = 0 ← correct by luck (clamped) - // But at small scroll (e.g., scroll=50): mount.rect.top = 50, scrollY = -50/0.75 = -66 → 0 ← WRONG, should be ~66 - // With container: scrollY = (100 - 50) / 0.75 = 66 ← CORRECT + // Now simulate small scroll (scrollTop=150) to see the offset difference clearly. + // The cached offset (0) stays valid. + // scrollY = (150 - 0) / 0.75 = 200 in layout space + // Anchor at page 0 (topOfIndex(0)=0, topOfIndex(1)=572), so pages [0,1,2] const smallScroll = 150; const mountTopSmall = toolbarHeight - smallScroll; // 100 - 150 = -50 @@ -516,11 +516,11 @@ describe('DomPainter virtualization (vertical)', () => { toJSON() {}, }) as DOMRect; + // Set scrollTop to match the simulated scroll position + Object.defineProperty(scrollWrapper, 'scrollTop', { value: smallScroll, writable: true, configurable: true }); painter.onScroll!(); const pagesSmallScroll = mount.querySelectorAll('.superdoc-page'); const indicesSmallScroll = Array.from(pagesSmallScroll).map((p) => Number((p as HTMLElement).dataset.pageIndex)); - // scrollY = (100 - (-50)) / 0.75 = 200 in layout space - // Anchor at page 0 (topOfIndex(0)=0, topOfIndex(1)=572), so pages [0,1,2] expect(indicesSmallScroll).toEqual([0, 1, 2]); // Verify: remove scroll container and the same scroll position gives different result diff --git a/tests/behavior/fixtures/superdoc.ts b/tests/behavior/fixtures/superdoc.ts index 7b525d396a..435cf0b133 100644 --- a/tests/behavior/fixtures/superdoc.ts +++ b/tests/behavior/fixtures/superdoc.ts @@ -9,7 +9,7 @@ const HARNESS_URL = 'http://localhost:9990'; interface HarnessConfig { layout?: boolean; toolbar?: 'none' | 'full'; - comments?: 'off' | 'on' | 'panel' | 'readonly'; + comments?: 'off' | 'on' | 'panel' | 'readonly' | 'disabled'; trackChanges?: boolean; showCaret?: boolean; showSelection?: boolean; diff --git a/tests/behavior/harness/main.ts b/tests/behavior/harness/main.ts index 3a0b7d289c..6b08647337 100644 --- a/tests/behavior/harness/main.ts +++ b/tests/behavior/harness/main.ts @@ -95,6 +95,10 @@ function init(file?: File, content?: ContentOverrideInput) { config.comments = { visible: true }; } else if (comments === 'readonly') { config.comments = { visible: true, readOnly: true }; + } else if (comments === 'disabled') { + // Explicitly disable the comments module (modules: { comments: false }). + // This matches the customer config pattern that triggers different scroll behavior. + config.modules = { ...(config.modules ?? {}), comments: false }; } // Track changes diff --git a/tests/behavior/tests/virtualization/scroll-acceleration.spec.ts b/tests/behavior/tests/virtualization/scroll-acceleration.spec.ts new file mode 100644 index 0000000000..f3f23ee5cc --- /dev/null +++ b/tests/behavior/tests/virtualization/scroll-acceleration.spec.ts @@ -0,0 +1,252 @@ +import { test, expect, type SuperDocFixture } from '../../fixtures/superdoc.js'; + +test.use({ config: { toolbar: 'none' } }); + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +async function generateLongDocument(page: any, paragraphCount = 200): Promise { + await page.evaluate((count: number) => { + const editor = (window as any).editor; + const { state } = editor; + const { schema } = state; + + const paragraphs: any[] = []; + for (let i = 0; i < count; i++) { + const text = schema.text( + `Paragraph ${i + 1}. ` + + 'Lorem ipsum dolor sit amet, consectetur adipiscing elit. ' + + 'Sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. ' + + 'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris.', + ); + const run = schema.nodes.run.create(null, text); + paragraphs.push(schema.nodes.paragraph.create(null, run)); + } + + const doc = schema.nodes.doc.create(null, paragraphs); + const tr = state.tr.replaceWith(0, state.doc.content.size, doc.content); + editor.view.dispatch(tr); + }, paragraphCount); +} + +/** Read scroll container metrics without mutating. */ +async function getScrollInfo(page: any): Promise<{ + scrollTop: number; + scrollHeight: number; + clientHeight: number; +}> { + return page.evaluate(() => { + const mount = document.querySelector('.superdoc-viewport') ?? document.querySelector('#editor'); + let scrollable: HTMLElement | null = null; + let el: HTMLElement | null = mount as HTMLElement; + while (el && el !== document.documentElement) { + if (el.scrollHeight > el.clientHeight + 10) { + scrollable = el; + break; + } + el = el.parentElement; + } + if (!scrollable) scrollable = document.documentElement; + return { + scrollTop: scrollable.scrollTop, + scrollHeight: scrollable.scrollHeight, + clientHeight: scrollable.clientHeight, + }; + }); +} + +/** Set scrollTop on the scroll container. */ +async function setScrollTop(page: any, value: number): Promise { + await page.evaluate((v: number) => { + const mount = document.querySelector('.superdoc-viewport') ?? document.querySelector('#editor'); + let scrollable: HTMLElement | null = null; + let el: HTMLElement | null = mount as HTMLElement; + while (el && el !== document.documentElement) { + if (el.scrollHeight > el.clientHeight + 10) { + scrollable = el; + break; + } + el = el.parentElement; + } + if (!scrollable) scrollable = document.documentElement; + scrollable.scrollTop = v; + }, value); +} + +async function getPageHeight(page: any): Promise { + return page.evaluate(() => { + const p = document.querySelector('.superdoc-page[data-page-index]') as HTMLElement; + return p ? p.offsetHeight : 1000; + }); +} + +async function getMountedPageIndices(page: any): Promise { + return page.evaluate(() => { + const pages = document.querySelectorAll('.superdoc-page[data-page-index]'); + return Array.from(pages) + .map((p) => Number((p as HTMLElement).dataset.pageIndex)) + .sort((a, b) => a - b); + }); +} + +// --------------------------------------------------------------------------- +// Shared test patterns (called from multiple describe blocks) +// --------------------------------------------------------------------------- + +/** Scroll incrementally and assert scroll position doesn't run away. */ +async function assertIncrementalScrollStable(superdoc: SuperDocFixture, steps = 12): Promise { + const pageHeight = await getPageHeight(superdoc.page); + const scrollStep = Math.floor(pageHeight * 0.8); + let targetScroll = 0; + + for (let step = 0; step < steps; step++) { + targetScroll += scrollStep; + await setScrollTop(superdoc.page, targetScroll); + await superdoc.waitForStable(300); + + const info = await getScrollInfo(superdoc.page); + const drift = Math.abs(info.scrollTop - targetScroll); + expect(drift).toBeLessThan(pageHeight * 3); + } + + const mounted = await getMountedPageIndices(superdoc.page); + expect(mounted.length).toBeGreaterThan(0); +} + +/** Fire many rapid small scrolls and assert position didn't rocket to the bottom. */ +async function assertRapidScrollStable(superdoc: SuperDocFixture, steps = 25): Promise { + const pageHeight = await getPageHeight(superdoc.page); + const smallStep = Math.floor(pageHeight / 5); + let targetScroll = 0; + + for (let i = 0; i < steps; i++) { + targetScroll += smallStep; + await setScrollTop(superdoc.page, targetScroll); + await superdoc.waitForStable(50); + } + + await superdoc.waitForStable(500); + const info = await getScrollInfo(superdoc.page); + + const bottomThreshold = info.scrollHeight - info.clientHeight - pageHeight; + expect(info.scrollTop).toBeLessThan(bottomThreshold); +} + +/** Set zoom, scroll incrementally, and assert stability. */ +async function assertScrollStableAtZoom(superdoc: SuperDocFixture, zoom: number): Promise { + await superdoc.page.evaluate((z: number) => { + (window as any).superdoc.setZoom(z); + }, zoom); + await superdoc.waitForStable(1000); + + await assertIncrementalScrollStable(superdoc); +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +test.describe('scroll virtualization stability', () => { + test('incremental scroll does not jump ahead', async ({ superdoc }) => { + await generateLongDocument(superdoc.page); + await superdoc.waitForStable(2000); + + const info = await getScrollInfo(superdoc.page); + expect(info.scrollHeight).toBeGreaterThan(info.clientHeight * 5); + + await assertIncrementalScrollStable(superdoc, 15); + }); + + test('scroll to middle of document shows mid-range pages', async ({ superdoc }) => { + await generateLongDocument(superdoc.page); + await superdoc.waitForStable(2000); + + const info = await getScrollInfo(superdoc.page); + await setScrollTop(superdoc.page, Math.floor(info.scrollHeight / 2)); + await superdoc.waitForStable(500); + + const mounted = await getMountedPageIndices(superdoc.page); + expect(mounted.length).toBeGreaterThan(0); + expect(Math.min(...mounted)).toBeGreaterThan(0); + }); + + test('rapid small scrolls do not cause runaway', async ({ superdoc }) => { + await generateLongDocument(superdoc.page); + await superdoc.waitForStable(2000); + + await assertRapidScrollStable(superdoc, 30); + }); + + test('virtual window shift preserves scroll stability', async ({ superdoc }) => { + await generateLongDocument(superdoc.page); + await superdoc.waitForStable(2000); + + const pageHeight = await getPageHeight(superdoc.page); + + // Scroll past the initial virtual window (~5 pages) to trigger a shift. + await setScrollTop(superdoc.page, pageHeight * 6); + await superdoc.waitForStable(500); + + const afterShift = await getScrollInfo(superdoc.page); + + // Wait for any cascading scroll events to settle. + await superdoc.waitForStable(300); + const afterSettle = await getScrollInfo(superdoc.page); + + const drift = Math.abs(afterSettle.scrollTop - afterShift.scrollTop); + expect(drift).toBeLessThan(pageHeight); + }); +}); + +test.describe('scroll stability with comments enabled', () => { + test.use({ config: { toolbar: 'none', comments: 'on' } }); + + test('incremental scroll stable with comments on', async ({ superdoc }) => { + await generateLongDocument(superdoc.page); + await superdoc.waitForStable(2000); + await assertIncrementalScrollStable(superdoc); + }); + + test('rapid scroll stable with comments on', async ({ superdoc }) => { + await generateLongDocument(superdoc.page); + await superdoc.waitForStable(2000); + await assertRapidScrollStable(superdoc); + }); +}); + +test.describe('scroll stability with comments disabled (modules.comments: false)', () => { + test.use({ config: { toolbar: 'none', comments: 'disabled' } }); + + test('incremental scroll stable with comments: false', async ({ superdoc }) => { + await generateLongDocument(superdoc.page); + await superdoc.waitForStable(2000); + await assertIncrementalScrollStable(superdoc); + }); + + test('rapid scroll stable with comments: false', async ({ superdoc }) => { + await generateLongDocument(superdoc.page); + await superdoc.waitForStable(2000); + await assertRapidScrollStable(superdoc); + }); + + test('scroll at 75% zoom with comments: false', async ({ superdoc }) => { + await generateLongDocument(superdoc.page); + await superdoc.waitForStable(2000); + await assertScrollStableAtZoom(superdoc, 75); + }); +}); + +test.describe('scroll stability at non-100% zoom', () => { + test('scroll does not accelerate at 75% zoom', async ({ superdoc }) => { + await generateLongDocument(superdoc.page); + await superdoc.waitForStable(2000); + await assertScrollStableAtZoom(superdoc, 75); + }); + + test('scroll does not accelerate at 150% zoom', async ({ superdoc }) => { + await generateLongDocument(superdoc.page); + await superdoc.waitForStable(2000); + await assertScrollStableAtZoom(superdoc, 150); + }); +});