SD-3227 - fix: enhance anchor navigation logic in TOC

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

@@ -141,7 +141,7 @@ describe('goToAnchor', () => {
     expect(result).toBe(true);
   });
 
-  it('should use nextFragmentY when bookmark is in a gap between fragments', async () => {
+  it('should use nextFragmentY when bookmark gap is closer to next fragment', async () => {
     const scrollContainer = createMockScrollContainer({ scrollTop: 0, rectTop: 0 });
     const layout = makeLayout([
       {
@@ -152,18 +152,71 @@ describe('goToAnchor', () => {
         ],
       },
     ]);
-    // Bookmark at position 50 — in the gap between fragments
+    // Bookmark at position 55 — gap to next (5) is smaller than gap to prev (15)
     const deps = makeDeps({
       layout,
       scrollContainer,
-      bookmarks: new Map([['heading1', 50]]),
+      bookmarks: new Map([['heading1', 55]]),
     });
 
     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)
+    expect(call.top).toBe(100 + 200);
+  });
+
+  it('falls back to next fragment on a PM-distance tie (preserves prior behaviour)', 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 — equidistant from both fragments
+    const deps = makeDeps({
+      layout,
+      scrollContainer,
+      bookmarks: new Map([['heading1', 50]]),
+    });
+
+    await goToAnchor(deps);
+
+    const call = (scrollContainer.scrollTo as ReturnType<typeof vi.fn>).mock.calls[0][0];
+    expect(call.top).toBe(100 + 200);
+  });
+
+  it('prefers the previous fragment when the bookmark sits just past it (SD-3227)', async () => {
+    // Mirrors the SD-3227 customer doc: the _Toc bookmark for the Section 1.3
+    // heading is embedded inside a hidden TC field and ends up at a PM
+    // position right after the paragraph's visible runs. It must still
+    // resolve to that paragraph, not to the next heading paragraph.
+    const scrollContainer = createMockScrollContainer({ scrollTop: 0, rectTop: 0 });
+    const layout = makeLayout([
+      {
+        number: 1,
+        fragments: [
+          { kind: 'para', pmStart: 0, pmEnd: 25, y: 72 },
+          { kind: 'para', pmStart: 40, pmEnd: 80, y: 300 },
+        ],
+      },
+    ]);
+    // Bookmark at position 28 — only 3 past prev.pmEnd, 12 before next.pmStart
+    const deps = makeDeps({
+      layout,
+      scrollContainer,
+      bookmarks: new Map([['heading1', 28]]),
+    });
+
+    await goToAnchor(deps);
+
+    const call = (scrollContainer.scrollTo as ReturnType<typeof vi.fn>).mock.calls[0][0];
+    // Should land on the first fragment (y=72), not the next one (y=300).
+    expect(call.top).toBe(100 + 72);
   });
 
   it('should handle Window as scrollContainer', async () => {

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

@@ -135,6 +135,9 @@ export async function goToAnchor({
     let nextFragmentPage: number | null = null;
     let nextFragmentStart: number | null = null;
     let nextFragmentY: number | null = null;
+    let prevFragmentPage: number | null = null;
+    let prevFragmentEnd: number | null = null;
+    let prevFragmentY: number | null = null;
 
     for (const page of layout.pages) {
       for (const fragment of page.fragments) {
@@ -156,14 +159,35 @@ export async function goToAnchor({
           nextFragmentStart = fragStart;
           nextFragmentY = fragment.y;
         }
+
+        // SD-3227: also track the last fragment that ends at or before our
+        // position. Bookmarks embedded inside hidden field instructions (e.g.
+        // `_Toc…` inside a TC field) often land at a PM position right after
+        // the paragraph's visible runs — they belong to that paragraph, but
+        // sit outside `fragment.pmEnd` (which is derived from visible runs).
+        // We resolve this by picking whichever neighbour is PM-closer below.
+        if (fragEnd <= pmPos && (prevFragmentEnd === null || fragEnd > prevFragmentEnd)) {
+          prevFragmentPage = page.number - 1;
+          prevFragmentEnd = fragEnd;
+          prevFragmentY = fragment.y;
+        }
       }
       if (pageIndex != null) break;
     }
 
-    // Use the page of the next fragment if bookmark is in a gap
-    if (pageIndex == null && nextFragmentPage != null) {
-      pageIndex = nextFragmentPage;
-      fragmentY = nextFragmentY;
+    // No fragment contained the bookmark — choose the neighbour whose PM
+    // range is closer. Ties go to the next fragment to preserve the prior
+    // gap-between-paragraphs behaviour.
+    if (pageIndex == null) {
+      const prevGap = prevFragmentEnd != null ? pmPos - prevFragmentEnd : Infinity;
+      const nextGap = nextFragmentStart != null ? nextFragmentStart - pmPos : Infinity;
+      if (prevFragmentPage != null && prevGap < nextGap) {
+        pageIndex = prevFragmentPage;
+        fragmentY = prevFragmentY;
+      } else if (nextFragmentPage != null) {
+        pageIndex = nextFragmentPage;
+        fragmentY = nextFragmentY;
+      }
     }
   }
 

packages/super-editor/src/editors/v1/core/super-converter/field-references/fld-preprocessors/tc-preprocessor.js

@@ -1,21 +1,51 @@
 /**
  * Processes a TC (table of contents entry) instruction and creates an `sd:tableOfContentsEntry` node.
+ *
+ * SD-3227: `w:bookmarkStart`/`w:bookmarkEnd` runs that sit inside the TC
+ * field's instruction (e.g. heading `_Toc...` targets) are lifted back out
+ * as siblings of the synthesized entry. The PM `tableOfContentsEntry` node
+ * is `atom: true`, so any bookmark left inside it would be invisible to
+ * `buildPositionMap` and `bookmarkStartNodeToBlocks`, leaving TOC
+ * navigation with no resolvable target.
+ *
+ * The hoist preserves each marker's original relative order. Bookmark
+ * markers that appear before the first non-bookmark node go before the
+ * entry; markers that appear once non-bookmark content has been seen go
+ * after the entry. This keeps `w:id`-matched start/end pairs intact and
+ * avoids the crossed-range corruption you'd get from bucketing all starts
+ * before and all ends after the entry.
+ *
  * @param {import('../../v2/types/index.js').OpenXmlNode[]} nodesToCombine The nodes to combine.
  * @param {string} instrText The instruction text.
  * @param {import('../../v2/docxHelper').ParsedDocx} [_docx] The docx object (unused).
  * @param {Array<{type: string, text?: string}>} [instructionTokens] Raw instruction tokens.
  * @returns {import('../../v2/types/index.js').OpenXmlNode[]}
  */
 export function preProcessTcInstruction(nodesToCombine, instrText, _docx, instructionTokens = null) {
+  const before = [];
+  const after = [];
+  const entryElements = [];
+  let seenContent = false;
+  for (const node of nodesToCombine) {
+    const isBookmarkMarker = node?.name === 'w:bookmarkStart' || node?.name === 'w:bookmarkEnd';
+    if (isBookmarkMarker) {
+      (seenContent ? after : before).push(node);
+    } else {
+      seenContent = true;
+      entryElements.push(node);
+    }
+  }
   return [
+    ...before,
     {
       name: 'sd:tableOfContentsEntry',
       type: 'element',
       attributes: {
         instruction: instrText,
         ...(instructionTokens ? { instructionTokens } : {}),
       },
-      elements: nodesToCombine,
+      elements: entryElements,
     },
+    ...after,
   ];
 }

packages/super-editor/src/editors/v1/core/super-converter/field-references/fld-preprocessors/tc-preprocessor.test.js

@@ -0,0 +1,101 @@
+import { describe, it, expect } from 'vitest';
+import { preProcessTcInstruction } from './tc-preprocessor.js';
+
+describe('preProcessTcInstruction', () => {
+  it('creates a single sd:tableOfContentsEntry when no bookmarks are embedded', () => {
+    const instrText = 'TC "Section 1.1 Certain Basic Terms" \\f C \\l "2"';
+
+    const result = preProcessTcInstruction([], instrText);
+
+    expect(result).toHaveLength(1);
+    expect(result[0]).toMatchObject({
+      name: 'sd:tableOfContentsEntry',
+      type: 'element',
+      attributes: { instruction: instrText },
+      elements: [],
+    });
+    expect(result[0].attributes).not.toHaveProperty('instructionTokens');
+  });
+
+  it('includes instructionTokens when provided', () => {
+    const tokens = [{ type: 'text', text: 'TC "x" \\l "2"' }];
+
+    const result = preProcessTcInstruction([], 'TC "x" \\l "2"', null, tokens);
+
+    expect(result[0].attributes.instructionTokens).toEqual(tokens);
+  });
+
+  it('hoists embedded bookmarkStart/bookmarkEnd out of the TC entry (SD-3227)', () => {
+    const bookmarkStart = {
+      name: 'w:bookmarkStart',
+      type: 'element',
+      attributes: { 'w:id': '7', 'w:name': '_Toc230123327' },
+      elements: [],
+    };
+    const bookmarkEnd = {
+      name: 'w:bookmarkEnd',
+      type: 'element',
+      attributes: { 'w:id': '7' },
+      elements: [],
+    };
+    const filler = { name: 'w:something-else', type: 'element', elements: [] };
+    const instrText = 'TC "Section 1.1 Certain Basic Terms" \\f C \\l "2"';
+
+    const result = preProcessTcInstruction([bookmarkStart, filler, bookmarkEnd], instrText);
+
+    expect(result).toEqual([
+      bookmarkStart,
+      {
+        name: 'sd:tableOfContentsEntry',
+        type: 'element',
+        attributes: { instruction: instrText },
+        elements: [filler],
+      },
+      bookmarkEnd,
+    ]);
+  });
+
+  it('keeps adjacent bookmark pairs in their original order (no crossed ranges)', () => {
+    // Bucketing all starts before and all ends after the entry would turn
+    // [start1, end1, start2, end2] into [start1, start2, entry, end1, end2],
+    // making bookmark 1's range cover bookmark 2 and vice versa.
+    const start1 = { name: 'w:bookmarkStart', type: 'element', attributes: { 'w:id': '1', 'w:name': '_Toc1' } };
+    const end1 = { name: 'w:bookmarkEnd', type: 'element', attributes: { 'w:id': '1' } };
+    const start2 = { name: 'w:bookmarkStart', type: 'element', attributes: { 'w:id': '2', 'w:name': '_Toc2' } };
+    const end2 = { name: 'w:bookmarkEnd', type: 'element', attributes: { 'w:id': '2' } };
+
+    const result = preProcessTcInstruction([start1, end1, start2, end2], 'TC "x"');
+
+    expect(result).toEqual([
+      start1,
+      end1,
+      start2,
+      end2,
+      {
+        name: 'sd:tableOfContentsEntry',
+        type: 'element',
+        attributes: { instruction: 'TC "x"' },
+        elements: [],
+      },
+    ]);
+  });
+
+  it('keeps post-content bookmark markers after the entry in their original order', () => {
+    const start = { name: 'w:bookmarkStart', type: 'element', attributes: { 'w:id': '1', 'w:name': '_Toc1' } };
+    const filler = { name: 'w:something-else', type: 'element', elements: [] };
+    const end1 = { name: 'w:bookmarkEnd', type: 'element', attributes: { 'w:id': '1' } };
+    const trailingStart = { name: 'w:bookmarkStart', type: 'element', attributes: { 'w:id': '2', 'w:name': '_Toc2' } };
+    const trailingEnd = { name: 'w:bookmarkEnd', type: 'element', attributes: { 'w:id': '2' } };
+
+    const result = preProcessTcInstruction([start, filler, end1, trailingStart, trailingEnd], 'TC "x"');
+
+    expect(result.map((n) => n.name)).toEqual([
+      'w:bookmarkStart',
+      'sd:tableOfContentsEntry',
+      'w:bookmarkEnd',
+      'w:bookmarkStart',
+      'w:bookmarkEnd',
+    ]);
+    expect(result[1].elements).toEqual([filler]);
+  });
+});