fix(painter-dom): prevent scroll acceleration feedback loop in virtualization (#2291)

* fix(painter-dom): prevent scroll acceleration feedback loop in virtualization

The virtual window update caused uncontrollable scroll acceleration in long
documents when an external scroll container handled scrolling. The root cause
was a positive feedback loop between spacer DOM mutations and browser scroll
anchoring.

The browser picked virtualPagesEl as the scroll anchor. When the top spacer
grew during window shifts, virtualPagesEl moved down, and scroll anchoring
adjusted scrollTop to compensate. This fired a new scroll event with a higher
scrollY, triggering another window shift, cascading to the end of the document.

Fix: set overflow-anchor: none on virtualPagesEl so the browser anchors on
page elements (children) instead. Page elements stay at stable positions
regardless of spacer changes, so scroll anchoring produces zero adjustment.

Also stabilize the scrollY calculation for external scroll containers by
caching the mount-to-container offset and using scrollTop instead of
getBoundingClientRect, which is susceptible to layout shifts during DOM
mutations.

* test(behavior): add scroll acceleration regression tests

Adds 8 Playwright behavior tests covering scroll virtualization stability:

- Incremental scroll does not jump ahead uncontrollably
- Scroll to mid-document shows correct pages (not start/end)
- Rapid small scrolls do not cause runaway to bottom
- Virtual window shift preserves scroll position stability
- Scroll stability with comments enabled (different DOM layout)
- Scroll stability at 75% and 150% zoom levels

All tests pass across Chromium, Firefox, and WebKit.

* test(behavior): add comments: disabled config and scroll tests for SD-2005

Extends the behavior test harness to support comments='disabled', which sets
modules: { comments: false } on the SuperDoc config. This matches the exact
customer configuration that triggered the scroll acceleration bug (SD-2005).

Adds 3 new test variants exercising scroll stability with comments explicitly
disabled, including incremental scroll, rapid scroll, and 75% zoom.

All 33 tests pass across Chromium, Firefox, and WebKit.

* refactor: simplify scroll acceleration tests and fix review findings

- Extract shared test patterns (assertIncrementalScrollStable,
  assertRapidScrollStable, assertScrollStableAtZoom) to eliminate
  copy-paste across describe blocks
- Split getScrollInfo into separate setScrollTop (write) and
  getScrollInfo (read-only) to avoid action-query conflation
- Extract getPageHeight helper (was duplicated 7x inline)
- Replace test.extend<{}>({}) with test.use() inside describe blocks
- Use waitForStable(50) instead of page.waitForTimeout(50)
- Remove redundant instanceof HTMLElement guard in hot path
- Remove as-any cast in harness (modules is already on SuperDocConfig)

* fix: drop duplicate scrollContainerMountOffset reset (already in resetState)

Author: tupizz

packages/layout-engine/painters/dom/src/renderer.ts

@@ -1,96 +1,96 @@
 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,
   shouldRebuildForSdtBoundary,
   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;

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

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;

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