SD-1830 - use fragment.lines for click position in multi-column layouts

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

@@ -959,7 +959,24 @@ export function clickToPosition(
     const { fragment, block, measure, pageIndex, pageY } = fragmentHit;
     // Handle paragraph fragments
     if (fragment.kind === 'para' && measure.kind === 'paragraph' && block.kind === 'paragraph') {
-      const lineIndex = findLineIndexAtY(measure, pageY, fragment.fromLine, fragment.toLine);
+      // Use fragment-specific lines when available (remeasured fragments in multi-column layouts).
+      // When a paragraph is remeasured for column width, fragment.lines contains the actual
+      // rendered lines which may differ from measure.lines.
+      let lines: Line[];
+      let fromLine: number;
+      let toLine: number;
+
+      if (fragment.lines && fragment.lines.length > 0) {
+        lines = fragment.lines;
+        fromLine = 0;
+        toLine = fragment.lines.length;
+      } else {
+        lines = measure.lines;
+        fromLine = fragment.fromLine;
+        toLine = fragment.toLine;
+      }
+
+      let lineIndex = findLineIndexAtY(lines, pageY, fromLine, toLine);
       if (lineIndex == null) {
         logClickStage('warn', 'no-line', {
           blockId: fragment.blockId,
@@ -968,7 +985,23 @@ export function clickToPosition(
         });
         return null;
       }
-      const line = measure.lines[lineIndex];
+
+      const line = lines[lineIndex];
+      // Convert to absolute index when using fragment-local lines
+      if (lines === fragment.lines) {
+        lineIndex = fragment.fromLine + lineIndex;
+      }
+
+      // Guard against undefined line (defensive check)
+      if (!line) {
+        logClickStage('warn', 'no-line', {
+          blockId: fragment.blockId,
+          pageIndex,
+          pageY,
+          reason: 'line is undefined after lookup',
+        });
+        return null;
+      }
 
       const isRTL = isRtlBlock(block);
       // Type guard: Validate indent structure and ensure numeric values
@@ -1077,7 +1110,7 @@ export function clickToPosition(
     const { cellBlock, cellMeasure, localX, localY, pageIndex } = tableHit;
 
     // Find the line at the local Y position within the cell paragraph
-    const lineIndex = findLineIndexAtY(cellMeasure, localY, 0, cellMeasure.lines.length);
+    const lineIndex = findLineIndexAtY(cellMeasure.lines, localY, 0, cellMeasure.lines.length);
     if (lineIndex != null) {
       const line = cellMeasure.lines[lineIndex];
       const isRTL = isRtlBlock(cellBlock);
@@ -2078,29 +2111,29 @@ const determineColumn = (layout: Layout, fragmentX: number): number => {
  *
  * @throws Never throws - returns null for invalid inputs
  */
-const findLineIndexAtY = (measure: Measure, offsetY: number, fromLine: number, toLine: number): number | null => {
-  if (measure.kind !== 'paragraph') return null;
+const findLineIndexAtY = (lines: Line[], offsetY: number, fromLine: number, toLine: number): number | null => {
+  if (!lines || lines.length === 0) return null;
 
   // Validate bounds to prevent out-of-bounds access
-  const lineCount = measure.lines.length;
+  const lineCount = lines.length;
   if (fromLine < 0 || toLine > lineCount || fromLine >= toLine) {
     return null;
   }
 
   let cursor = 0;
-  // Only search within the fragment's line range
+  // Only search within the specified line range
   for (let i = fromLine; i < toLine; i += 1) {
-    const line = measure.lines[i];
+    const line = lines[i];
     // Guard against undefined lines (defensive check for corrupted data)
     if (!line) return null;
 
     const next = cursor + line.lineHeight;
     if (offsetY >= cursor && offsetY < next) {
-      return i; // Return absolute line index within measure
+      return i; // Return line index within the array
     }
     cursor = next;
   }
-  // If beyond all lines, return the last line in the fragment
+  // If beyond all lines, return the last line in the range
   return toLine - 1;
 };
 

packages/layout-engine/layout-bridge/test/clickToPosition.test.ts

@@ -1,6 +1,6 @@
 import { describe, it, expect } from 'vitest';
 import { clickToPosition, hitTestPage } from '../src/index.ts';
-import type { Layout } from '@superdoc/contracts';
+import type { Layout, FlowBlock, Measure, Line, ParaFragment } from '@superdoc/contracts';
 import {
   simpleLayout,
   blocks,
@@ -100,3 +100,153 @@ describe('hitTestPage with pageGap', () => {
     expect(result?.pageIndex).toBe(1);
   });
 });
+
+describe('clickToPosition with fragment.lines', () => {
+  // Tests for multi-column documents where fragments have remeasured lines
+  // that differ from measure.lines.
+  //
+  // Example scenario - paragraph "Hello world" in a two-column layout:
+  //
+  // Original measure (full page width):     Remeasured for column width:
+  // ┌────────────────────────────────┐      ┌──────────────┐
+  // │ Hello world                    │      │ Hello        │  ← line 0
+  // └────────────────────────────────┘      │ world        │  ← line 1
+  //           (1 line)                      └──────────────┘
+  //                                              (2 lines)
+  //
+  // measure.lines = [line0]                 fragment.lines = [line0, line1]
+  //
+  // The bug: using measure.lines with fragment.fromLine/toLine indices
+  // caused out-of-bounds access when the fragment had more lines than measure.
+
+  const remeasuredLine1: Line = {
+    fromRun: 0,
+    fromChar: 0,
+    toRun: 0,
+    toChar: 5,
+    width: 100,
+    ascent: 12,
+    descent: 4,
+    lineHeight: 20,
+  };
+
+  const remeasuredLine2: Line = {
+    fromRun: 0,
+    fromChar: 5,
+    toRun: 1,
+    toChar: 5,
+    width: 100,
+    ascent: 12,
+    descent: 4,
+    lineHeight: 20,
+  };
+
+  // Block that will be rendered in two columns
+  const twoColumnBlock: FlowBlock = {
+    kind: 'paragraph',
+    id: 'two-column-para',
+    runs: [
+      { text: 'Hello ', fontFamily: 'Arial', fontSize: 16, pmStart: 1, pmEnd: 7 },
+      { text: 'world', fontFamily: 'Arial', fontSize: 16, pmStart: 7, pmEnd: 12 },
+    ],
+  };
+
+  // Original measure only has 1 line (measured at full width)
+  const originalMeasure: Measure = {
+    kind: 'paragraph',
+    lines: [
+      {
+        fromRun: 0,
+        fromChar: 0,
+        toRun: 1,
+        toChar: 5,
+        width: 200,
+        ascent: 12,
+        descent: 4,
+        lineHeight: 20,
+      },
+    ],
+    totalHeight: 20,
+  };
+
+  // Fragment in column 2 has remeasured lines with indices that don't exist in measure.lines
+  // This simulates a paragraph that wraps to multiple lines when remeasured at column width
+  const fragmentWithRemeasuredLines: ParaFragment = {
+    kind: 'para',
+    blockId: 'two-column-para',
+    fromLine: 0, // These indices are into the remeasured lines array
+    toLine: 2,
+    x: 300, // Column 2 position
+    y: 40,
+    width: 150,
+    pmStart: 1,
+    pmEnd: 12,
+    lines: [remeasuredLine1, remeasuredLine2], // The remeasured lines
+  };
+
+  const twoColumnLayout: Layout = {
+    pageSize: { w: 600, h: 800 },
+    columns: { count: 2, gap: 20 },
+    pages: [
+      {
+        number: 1,
+        fragments: [fragmentWithRemeasuredLines],
+      },
+    ],
+  };
+
+  it('uses fragment.lines when available instead of measure.lines', () => {
+    // Click in the middle of the fragment (y=50 is within the fragment which starts at y=40)
+    // This should use fragment.lines to find the line, not measure.lines
+    // Without the fix, this would throw "can't access property 'fromRun', line is undefined"
+    // because fragment.fromLine/toLine indices don't match measure.lines
+    const result = clickToPosition(twoColumnLayout, [twoColumnBlock], [originalMeasure], { x: 350, y: 50 });
+
+    expect(result).not.toBeNull();
+    expect(result?.blockId).toBe('two-column-para');
+    expect(result?.pos).toBeGreaterThanOrEqual(1);
+    expect(result?.pos).toBeLessThanOrEqual(12);
+  });
+
+  it('correctly maps click position in second line of fragment with remeasured lines', () => {
+    // Click in the second line of the fragment (y=65 is within line 2 which starts at y=40+20=60)
+    const result = clickToPosition(twoColumnLayout, [twoColumnBlock], [originalMeasure], { x: 350, y: 65 });
+
+    expect(result).not.toBeNull();
+    expect(result?.blockId).toBe('two-column-para');
+    // The click should map to a position in the second line's range
+    expect(result?.pos).toBeGreaterThanOrEqual(1);
+    expect(result?.pos).toBeLessThanOrEqual(12);
+  });
+
+  it('handles fragment without lines array (uses measure.lines)', () => {
+    // Fragment without remeasured lines - should use measure.lines
+    const fragmentWithoutLines: ParaFragment = {
+      kind: 'para',
+      blockId: 'two-column-para',
+      fromLine: 0,
+      toLine: 1,
+      x: 30,
+      y: 40,
+      width: 200,
+      pmStart: 1,
+      pmEnd: 12,
+      // No `lines` property - should fall back to measure.lines
+    };
+
+    const layoutWithoutFragmentLines: Layout = {
+      pageSize: { w: 400, h: 500 },
+      pages: [
+        {
+          number: 1,
+          fragments: [fragmentWithoutLines],
+        },
+      ],
+    };
+
+    const result = clickToPosition(layoutWithoutFragmentLines, [twoColumnBlock], [originalMeasure], { x: 100, y: 50 });
+
+    expect(result).not.toBeNull();
+    expect(result?.blockId).toBe('two-column-para');
+  });
+});