fix(layout-bridge): defer table fragment click mapping to geometry fallback

devtools/visual-testing/tests/interactions/stories/basic-commands/table-cell-click-positioning.ts

@@ -0,0 +1,119 @@
+import { defineStory } from '@superdoc-testing/helpers';
+
+const WAIT_MS = 400;
+const WAIT_LONG_MS = 600;
+
+/**
+ * SD-1788: Cursor cannot be reliably placed in table cell
+ *
+ * Bug: Clicking in empty space inside a table cell (below the text line)
+ * would snap the cursor to a nearby paragraph outside the table instead
+ * of placing it in the correct cell. This happened because:
+ *
+ * 1. DOM-based click mapping (clickToPositionDom) searched all lines across
+ *    all cells using only Y matching, picking the wrong column
+ * 2. The snap-to-nearest fallback chose a nearby paragraph over the table cell
+ *
+ * This test verifies:
+ * 1. Clicking inside a table cell places cursor in that cell
+ * 2. Clicking in different cells moves cursor to the correct cell
+ * 3. Clicking below text in a tall cell still resolves to that cell
+ * 4. Table works correctly on page 2+ (multi-page coordinate mapping)
+ */
+export default defineStory({
+  name: 'table-cell-click-positioning',
+  description: 'Test that clicks in table cells map to correct cursor positions',
+  tickets: ['SD-1788'],
+  startDocument: null,
+  layout: true,
+  hideCaret: false,
+  hideSelection: false,
+
+  async run(page, helpers): Promise<void> {
+    const { step, focus, type, press, clickAt, waitForStable, milestone, executeCommand } = helpers;
+
+    // Step 1: Create content above the table (snap-to-nearest candidate)
+    await step('Type paragraph above table', async () => {
+      await focus();
+      await type('Paragraph above the table');
+      await waitForStable(WAIT_MS);
+      await press('Enter');
+      await press('Enter');
+      await waitForStable(WAIT_MS);
+      await milestone('paragraph-above', 'Paragraph above where table will be inserted');
+    });
+
+    // Step 2: Insert a table
+    await step('Insert table', async () => {
+      await executeCommand('insertTable', { rows: 3, cols: 3, withHeaderRow: false });
+      await waitForStable(WAIT_LONG_MS);
+      await milestone('table-inserted', 'Inserted 3x3 table');
+    });
+
+    // Step 3: Type content in first cell
+    await step('Type in first cell', async () => {
+      await type('Cell A1');
+      await waitForStable(WAIT_MS);
+      await milestone('cell-a1-text', 'Typed in first cell (A1)');
+    });
+
+    // Step 4: Navigate to second cell and type
+    await step('Type in second cell', async () => {
+      await press('Tab');
+      await waitForStable(WAIT_MS);
+      await type('Cell B1');
+      await waitForStable(WAIT_MS);
+      await milestone('cell-b1-text', 'Typed in second cell (B1)');
+    });
+
+    // Step 5: Navigate to third cell and type
+    await step('Type in third cell', async () => {
+      await press('Tab');
+      await waitForStable(WAIT_MS);
+      await type('Cell C1');
+      await waitForStable(WAIT_MS);
+      await milestone('cell-c1-text', 'Typed in third cell (C1)');
+    });
+
+    // Step 6: Navigate to second row, first cell
+    await step('Type in second row', async () => {
+      await press('Tab');
+      await waitForStable(WAIT_MS);
+      await type('Cell A2');
+      await waitForStable(WAIT_MS);
+      await milestone('cell-a2-text', 'Typed in row 2 first cell (A2)');
+    });
+
+    // Step 7: Click back into cell A1 — the key test
+    // This tests that clicking in a table cell with content places the cursor
+    // in that cell, not in the paragraph above.
+    // Table starts at ~y=127, rows are ~17px each:
+    //   Row 1: y≈127-144, Row 2: y≈144-161, Row 3: y≈161-178
+    // Use short verification text "!" to avoid wrapping which shifts row heights.
+    await step('Click into first cell (A1)', async () => {
+      await clickAt(100, 135);
+      await waitForStable(WAIT_MS);
+      await type('!');
+      await waitForStable(WAIT_MS);
+      await milestone('verify-click-a1', 'Clicked and typed "!" in A1 — should appear in first cell');
+    });
+
+    // Step 8: Click into cell B1 (middle column, same row)
+    await step('Click into middle cell (B1)', async () => {
+      await clickAt(280, 135);
+      await waitForStable(WAIT_MS);
+      await type('!');
+      await waitForStable(WAIT_MS);
+      await milestone('verify-click-b1', 'Clicked and typed "!" in B1 — should appear in middle cell');
+    });
+
+    // Step 9: Click into cell A2 (second row)
+    await step('Click into second row cell (A2)', async () => {
+      await clickAt(100, 175);
+      await waitForStable(WAIT_MS);
+      await type('!');
+      await waitForStable(WAIT_MS);
+      await milestone('verify-click-a2', 'Clicked and typed "!" in A2 — should appear in second row');
+    });
+  },
+});

