SD-1830 - use fragment.lines for click position in multi-column layouts (#1963)

* fix: use fragment.lines for click position in multi-column layouts

* test: add behavior test for multi-column click positioning (SD-1830)

End-to-end Playwright tests that verify clicking in two-column
documents works correctly across Chromium, Firefox, and WebKit.
Covers the customer-reported TypeError from IT-407.

---------

Co-authored-by: Caio Pizzol <caio@harbourshare.com>

Author: mattConnHarbour

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

@@ -1037,7 +1037,11 @@ 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 for column width),
+      // otherwise slice from measure.lines for this fragment's range.
+      const lines = fragment.lines ?? measure.lines.slice(fragment.fromLine, fragment.toLine);
+
+      const lineIndex = findLineIndexAtY(lines, pageY, 0, lines.length);
       if (lineIndex == null) {
         logClickStage('warn', 'no-line', {
           blockId: fragment.blockId,
@@ -1046,7 +1050,8 @@ export function clickToPosition(
         });
         return null;
       }
-      const line = measure.lines[lineIndex];
+
+      const line = lines[lineIndex];
 
       const isRTL = isRtlBlock(block);
       // Type guard: Validate indent structure and ensure numeric values
@@ -1155,7 +1160,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);
@@ -2151,43 +2156,43 @@ const determineColumn = (layout: Layout, fragmentX: number): number => {
 };
 
 /**
- * Finds the line index at a given Y offset within a paragraph measure.
+ * Finds the line index at a given Y offset within a set of lines.
  *
  * This function searches within a specified range of lines to determine which line
  * contains the given Y coordinate. It validates bounds to prevent out-of-bounds
  * access in case of corrupted layout data.
  *
- * @param measure - The paragraph measure containing line data
+ * @param lines - The array of lines to search through
  * @param offsetY - The Y offset in pixels to search for
  * @param fromLine - The starting line index (inclusive)
  * @param toLine - The ending line index (exclusive)
  * @returns The line index containing the Y offset, or null if invalid
  *
  * @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, hitTestTableFragment } from '../src/index.ts';
-import type { FlowBlock, Layout, Measure } from '@superdoc/contracts';
+import type { Layout, FlowBlock, Measure, Line, ParaFragment } from '@superdoc/contracts';
 import {
   simpleLayout,
   blocks,
@@ -107,6 +107,246 @@ describe('hitTestPage with pageGap', () => {
   });
 });
 
+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.
+
+  // ─────────────────────────────────────────────────────────────────────────────
+  // REMEASURED LINES
+  // ─────────────────────────────────────────────────────────────────────────────
+  // These represent the line breaks after remeasuring at column width.
+  // The paragraph "Hello world" wraps into two lines:
+  //
+  //   remeasuredLine1: "Hello "    (run 0, chars 0-5)
+  //   remeasuredLine2: "world"     (run 0 char 5 → run 1 char 5)
+  //
+  //   ┌──────────────┐
+  //   │ H e l l o    │  ← remeasuredLine1 (y: 0-20)
+  //   │ w o r l d    │  ← remeasuredLine2 (y: 20-40)
+  //   └──────────────┘
+  //
+  const remeasuredLine1: Line = {
+    fromRun: 0,
+    fromChar: 0,
+    toRun: 0,
+    toChar: 5, // "Hello" (5 chars, space trimmed)
+    width: 100,
+    ascent: 12,
+    descent: 4,
+    lineHeight: 20,
+  };
+
+  const remeasuredLine2: Line = {
+    fromRun: 0,
+    fromChar: 5, // continues from end of line 1
+    toRun: 1,
+    toChar: 5, // "world" (5 chars)
+    width: 100,
+    ascent: 12,
+    descent: 4,
+    lineHeight: 20,
+  };
+
+  // ─────────────────────────────────────────────────────────────────────────────
+  // FLOW BLOCK (ProseMirror content)
+  // ─────────────────────────────────────────────────────────────────────────────
+  // The source paragraph content with two runs:
+  //
+  //   run 0: "Hello "  (pmStart: 1, pmEnd: 7)
+  //   run 1: "world"   (pmStart: 7, pmEnd: 12)
+  //
+  //   PM positions:  1  2  3  4  5  6  7  8  9  10 11 12
+  //   Characters:    H  e  l  l  o     w  o  r  l  d
+  //                  └─── run 0 ───┘   └─── run 1 ───┘
+  //
+  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 (full page width)
+  // ─────────────────────────────────────────────────────────────────────────────
+  // When measured at full page width, the entire paragraph fits on one line:
+  //
+  //   ┌────────────────────────────────────────┐
+  //   │ H e l l o   w o r l d                  │  ← single line (y: 0-20)
+  //   └────────────────────────────────────────┘
+  //
+  //   measure.lines.length = 1
+  //
+  const originalMeasure: Measure = {
+    kind: 'paragraph',
+    lines: [
+      {
+        fromRun: 0,
+        fromChar: 0,
+        toRun: 1,
+        toChar: 5, // entire paragraph: "Hello world"
+        width: 200,
+        ascent: 12,
+        descent: 4,
+        lineHeight: 20,
+      },
+    ],
+    totalHeight: 20,
+  };
+
+  // ─────────────────────────────────────────────────────────────────────────────
+  // FRAGMENT (positioned on page, with remeasured lines)
+  // ─────────────────────────────────────────────────────────────────────────────
+  // This fragment is placed in column 2 of a two-column layout.
+  // It contains `lines` array with the remeasured line breaks.
+  //
+  // Page layout (600px wide):
+  //
+  //   x=0        x=290  x=310       x=600
+  //   ┌──────────┐      ┌──────────┐
+  //   │ Column 1 │      │ Column 2 │
+  //   │          │      │┌────────┐│
+  //   │          │      ││ Hello  ││ ← fragment at (300, 40)
+  //   │          │      ││ world  ││
+  //   │          │      │└────────┘│
+  //   └──────────┘      └──────────┘
+  //
+  // THE BUG: fragment.fromLine=0, fragment.toLine=2 are indices into
+  // fragment.lines (length 2), but the old code used these to access
+  // measure.lines (length 1), causing measure.lines[1] → undefined
+  //
+  const fragmentWithRemeasuredLines: ParaFragment = {
+    kind: 'para',
+    blockId: 'two-column-para',
+    fromLine: 0, // index into fragment.lines (NOT measure.lines)
+    toLine: 2, // would be out-of-bounds for measure.lines!
+    x: 300, // positioned in column 2
+    y: 40,
+    width: 150,
+    pmStart: 1,
+    pmEnd: 12,
+    lines: [remeasuredLine1, remeasuredLine2], // the remeasured lines for this fragment
+  };
+
+  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 first line of the fragment:
+    //
+    //   Click point: (350, 50)
+    //
+    //   Fragment at (300, 40):
+    //   y=40  ┌──────────────┐
+    //         │ Hello    ← * │  click y=50 hits line 1 (y: 40-60)
+    //   y=60  │ world        │
+    //   y=80  └──────────────┘
+    //              x=350
+    //
+    // Without the fix: TypeError because measure.lines[1] is undefined
+    // With the fix: uses fragment.lines to find line, returns valid position
+    // ───────────────────────────────────────────────────────────────────────
+    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:
+    //
+    //   Click point: (350, 65)
+    //
+    //   Fragment at (300, 40):
+    //   y=40  ┌──────────────┐
+    //         │ Hello        │
+    //   y=60  │ world    ← * │  click y=65 hits line 2 (y: 60-80)
+    //   y=80  └──────────────┘
+    //              x=350
+    //
+    // This tests that we correctly index into fragment.lines[1] ("world")
+    // ───────────────────────────────────────────────────────────────────────
+    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 ("world" starts at position 7)
+    expect(result?.pos).toBeGreaterThanOrEqual(7);
+    expect(result?.pos).toBeLessThanOrEqual(12);
+  });
+
+  it('handles fragment without lines array (uses measure.lines)', () => {
+    // ───────────────────────────────────────────────────────────────────────
+    // Fallback test: fragment WITHOUT remeasured lines
+    //
+    // When fragment.lines is absent, we fall back to measure.lines.
+    // This is the common case for single-column layouts.
+    //
+    //   Fragment at (30, 40), width=200 (full width, no remeasure):
+    //   y=40  ┌────────────────────────────────┐
+    //         │ Hello world                ← * │  click y=50 hits line 1
+    //   y=60  └────────────────────────────────┘
+    //                    x=100
+    //
+    // ───────────────────────────────────────────────────────────────────────
+    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');
+  });
+});
+
 describe('hitTestTableFragment with rowspan (SD-1626 / IT-22)', () => {
   // Table is at x:30, y:60, width:300, height:48
   // Row 0: y:60-84 (height 24) - has 3 cells