fix(toc): anchor scroll precision within pages navigation (SD-2186)

packages/super-editor/src/core/presentation-editor/CLAUDE.md

@@ -0,0 +1,28 @@
+# PresentationEditor
+
+Wraps a hidden ProseMirror `Editor` and renders via the layout-engine pipeline (`DomPainter`).
+
+## DOM Hierarchy
+
+```
+host app scroll container (e.g. .dev-app__main, overflow: auto)  ← actual scroll viewport
+  └── #visibleHost (.presentation-editor, overflow: visible)       ← options.element, NOT scrollable
+       └── #viewportHost
+            └── #painterHost (.presentation-editor__pages)         ← has overflow CSS but NOT the scroller
+                 └── page elements (data-page-index)
+```
+
+- `#visibleHost` is the element passed as `options.element` — it is **not** the scroll container.
+- `#scrollContainer` is computed at setup via `#findScrollableAncestor(#visibleHost)` — it walks up the DOM to find the first ancestor with `overflow: auto/scroll`. This is the element that actually scrolls.
+- When implementing scroll-related features, always use `#scrollContainer` (not `#visibleHost`) for scroll position reads/writes.
+- `#scrollPageIntoView` sets `#visibleHost.scrollTop` which only works if `#visibleHost` happens to be scrollable — this is a known inconsistency; prefer using `#scrollContainer`.
+
+## Key Files
+
+| File | Purpose |
+|------|---------|
+| `PresentationEditor.ts` | Main class — lifecycle, layout orchestration, scroll, zoom |
+| `pointer-events/EditorInputManager.ts` | Click/drag handling, link clicks, selection |
+| `utils/AnchorNavigation.ts` | TOC / bookmark navigation logic |
+| `dom/PageDom.ts` | DOM queries for page elements |
+| `tests/` | Unit tests for PresentationEditor features |

packages/super-editor/src/core/presentation-editor/PresentationEditor.ts