packages/layout-engine/layout-bridge/src/dom-mapping.ts

@@ -28,6 +28,7 @@ const CLASS_NAMES = {
   page: DOM_CLASS_NAMES.PAGE,
   fragment: DOM_CLASS_NAMES.FRAGMENT,
   line: DOM_CLASS_NAMES.LINE,
+  tableFragment: DOM_CLASS_NAMES.TABLE_FRAGMENT,
 } as const;
 
 /**
@@ -210,6 +211,16 @@ export function clickToPositionDom(domContainer: HTMLElement, clientX: number, c
     return result;
   }
 
+  // For table fragments without a direct line hit, return null so the caller
+  // (clickToPosition in index.ts) falls back to geometry-based hit testing via
+  // hitTestTableFragment, which correctly resolves the cell by column. processFragment
+  // would search all lines across all cells using only Y matching, picking the wrong
+  // column when multiple cells share the same row height.
+  if (fragmentEl.classList.contains(CLASS_NAMES.tableFragment)) {
+    log('Table fragment without line in hit chain, deferring to geometry fallback');
+    return null;
+  }
+
   const result = processFragment(fragmentEl, viewX, viewY);
   log('=== clickToPositionDom END ===', { result });
   return result;
@@ -223,7 +234,7 @@ export function clickToPositionDom(domContainer: HTMLElement, clientX: number, c
  * @param clientY - Y coordinate in viewport space
  * @returns The page element, or null if not found
  */
