fix: bubble positioning for tcs in headers or footers

Author: harbournick

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

@@ -1692,17 +1692,12 @@ export class PresentationEditor extends EventEmitter {
     return this.getRangeRects(selection.from, selection.to, relativeTo);
   }
 
-  /**
-   * Convert an arbitrary document range into layout-based bounding rects.
-   *
-   * @param from - Start position in the ProseMirror document
-   * @param to - End position in the ProseMirror document
-   * @param relativeTo - Optional HTMLElement for coordinate reference. If provided, returns coordinates
-   *                     relative to this element's bounding rect. If omitted, returns absolute viewport
-   *                     coordinates relative to the selection overlay.
-   * @returns Array of rects, each containing pageIndex and position data (left, top, right, bottom, width, height)
-   */
-  getRangeRects(from: number, to: number, relativeTo?: HTMLElement): RangeRect[] {
+  #computeRangeRects(
+    from: number,
+    to: number,
+    relativeTo?: HTMLElement,
+    options: { forceBodySurface?: boolean } = {},
+  ): RangeRect[] {
     if (!this.#selectionOverlay) return [];
     if (!Number.isFinite(from) || !Number.isFinite(to)) return [];
 
@@ -1720,11 +1715,13 @@ export class PresentationEditor extends EventEmitter {
     let usedDomRects = false;
     const sessionMode = this.#headerFooterSession?.session?.mode ?? 'body';
     const activeNoteSession = this.#getActiveNoteStorySession();
+    const useHeaderFooterSurface = !options.forceBodySurface && sessionMode !== 'body';
+    const useNoteSurface = !options.forceBodySurface && activeNoteSession != null;
     const layoutRectSource = () => {
-      if (sessionMode !== 'body') {
+      if (useHeaderFooterSurface) {
         return this.#computeHeaderFooterSelectionRects(start, end);
       }
-      if (activeNoteSession) {
+      if (useNoteSurface) {
         return this.#computeNoteSelectionRects(start, end);
       }
       const domRects = this.#computeSelectionRectsFromDom(start, end);
@@ -1751,7 +1748,7 @@ export class PresentationEditor extends EventEmitter {
     let domCaretStart: { pageIndex: number; x: number; y: number } | null = null;
     let domCaretEnd: { pageIndex: number; x: number; y: number } | null = null;
     const pageDelta: Record<number, { dx: number; dy: number }> = {};
-    if (!usedDomRects && !activeNoteSession) {
+    if (!usedDomRects && !useNoteSurface) {
       // Geometry fallback path: apply a small DOM-based delta to reduce drift.
       try {
         domCaretStart = this.#computeDomCaretPageLocal(start);
@@ -1772,8 +1769,8 @@ export class PresentationEditor extends EventEmitter {
     }
 
     const pageHeight = this.#getBodyPageHeight();
-    const pageGap = sessionMode === 'body' ? (this.#layoutState.layout?.pageGap ?? 0) : 0;
-    const finalRects = rawRects
+    const pageGap = useHeaderFooterSurface || !this.#layoutState.layout ? 0 : (this.#layoutState.layout.pageGap ?? 0);
+    return rawRects
       .map((rect: LayoutRect, idx: number, allRects: LayoutRect[]) => {
         let adjustedX = rect.x;
         let adjustedY = rect.y;
@@ -1817,8 +1814,20 @@ export class PresentationEditor extends EventEmitter {
         };
       })
       .filter((rect: RangeRect | null): rect is RangeRect => Boolean(rect));
+  }
 
-    return finalRects;
+  /**
+   * Convert an arbitrary document range into layout-based bounding rects.
+   *
+   * @param from - Start position in the ProseMirror document
+   * @param to - End position in the ProseMirror document
+   * @param relativeTo - Optional HTMLElement for coordinate reference. If provided, returns coordinates
+   *                     relative to this element's bounding rect. If omitted, returns absolute viewport
+   *                     coordinates relative to the selection overlay.
+   * @returns Array of rects, each containing pageIndex and position data (left, top, right, bottom, width, height)
+   */
+  getRangeRects(from: number, to: number, relativeTo?: HTMLElement): RangeRect[] {
+    return this.#computeRangeRects(from, to, relativeTo);
   }
 
   /**
@@ -1853,6 +1862,42 @@ export class PresentationEditor extends EventEmitter {
     };
   }
 
+  #getThreadSelectionBounds(
+    data: { storyKey?: unknown; start?: unknown; end?: unknown; pos?: unknown },
+    relativeTo: HTMLElement | undefined,
+  ): {
+    bounds: { top: number; left: number; bottom: number; right: number; width: number; height: number };
+    rects: RangeRect[];
+    pageIndex: number;
+  } | null {
+    const start = Number.isFinite(data.start ?? data.pos) ? Number(data.start ?? data.pos) : undefined;
+    const end = Number.isFinite(data.end) ? Number(data.end) : start;
+    if (!Number.isFinite(start) || !Number.isFinite(end)) {
+      return null;
+    }
+
+    const storyKey = typeof data.storyKey === 'string' ? data.storyKey : null;
+    const rects =
+      storyKey === BODY_STORY_KEY
+        ? this.#computeRangeRects(start!, end!, relativeTo, { forceBodySurface: true })
+        : this.getRangeRects(start!, end!, relativeTo);
+
+    if (!rects.length) {
+      return null;
+    }
+
+    const bounds = this.#aggregateLayoutBounds(rects);
+    if (!bounds) {
+      return null;
+    }
+
+    return {
+      rects,
+      bounds,
+      pageIndex: rects[0]?.pageIndex ?? 0,
+    };
+  }
+
   /**
    * Remap comment positions to layout coordinates with bounds and rects.
    * Takes a positions object with threadIds as keys and position data as values.
@@ -1925,7 +1970,7 @@ export class PresentationEditor extends EventEmitter {
         return;
       }
 
-      const layoutRange = this.getSelectionBounds(start!, end!, relativeTo);
+      const layoutRange = this.#getThreadSelectionBounds(data, relativeTo);
       if (!layoutRange) {
         remapped[threadId] = data;
         return;

tests/behavior/tests/comments/body-tracked-change-anchor-stays-in-body-during-footnote-edit.spec.ts

@@ -0,0 +1,145 @@
+import path from 'node:path';
+import { fileURLToPath } from 'node:url';
+import type { Locator, Page } from '@playwright/test';
+import { test, expect } from '../../fixtures/superdoc.js';
+
+const __dirname = path.dirname(fileURLToPath(import.meta.url));
+const DOC_PATH = path.resolve(
+  __dirname,
+  '../../../../packages/super-editor/src/editors/v1/tests/data/basic-footnotes.docx',
+);
+
+test.use({
+  config: {
+    toolbar: 'full',
+    comments: 'panel',
+    trackChanges: true,
+    documentMode: 'suggesting',
+    useHiddenHostForStoryParts: true,
+    showCaret: true,
+    showSelection: true,
+  },
+});
+
+type TrackedChangePosition = {
+  key: string;
+  top: number;
+  left: number;
+  pageIndex: number | null;
+};
+
+function getFootnoteLocator(page: Page, noteId: string): Locator {
+  return page.locator(`[data-block-id^="footnote-${noteId}-"]`).first();
+}
+
+async function getBodyTrackedChangePosition(page: Page): Promise<TrackedChangePosition | null> {
+  return page.evaluate(() => {
+    const positions = (window as any).superdoc?.commentsStore?.editorCommentPositions ?? {};
+    for (const [key, entry] of Object.entries(positions)) {
+      if (!key.startsWith('tc::body::')) {
+        continue;
+      }
+
+      const bounds = (entry as { bounds?: { top?: unknown; left?: unknown } }).bounds;
+      if (!bounds || !Number.isFinite(bounds.top) || !Number.isFinite(bounds.left)) {
+        continue;
+      }
+
+      const pageIndex = (entry as { pageIndex?: unknown }).pageIndex;
+      return {
+        key,
+        top: Number(bounds.top),
+        left: Number(bounds.left),
+        pageIndex: Number.isFinite(pageIndex) ? Number(pageIndex) : null,
+      };
+    }
+
+    return null;
+  });
+}
+
+async function getTrackedChangePositionByKey(page: Page, key: string): Promise<TrackedChangePosition | null> {
+  return page.evaluate((targetKey: string) => {
+    const entry = (window as any).superdoc?.commentsStore?.editorCommentPositions?.[targetKey];
+    const bounds = entry?.bounds;
+    if (!bounds || !Number.isFinite(bounds.top) || !Number.isFinite(bounds.left)) {
+      return null;
+    }
+
+    const pageIndex = entry?.pageIndex;
+    return {
+      key: targetKey,
+      top: Number(bounds.top),
+      left: Number(bounds.left),
+      pageIndex: Number.isFinite(pageIndex) ? Number(pageIndex) : null,
+    };
+  }, key);
+}
+
+async function activateFootnote(
+  superdoc: { page: Page; waitForStable: (ms?: number) => Promise<void> },
+  noteId: string,
+) {
+  const footnote = getFootnoteLocator(superdoc.page, noteId);
+  await footnote.scrollIntoViewIfNeeded();
+  await footnote.waitFor({ state: 'visible', timeout: 15_000 });
+
+  const box = await footnote.boundingBox();
+  expect(box).toBeTruthy();
+
+  await superdoc.page.mouse.dblclick(box!.x + box!.width / 2, box!.y + box!.height / 2);
+  await superdoc.waitForStable();
+
+  await expect
+    .poll(() =>
+      superdoc.page.evaluate(() => {
+        const session = (window as any).editor?.presentationEditor?.getStorySessionManager?.()?.getActiveSession?.();
+        return session?.locator?.storyType ?? null;
+      }),
+    )
+    .toBe('footnote');
+
+  return footnote;
+}
+
+test('body tracked-change anchors stay in body space while editing a footnote in suggesting mode', async ({
+  superdoc,
+  browserName,
+}) => {
+  test.skip(
+    browserName === 'firefox',
+    'Headless Firefox does not yet persist hidden-host footnote edits through the behavior harness.',
+  );
+
+  await superdoc.loadDocument(DOC_PATH);
+  await superdoc.waitForStable();
+
+  const bodyLine = superdoc.page.locator('.superdoc-line', { hasText: 'Simple text1 with footnotes' }).first();
+  await bodyLine.waitFor({ state: 'visible', timeout: 15_000 });
+
+  const lineBox = await bodyLine.boundingBox();
+  expect(lineBox).toBeTruthy();
+
+  await superdoc.page.mouse.click(lineBox!.x + 12, lineBox!.y + lineBox!.height / 2);
+  await superdoc.page.keyboard.insertText('BODYFIX ');
+  await superdoc.waitForStable();
+
+  await expect.poll(() => getBodyTrackedChangePosition(superdoc.page)).not.toBeNull();
+  const before = await getBodyTrackedChangePosition(superdoc.page);
+  expect(before).toBeTruthy();
+
+  const footnote = await activateFootnote(superdoc, '1');
+  await expect(footnote).toContainText('This is a simple footnote');
+
+  await superdoc.page.keyboard.press('End');
+  await superdoc.page.keyboard.insertText('NOTEFIX');
+  await superdoc.waitForStable();
+
+  await expect.poll(() => getTrackedChangePositionByKey(superdoc.page, before!.key)).not.toBeNull();
+  const after = await getTrackedChangePositionByKey(superdoc.page, before!.key);
+  expect(after).toBeTruthy();
+
+  expect(after!.pageIndex).toBe(before!.pageIndex);
+  expect(Math.abs(after!.top - before!.top)).toBeLessThanOrEqual(40);
+  expect(Math.abs(after!.left - before!.left)).toBeLessThanOrEqual(40);
+});