@@ -4997,6 +4997,8 @@ export class PresentationEditor extends EventEmitter {
         bookmarks: this.#layoutState.bookmarks,
         pageGeometryHelper: this.#pageGeometryHelper ?? undefined,
         painterHost: this.#painterHost,
+        scrollContainer: this.#scrollContainer ?? this.#visibleHost,
+        zoom: this.zoom,
         scrollPageIntoView: (pageIndex) => this.#scrollPageIntoView(pageIndex),
         waitForPageMount: (pageIndex, timeoutMs) => this.#waitForPageMount(pageIndex, { timeout: timeoutMs }),
         getActiveEditor: () => this.getActiveEditor(),

packages/super-editor/src/core/presentation-editor/utils/AnchorNavigation.test.ts

@@ -0,0 +1,203 @@
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { goToAnchor, type GoToAnchorDeps } from './AnchorNavigation.js';
+
+const mockSelectionToRects = vi.fn(() => []);
+
+vi.mock('@superdoc/layout-bridge', () => ({
+  selectionToRects: (...args: unknown[]) => mockSelectionToRects(...args),
+}));
+
+vi.mock('../dom/PageDom.js', () => ({
+  getPageElementByIndex: (_host: HTMLElement, pageIndex: number) => {
+    // Return a mock page element whose getBoundingClientRect is controlled per-test
+    const el = document.createElement('div');
+    el.setAttribute('data-page-index', String(pageIndex));
+    el.scrollIntoView = vi.fn();
+    // Default rect — tests override via mockPageRect
+    el.getBoundingClientRect = () => currentPageRect;
+    return el;
+  },
+}));
+
+// Shared state that tests can set before calling goToAnchor
+let currentPageRect: DOMRect;
+
+function makeLayout(
+  pages: Array<{ number: number; fragments: Array<{ kind: string; pmStart: number; pmEnd: number; y: number }> }>,
+) {
+  return {
+    pageSize: { w: 612, h: 792 },
+    pages: pages.map((p) => ({
+      ...p,
+      numberText: String(p.number),
+      size: { w: 612, h: 792 },
+      margins: { top: 72, bottom: 72, left: 72, right: 72, header: 36, footer: 36 },
+      sectionRefs: { headerRefs: {}, footerRefs: {} },
+    })),
+  };
+}
+
+function makeDeps(overrides: Partial<GoToAnchorDeps> = {}): GoToAnchorDeps {
+  return {
+    anchor: 'heading1',
+    layout: makeLayout([
+      {
+        number: 1,
+        fragments: [{ kind: 'para', pmStart: 0, pmEnd: 100, y: 72 }],
+      },
+      {
+        number: 2,
+        fragments: [{ kind: 'para', pmStart: 100, pmEnd: 200, y: 150 }],
+      },
+    ]),
+    blocks: [],
+    measures: [],
+    bookmarks: new Map([['heading1', 50]]),
+    painterHost: document.createElement('div'),
+    scrollContainer: createMockScrollContainer(),
+    zoom: 1,
+    scrollPageIntoView: vi.fn(),
+    waitForPageMount: vi.fn(async () => true),
+    getActiveEditor: () => ({ commands: { setTextSelection: vi.fn() } }) as never,
+    timeoutMs: 5000,
+    ...overrides,
+  };
+}
+
+function createMockScrollContainer(overrides: { scrollTop?: number; rectTop?: number } = {}) {
+  const el = document.createElement('div');
+  Object.defineProperty(el, 'scrollTop', {
+    value: overrides.scrollTop ?? 0,
+    writable: true,
+  });
+  el.getBoundingClientRect = () => new DOMRect(0, overrides.rectTop ?? 0, 800, 600);
+  el.scrollTo = vi.fn();
+  return el;
+}
+
+describe('goToAnchor', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    mockSelectionToRects.mockReturnValue([]);
+    // Default page rect: page top at y=100 in screen space
+    currentPageRect = new DOMRect(0, 100, 612, 792);
+  });
+
+  it('should use scrollContainer.scrollTo with fragment Y offset', async () => {
+    const scrollContainer = createMockScrollContainer({ scrollTop: 200, rectTop: 0 });
+    const deps = makeDeps({ scrollContainer });
+
+    const result = await goToAnchor(deps);
+
+    expect(result).toBe(true);
+    expect(scrollContainer.scrollTo).toHaveBeenCalledWith({
+      top: expect.any(Number),
+      behavior: 'instant',
+    });
+
+    // pageRect.top(100) - containerRect.top(0) + scrollTop(200) + fragmentY(72) * zoom(1) = 372
+    const call = (scrollContainer.scrollTo as ReturnType<typeof vi.fn>).mock.calls[0][0];
+    expect(call.top).toBe(372);
+  });
+
+  it('should scale fragmentY by zoom factor', async () => {
+    const scrollContainer = createMockScrollContainer({ scrollTop: 0, rectTop: 0 });
+    const deps = makeDeps({ scrollContainer, zoom: 1.5 });
+
+    await goToAnchor(deps);
+
+    // pageRect.top(100) - containerRect.top(0) + scrollTop(0) + fragmentY(72) * zoom(1.5) = 208
+    const call = (scrollContainer.scrollTo as ReturnType<typeof vi.fn>).mock.calls[0][0];
+    expect(call.top).toBe(100 + 72 * 1.5);
+  });
+
+  it('should scale fragmentY at zoom < 1', async () => {
+    const scrollContainer = createMockScrollContainer({ scrollTop: 0, rectTop: 0 });
+    const deps = makeDeps({ scrollContainer, zoom: 0.5 });
+
+    await goToAnchor(deps);
+
+    const call = (scrollContainer.scrollTo as ReturnType<typeof vi.fn>).mock.calls[0][0];
+    expect(call.top).toBe(100 + 72 * 0.5);
+  });
+
+  it('should fall back to scrollIntoView when fragmentY is null', async () => {
+    // Layout with no fragments matching the bookmark position
+    const layout = makeLayout([
+      {
+        number: 1,
+        fragments: [{ kind: 'para', pmStart: 200, pmEnd: 300, y: 72 }],
+      },
+    ]);
+    // Bookmark at position 50 — no fragment contains it, and nextFragment starts at 200
+    const deps = makeDeps({
+      layout,
+      bookmarks: new Map([['heading1', 50]]),
+    });
+
+    const result = await goToAnchor(deps);
+
+    // Should still succeed — uses nextFragment fallback which sets fragmentY
+    expect(result).toBe(true);
+  });
+
+  it('should use nextFragmentY when bookmark is in a gap between fragments', async () => {
+    const scrollContainer = createMockScrollContainer({ scrollTop: 0, rectTop: 0 });
+    const layout = makeLayout([
+      {
+        number: 1,
+        fragments: [
+          { kind: 'para', pmStart: 0, pmEnd: 40, y: 72 },
+          { kind: 'para', pmStart: 60, pmEnd: 100, y: 200 },
+        ],
+      },
+    ]);
+    // Bookmark at position 50 — in the gap between fragments
+    const deps = makeDeps({
+      layout,
+      scrollContainer,
+      bookmarks: new Map([['heading1', 50]]),
+    });
+
+    await goToAnchor(deps);
+
+    // Should use nextFragmentY = 200 from the second fragment
+    const call = (scrollContainer.scrollTo as ReturnType<typeof vi.fn>).mock.calls[0][0];
+    expect(call.top).toBe(100 + 200); // pageRect.top + nextFragmentY * zoom(1)
+  });
+
+  it('should handle Window as scrollContainer', async () => {
+    const mockWindow = {
+      scrollY: 500,
+      scrollTo: vi.fn(),
+    } as unknown as Window;
+
+    const deps = makeDeps({ scrollContainer: mockWindow });
+
+    await goToAnchor(deps);
+
+    // pageRect.top(100) + scrollY(500) + fragmentY(72) * zoom(1) = 672
+    expect(mockWindow.scrollTo).toHaveBeenCalledWith({
+      top: 672,
+      behavior: 'instant',
+    });
+  });
+
+  it('should not use rect.y for fragmentY (coordinate space mismatch)', async () => {
+    // Even when selectionToRects returns a rect, we should NOT use rect.y
+    // because it's document-absolute, not page-relative like fragment.y
+    mockSelectionToRects.mockReturnValue([{ x: 72, y: 9999, width: 100, height: 20, pageIndex: 0 }]);
+
+    const scrollContainer = createMockScrollContainer({ scrollTop: 0, rectTop: 0 });
+    const deps = makeDeps({ scrollContainer });
+
+    await goToAnchor(deps);
+
+    // fragmentY should be null (not 9999), so it falls into the fragment scan
+    // which finds fragment.y = 72 for page 0
+    // BUT: since pageIndex was already set from rect, the fragment scan is skipped.
+    // fragmentY remains null, so it should fall back to scrollIntoView
+    // This is expected — when rect gives us a pageIndex but no valid fragmentY,
+    // we do the page-level scroll as a safe fallback.
+  });
+});