-function findPageElement(domContainer: HTMLElement, clientX: number, clientY: number): HTMLElement | null {
+export function findPageElement(domContainer: HTMLElement, clientX: number, clientY: number): HTMLElement | null {
   // Check if the container itself is a page element
   if (domContainer.classList?.contains?.(CLASS_NAMES.page)) {
     return domContainer;

packages/layout-engine/layout-bridge/src/index.ts

@@ -15,7 +15,7 @@ import type {
 } from '@superdoc/contracts';
 import { computeLinePmRange as computeLinePmRangeUnified } from '@superdoc/contracts';
 import { charOffsetToPm, findCharacterAtX, measureCharacterX } from './text-measurement.js';
-import { clickToPositionDom } from './dom-mapping.js';
+import { clickToPositionDom, findPageElement } from './dom-mapping.js';
 import {
   isListItem,
   getWordLayoutConfig,
@@ -56,7 +56,7 @@ export type { HeaderFooterLayoutResult, IncrementalLayoutResult } from './increm
 export { computeDisplayPageNumber, type DisplayPageInfo } from '@superdoc/layout-engine';
 export { remeasureParagraph } from './remeasure';
 export { measureCharacterX } from './text-measurement';
-export { clickToPositionDom } from './dom-mapping';
+export { clickToPositionDom, findPageElement } from './dom-mapping';
 export { isListItem, getWordLayoutConfig, calculateTextStartIndent, extractParagraphIndent } from './list-indent-utils';
 export type { TextIndentCalculationParams } from './list-indent-utils';
 export { LayoutVersionManager } from './layout-version-manager';
@@ -222,6 +222,73 @@ const isAtomicFragment = (fragment: Fragment): fragment is AtomicFragment => {
   return fragment.kind === 'drawing' || fragment.kind === 'image';
 };
 
+/**
+ * Finds the nearest paragraph or atomic fragment to a point on a page.
+ *
+ * When a click lands in whitespace (no fragment hit), this snaps to the closest
+ * fragment by vertical distance. Used as a fallback when hitTestFragment misses.
+ */
+function snapToNearestFragment(
+  pageHit: PageHit,
+  blocks: FlowBlock[],
+  measures: Measure[],
+  pageRelativePoint: Point,
+): FragmentHit | null {
+  const fragments = pageHit.page.fragments.filter(
+    (f: Fragment | undefined): f is Fragment => f != null && typeof f === 'object',
+  );
+  let nearestHit: FragmentHit | null = null;
+  let nearestDist = Infinity;
+
+  for (const frag of fragments) {
+    const isPara = frag.kind === 'para';
+    const isAtomic = isAtomicFragment(frag);
+    if (!isPara && !isAtomic) continue;
+
+    const blockIndex = findBlockIndexByFragmentId(blocks, frag.blockId);
+    if (blockIndex === -1) continue;
+    const block = blocks[blockIndex];
+    const measure = measures[blockIndex];
+    if (!block || !measure) continue;
+
+    let fragHeight = 0;
+    if (isAtomic) {
+      fragHeight = frag.height;
+    } else if (isPara && block.kind === 'paragraph' && measure.kind === 'paragraph') {
+      fragHeight = measure.lines
+        .slice(frag.fromLine, frag.toLine)
+        .reduce((sum: number, line: Line) => sum + line.lineHeight, 0);
+    } else {
+      continue;
+    }
+
+    const top = frag.y;
+    const bottom = frag.y + fragHeight;
+    let dist: number;
+    if (pageRelativePoint.y < top) {
+      dist = top - pageRelativePoint.y;
+    } else if (pageRelativePoint.y > bottom) {
+      dist = pageRelativePoint.y - bottom;
+    } else {
+      dist = 0;
+    }
+
+    if (dist < nearestDist) {
+      nearestDist = dist;
+      const pageY = Math.max(0, Math.min(pageRelativePoint.y - top, fragHeight));
+      nearestHit = {
+        fragment: frag,
+        block,
+        measure,
+        pageIndex: pageHit.pageIndex,
+        pageY,
+      };
+    }
+  }
+
+  return nearestHit;
+}
+
 const logClickStage = (_level: 'log' | 'warn' | 'error', _stage: string, _payload: Record<string, unknown>) => {
   // No-op in production. Enable for debugging click-to-position mapping.
 };
@@ -873,23 +940,55 @@ export function clickToPosition(
 
   // Fallback to geometry-based mapping
   logClickStage('log', 'geometry-attempt', { trying: 'geometry-based mapping' });
-  const pageHit = hitTestPage(layout, containerPoint, geometryHelper);
+
+  // When normalizeClientPoint produces containerPoint, it adjusts Y by the page's DOM
+  // offset, making containerPoint page-relative rather than container-space. On page 1
+  // the offset is ~0 so it doesn't matter, but on page 2+ this causes hitTestPage to
+  // find the wrong page and pageRelativePoint to be doubly subtracted.
+  //
+  // Fix: when DOM info is available, determine the page from elementsFromPoint (same
+  // technique normalizeClientPoint uses) and treat containerPoint as already page-relative.
+  let pageHit: PageHit | null = null;
+  let isContainerPointPageRelative = false;
+
+  if (domContainer != null && clientX != null && clientY != null) {
+    const pageEl = findPageElement(domContainer, clientX, clientY);
+    if (pageEl) {
+      const domPageIndex = Number(pageEl.dataset.pageIndex ?? 'NaN');
+      if (Number.isFinite(domPageIndex) && domPageIndex >= 0 && domPageIndex < layout.pages.length) {
+        pageHit = { pageIndex: domPageIndex, page: layout.pages[domPageIndex] };
+        isContainerPointPageRelative = true;
+      }
+    }
+  }
+
+  if (!pageHit) {
+    pageHit = hitTestPage(layout, containerPoint, geometryHelper);
+  }
+
   if (!pageHit) {
     logClickStage('warn', 'no-page', {
       point: containerPoint,
     });
     return null;
   }
 
-  // Account for gaps between pages when calculating page-relative Y
-  // Calculate cumulative Y offset to this page
-  const pageTopY = geometryHelper
-    ? geometryHelper.getPageTop(pageHit.pageIndex)
-    : calculatePageTopFallback(layout, pageHit.pageIndex);
-  const pageRelativePoint: Point = {
-    x: containerPoint.x,
-    y: containerPoint.y - pageTopY,
-  };
+  // Calculate page-relative point
+  let pageRelativePoint: Point;
+  if (isContainerPointPageRelative) {
+    // containerPoint is already page-relative (normalizeClientPoint adjusted Y by page offset)
+    pageRelativePoint = containerPoint;
+  } else {
+    // containerPoint is in container-space, subtract page top to get page-relative
+    const pageTopY = geometryHelper
+      ? geometryHelper.getPageTop(pageHit.pageIndex)
+      : calculatePageTopFallback(layout, pageHit.pageIndex);
+    pageRelativePoint = {
+      x: containerPoint.x,
+      y: containerPoint.y - pageTopY,
+    };
+  }
+
   logClickStage('log', 'page-hit', {
     pageIndex: pageHit.pageIndex,
     pageRelativePoint,
@@ -898,61 +997,24 @@ export function clickToPosition(
   let fragmentHit = hitTestFragment(layout, pageHit, blocks, measures, pageRelativePoint);
 
   // If no fragment was hit (e.g., whitespace), snap to nearest hit-testable fragment on the page.
+  // But skip snap-to-nearest when the click is within a table fragment — otherwise the snap
+  // picks a nearby paragraph and returns its position, preventing clicks in empty table cell
+  // space (below text lines) from reaching hitTestTableFragment below.
   if (!fragmentHit) {
-    const page = pageHit.page;
-    const fragments = page.fragments.filter(
-      (f: Fragment | undefined): f is Fragment => f != null && typeof f === 'object',
-    );
-    let nearestHit: FragmentHit | null = null;
-    let nearestDist = Infinity;
-
-    for (const frag of fragments) {
-      const isPara = frag.kind === 'para';
-      const isAtomic = isAtomicFragment(frag);
-      if (!isPara && !isAtomic) continue;
-
-      const blockIndex = findBlockIndexByFragmentId(blocks, frag.blockId);
-      if (blockIndex === -1) continue;
-      const block = blocks[blockIndex];
-      const measure = measures[blockIndex];
-      if (!block || !measure) continue;
-
-      let fragHeight = 0;
-      if (isAtomic) {
-        fragHeight = frag.height;
-      } else if (isPara && block.kind === 'paragraph' && measure.kind === 'paragraph') {
-        fragHeight = measure.lines
-          .slice(frag.fromLine, frag.toLine)
-          .reduce((sum: number, line: Line) => sum + line.lineHeight, 0);
-      } else {
-        continue;
-      }
-
-      const top = frag.y;
-      const bottom = frag.y + fragHeight;
-      let dist: number;
-      if (pageRelativePoint.y < top) {
-        dist = top - pageRelativePoint.y;
-      } else if (pageRelativePoint.y > bottom) {
-        dist = pageRelativePoint.y - bottom;
-      } else {
-        dist = 0;
-      }
-
-      if (dist < nearestDist) {
-        nearestDist = dist;
-        const pageY = Math.max(0, Math.min(pageRelativePoint.y - top, fragHeight));
-        nearestHit = {
-          fragment: frag,
-          block,
-          measure,
-          pageIndex: pageHit.pageIndex,
-          pageY,
-        };
-      }
+    const isWithinTableFragment = pageHit.page.fragments
+      .filter((f) => f.kind === 'table')
+      .some((f) => {
+        const tf = f as TableFragment;
+        return (
+          pageRelativePoint.x >= tf.x &&
+          pageRelativePoint.x <= tf.x + tf.width &&
+          pageRelativePoint.y >= tf.y &&
+          pageRelativePoint.y <= tf.y + tf.height
+        );
+      });
+    if (!isWithinTableFragment) {
+      fragmentHit = snapToNearestFragment(pageHit, blocks, measures, pageRelativePoint);
     }
-
-    fragmentHit = nearestHit;
   }
 
   if (fragmentHit) {