fix(painter-dom): skip non-scrollable scroll container in virtualization (SD-2199) (#2383)

Only first 7 pages rendered on documents with 8+ pages when SuperDoc was
mounted inside an unconstrained flex layout. The scroll container detection
found an ancestor with overflow:auto CSS but that element was never actually
scrollable because its parent only had min-height (no height constraint).
The element grew to fit content instead of constraining it, so scrollTop
stayed at 0 and the virtual window never advanced beyond pages 0-6.

The fix adds a runtime check in updateVirtualWindow() that verifies the
scroll container is actually scrollable (scrollHeight > clientHeight)
before using its scrollTop. When the container is not scrollable, it falls
through to the viewport-based getBoundingClientRect calculation which works
correctly with window-level scrolling.

Author: tupizz

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

@@ -1799,9 +1799,16 @@ export class DomPainter {
     const zoom = this.zoomFactor;
     let scrollY: number;
     const isContainerScrollable = this.mount.scrollHeight > this.mount.clientHeight + 1;
+    // Check if the external scroll container is actually scrollable (content overflows its
+    // visible area). An element can have overflow:auto but still not scroll if it's in an
+    // unconstrained flex layout where the parent has only min-height (no height). In that
+    // case the element grows to fit content and scrollTop stays 0 — fall through to the
+    // viewport-based calculation instead.
+    const scrollCont = this.scrollContainer;
+    const isScrollContainerActive = scrollCont != null && scrollCont.scrollHeight > scrollCont.clientHeight + 1;
     if (isContainerScrollable) {
       scrollY = Math.max(0, this.mount.scrollTop - paddingTop);
-    } else if (this.scrollContainer) {
+    } else if (isScrollContainerActive) {
       // Intermediate scroll ancestor (e.g., a wrapper div with overflow-y: auto).
       // Use scrollContainer.scrollTop with a cached mount offset instead of
       // getBoundingClientRect(). Rects are affected by spacer DOM mutations
@@ -1811,10 +1818,10 @@ export class DomPainter {
       // 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;
+        const containerRect = scrollCont.getBoundingClientRect();
+        this.scrollContainerMountOffset = mountRect.top - containerRect.top + scrollCont.scrollTop;
       }
-      scrollY = Math.max(0, (this.scrollContainer.scrollTop - this.scrollContainerMountOffset) / zoom - paddingTop);
+      scrollY = Math.max(0, (scrollCont.scrollTop - this.scrollContainerMountOffset) / zoom - paddingTop);
     } else {
       const rect = this.mount.getBoundingClientRect();
       // rect.top is in screen space (affected by CSS transform: scale).

packages/layout-engine/painters/dom/src/virtualization.test.ts

@@ -686,4 +686,65 @@ describe('DomPainter virtualization (vertical)', () => {
     expect(mount.querySelector('.superdoc-page-header')).toBeNull();
     expect(mount.querySelector('.superdoc-page-footer')).toBeNull();
   });
+
+  it('falls through to viewport-based calculation when scroll container is not actually scrollable (SD-2199)', () => {
+    // Reproduces the version tester bug: a wrapper has overflow:auto but is in an
+    // unconstrained flex layout (parent has only min-height, no height). The wrapper
+    // grows to fit content and scrollTop stays 0, so the scroll container branch
+    // must fall through to the viewport-based getBoundingClientRect path.
+    const pageCount = 20;
+    const painter = createDomPainter({
+      blocks: [block],
+      measures: [measure],
+      virtualization: { enabled: true, window: 5, overscan: 1, gap: 72, paddingTop: 0 },
+    });
+
+    const layout = makeLayout(pageCount);
+    painter.paint(layout, mount);
+
+    // Mount itself is not scrollable
+    Object.defineProperty(mount, 'scrollHeight', { value: 100, configurable: true });
+    Object.defineProperty(mount, 'clientHeight', { value: 600, configurable: true });
+
+    // Create a scroll container that has overflow:auto CSS but is NOT actually
+    // scrollable (scrollHeight == clientHeight, like an unconstrained flex child).
+    const scrollWrapper = document.createElement('div');
+    Object.defineProperty(scrollWrapper, 'scrollHeight', { value: 8000, configurable: true });
+    Object.defineProperty(scrollWrapper, 'clientHeight', { value: 8000, configurable: true });
+    Object.defineProperty(scrollWrapper, 'scrollTop', { value: 0, writable: true, configurable: true });
+    scrollWrapper.getBoundingClientRect = () =>
+      ({ top: 0, left: 0, right: 400, bottom: 8000, width: 400, height: 8000, x: 0, y: 0, toJSON() {} }) as DOMRect;
+
+    painter.setScrollContainer!(scrollWrapper);
+
+    // Simulate window scroll: mount's rect.top becomes negative as user scrolls
+    const layoutScrollTarget = 5000;
+    mount.getBoundingClientRect = () =>
+      ({
+        top: -layoutScrollTarget,
+        left: 0,
+        right: 400,
+        bottom: 600 - layoutScrollTarget,
+        width: 400,
+        height: 600,
+        x: 0,
+        y: -layoutScrollTarget,
+        toJSON() {},
+      }) as DOMRect;
+
+    painter.onScroll!();
+
+    // With the fix, the non-scrollable scroll container is bypassed and
+    // getBoundingClientRect is used: scrollY = -(-5000) / 1 = 5000
+    // At scrollY=5000, anchor is page 8 (topOfIndex(8)=4576, topOfIndex(9)=5148)
+    // Window = 5, overscan = 1: start = max(0, 8 - 2 - 1) = 5, end = min(19, 5 + 4 + 2) = 11
+    // Pages: 5,6,7,8,9,10,11
+    const pages = mount.querySelectorAll('.superdoc-page');
+    const indices = Array.from(pages).map((p) => Number((p as HTMLElement).dataset.pageIndex));
+
+    // Key assertion: pages beyond the initial window (0-6) should be rendered
+    expect(indices.some((i) => i > 6)).toBe(true);
+    // Anchor page (8) should be in the rendered set
+    expect(indices).toContain(8);
+  });
 });