packages/super-editor/src/core/presentation-editor/utils/AnchorNavigation.ts

@@ -85,6 +85,8 @@ export type GoToAnchorDeps = {
   bookmarks: Map<string, number>;
   pageGeometryHelper?: PageGeometryHelper;
   painterHost: HTMLElement;
+  scrollContainer: Element | Window;
+  zoom: number;
   scrollPageIntoView: (pageIndex: number) => void;
   waitForPageMount: (pageIndex: number, timeoutMs: number) => Promise<boolean>;
   getActiveEditor: () => Editor;
@@ -99,6 +101,8 @@ export async function goToAnchor({
   bookmarks,
   pageGeometryHelper,
   painterHost,
+  scrollContainer,
+  zoom,
   scrollPageIntoView,
   waitForPageMount,
   getActiveEditor,
@@ -117,14 +121,18 @@ export async function goToAnchor({
   const rects = selectionToRects(layout, blocks, measures, pmPos, pmPos + 1, pageGeometryHelper) ?? [];
   const rect = rects[0];
 
-  // Find the page containing this position by scanning fragments
-  // Bookmarks often fall in gaps between fragments (e.g., at page/section breaks),
-  // so we also track the first fragment starting after the position as a fallback
+  // Find the page and fragment Y offset for the bookmark position.
+  // selectionToRects often returns empty for bookmarks (zero-width inline nodes),
+  // so we scan layout fragments to find the precise Y coordinate within the page.
+  // Note: rect?.y is document-absolute (not page-relative), so we only use pageIndex
+  // from the rect and always derive fragmentY from layout fragments.
   let pageIndex: number | null = rect?.pageIndex ?? null;
+  let fragmentY: number | null = null;
 
   if (pageIndex == null) {
     let nextFragmentPage: number | null = null;
     let nextFragmentStart: number | null = null;
+    let nextFragmentY: number | null = null;
 
     for (const page of layout.pages) {
       for (const fragment of page.fragments) {
@@ -136,13 +144,15 @@ export async function goToAnchor({
         // Exact match: position is within this fragment
         if (pmPos >= fragStart && pmPos < fragEnd) {
           pageIndex = page.number - 1;
+          fragmentY = fragment.y;
           break;
         }
 
         // Track the first fragment that starts after our position
         if (fragStart > pmPos && (nextFragmentStart === null || fragStart < nextFragmentStart)) {
           nextFragmentPage = page.number - 1;
           nextFragmentStart = fragStart;
+          nextFragmentY = fragment.y;
         }
       }
       if (pageIndex != null) break;
@@ -151,6 +161,7 @@ export async function goToAnchor({
     // Use the page of the next fragment if bookmark is in a gap
     if (pageIndex == null && nextFragmentPage != null) {
       pageIndex = nextFragmentPage;
+      fragmentY = nextFragmentY;
     }
   }
 
@@ -160,9 +171,29 @@ export async function goToAnchor({
   scrollPageIntoView(pageIndex);
   await waitForPageMount(pageIndex, timeoutMs);
 
-  // Scroll the page element into view
+  // Scroll to the precise position within the page using the fragment Y offset.
+  // We use the passed-in scrollContainer rather than discovering it via DOM traversal,
+  // because intermediate elements (like painterHost) may have overflow CSS but are
+  // not the actual scroll viewport.
   const pageEl = getPageElementByIndex(painterHost, pageIndex);
-  if (pageEl) {
+
+  if (pageEl && fragmentY != null) {
+    // fragmentY is in layout-space (unscaled) pixels — scale to screen-space to match
+    // getBoundingClientRect() values which already account for CSS transform: scale(zoom).
+    const scaledY = fragmentY * zoom;
+
+    if (scrollContainer instanceof Element) {
+      const pageRect = pageEl.getBoundingClientRect();
+      const containerRect = scrollContainer.getBoundingClientRect();
+      const targetY = pageRect.top - containerRect.top + scrollContainer.scrollTop + scaledY;
+      scrollContainer.scrollTo({ top: targetY, behavior: 'instant' });
+    } else {
+      // Window scroll
+      const pageRect = pageEl.getBoundingClientRect();
+      const targetY = pageRect.top + scrollContainer.scrollY + scaledY;
+      scrollContainer.scrollTo({ top: targetY, behavior: 'instant' });
+    }
+  } else if (pageEl) {
     pageEl.scrollIntoView({ behavior: 'instant', block: 'start' });
   }
 
@@ -171,8 +202,6 @@ export async function goToAnchor({
   if (activeEditor?.commands?.setTextSelection) {
     activeEditor.commands.setTextSelection({ from: pmPos, to: pmPos });
   } else {
-    // Navigation succeeded visually (page scrolled), but caret positioning is unavailable
-    // This is not an error - log a warning for debugging
     console.warn(
       '[PresentationEditor] goToAnchor: Navigation succeeded but could not move caret (editor commands unavailable)',
     );