fix(layout-bridge): correct click-to-position mapping for RTL text (#2541)

* fix(layout-bridge): correct click-to-position mapping for RTL text

* fix: ignore hidden spans when computing line visual bounds

* fix: failing tests

* chore: fixed pr comments and added RTL test

Author: claudiu-ior

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

@@ -31,6 +31,14 @@ const CLASS_NAMES = {
   tableFragment: DOM_CLASS_NAMES.TABLE_FRAGMENT,
 } as const;
 
+function isRtlLine(lineEl: HTMLElement): boolean {
+  return getComputedStyle(lineEl).direction === 'rtl';
+}
+
+function isVisibleRect(rect: DOMRect): boolean {
+  return rect.width > 0 && rect.height > 0;
+}
+
 /**
  * Maps a click coordinate to a ProseMirror document position using DOM data attributes.
  *
@@ -365,69 +373,7 @@ function processFragment(fragmentEl: HTMLElement, viewX: number, viewY: number):
     }),
   );
 
-  if (spanEls.length === 0) {
-    log('No spans in line, returning lineStart:', lineStart);
-    return lineStart;
-  }
-
-  // Check if click is before first span or after last span
-  const firstRect = spanEls[0].getBoundingClientRect();
-  if (viewX <= firstRect.left) {
-    log('Click before first span, returning lineStart:', lineStart);
-    return lineStart;
-  }
-
-  const lastRect = spanEls[spanEls.length - 1].getBoundingClientRect();
-  if (viewX >= lastRect.right) {
-    log('Click after last span, returning lineEnd:', lineEnd);
-    return lineEnd;
-  }
-
-  // Find the target element (span or anchor) containing or nearest to the X coordinate
-  const targetEl = findSpanAtX(spanEls, viewX);
-  if (!targetEl) {
-    log('No target element found, returning lineStart:', lineStart);
-    return lineStart;
-  }
-
-  const spanStart = Number(targetEl.dataset.pmStart ?? 'NaN');
-  const spanEnd = Number(targetEl.dataset.pmEnd ?? 'NaN');
-  const targetRect = targetEl.getBoundingClientRect();
-
-  log('Target element:', {
-    tag: targetEl.tagName,
-    pmStart: spanStart,
-    pmEnd: spanEnd,
-    text: targetEl.textContent?.substring(0, 30),
-    visibility: targetEl.style.visibility,
-    rect: { left: targetRect.left, right: targetRect.right, width: targetRect.width },
-    pageX: viewX,
-    pageY: viewY,
-  });
-
-  if (!Number.isFinite(spanStart) || !Number.isFinite(spanEnd)) {
-    log('Element has invalid PM positions');
-    return null;
-  }
-
-  // Get the text node and find the character index
-  const firstChild = targetEl.firstChild;
-  if (!firstChild || firstChild.nodeType !== Node.TEXT_NODE || !firstChild.textContent) {
-    // Empty element or non-text node: choose closer edge
-    const elRect = targetEl.getBoundingClientRect();
-    const closerToLeft = Math.abs(viewX - elRect.left) <= Math.abs(viewX - elRect.right);
-    const snapPos = closerToLeft ? spanStart : spanEnd;
-    log('Empty/non-text element, snapping to:', { closerToLeft, snapPos });
-    return snapPos;
-  }
-
-  const textNode = firstChild as Text;
-  const charIndex = findCharIndexAtX(textNode, targetEl, viewX);
-  const pos = mapCharIndexToPm(spanStart, spanEnd, textNode.length, charIndex);
-
-  log('Character position:', { charIndex, spanStart, finalPos: pos });
-
-  return pos;
+  return resolveLinePosition(lineEl, lineStart, lineEnd, spanEls, viewX);
 }
 
 function mapCharIndexToPm(spanStart: number, spanEnd: number, textLength: number, charIndex: number): number {
@@ -505,25 +451,56 @@ function processLineElement(lineEl: HTMLElement, viewX: number): number | null {
     }),
   );
 
+  return resolveLinePosition(lineEl, lineStart, lineEnd, spanEls, viewX);
+}
+
+/**
+ * Shared logic for resolving a click's X coordinate to a ProseMirror position
+ * within a line. Used by both `processFragment` (after locating the line by Y)
+ * and `processLineElement` (when the line is already known from the hit chain).
+ *
+ * Handles RTL-aware boundary snapping, hidden-span filtering, empty-element
+ * snapping, and character-level position mapping via `findCharIndexAtX`.
+ *
+ * @internal
+ */
+function resolveLinePosition(
+  lineEl: HTMLElement,
+  lineStart: number,
+  lineEnd: number,
+  spanEls: HTMLElement[],
+  viewX: number,
+): number | null {
   if (spanEls.length === 0) {
     log('No spans in line, returning lineStart:', lineStart);
     return lineStart;
   }
 
-  // Check if click is before first span or after last span
-  const firstRect = spanEls[0].getBoundingClientRect();
-  if (viewX <= firstRect.left) {
-    log('Click before first span, returning lineStart:', lineStart);
-    return lineStart;
+  const rtl = isRtlLine(lineEl);
+
+  // Filter out non-rendered spans (display:none field annotations, hidden tracked
+  // changes, etc.) whose zero-sized rects would collapse the bounds to 0.
+  // When every rect is zero-sized (e.g. JSDOM) fall back to the unfiltered set
+  // so the downstream logic still runs.
+  const allRects = spanEls.map((el) => el.getBoundingClientRect());
+  const visibleRects = allRects.filter(isVisibleRect);
+  const boundsRects = visibleRects.length > 0 ? visibleRects : allRects;
+
+  const visualLeft = Math.min(...boundsRects.map((r) => r.left));
+  const visualRight = Math.max(...boundsRects.map((r) => r.right));
+
+  if (viewX <= visualLeft) {
+    const pos = rtl ? lineEnd : lineStart;
+    log('Click to visual left of all spans, returning:', pos);
+    return pos;
   }
 
-  const lastRect = spanEls[spanEls.length - 1].getBoundingClientRect();
-  if (viewX >= lastRect.right) {
-    log('Click after last span, returning lineEnd:', lineEnd);
-    return lineEnd;
+  if (viewX >= visualRight) {
+    const pos = rtl ? lineStart : lineEnd;
+    log('Click to visual right of all spans, returning:', pos);
+    return pos;
   }
 
-  // Find the target element containing or nearest to the X coordinate
   const targetEl = findSpanAtX(spanEls, viewX);
   if (!targetEl) {
     log('No target element found, returning lineStart:', lineStart);
@@ -532,38 +509,38 @@ function processLineElement(lineEl: HTMLElement, viewX: number): number | null {
 
   const spanStart = Number(targetEl.dataset.pmStart ?? 'NaN');
   const spanEnd = Number(targetEl.dataset.pmEnd ?? 'NaN');
-  const targetRect = targetEl.getBoundingClientRect();
 
   log('Target element:', {
     tag: targetEl.tagName,
     pmStart: spanStart,
     pmEnd: spanEnd,
     text: targetEl.textContent?.substring(0, 30),
     visibility: targetEl.style.visibility,
-    rect: { left: targetRect.left, right: targetRect.right, width: targetRect.width },
+    rect: (() => {
+      const r = targetEl.getBoundingClientRect();
+      return { left: r.left, right: r.right, width: r.width };
+    })(),
   });
 
   if (!Number.isFinite(spanStart) || !Number.isFinite(spanEnd)) {
     log('Element has invalid PM positions');
     return null;
   }
 
-  // Get the text node and find the character index
   const firstChild = targetEl.firstChild;
   if (!firstChild || firstChild.nodeType !== Node.TEXT_NODE || !firstChild.textContent) {
-    // Empty element or non-text node: choose closer edge
     const elRect = targetEl.getBoundingClientRect();
     const closerToLeft = Math.abs(viewX - elRect.left) <= Math.abs(viewX - elRect.right);
-    const snapPos = closerToLeft ? spanStart : spanEnd;
-    log('Empty/non-text element, snapping to:', { closerToLeft, snapPos });
+    const snapPos = rtl ? (closerToLeft ? spanEnd : spanStart) : closerToLeft ? spanStart : spanEnd;
+    log('Empty/non-text element, snapping to:', { closerToLeft, rtl, snapPos });
     return snapPos;
   }
 
   const textNode = firstChild as Text;
-  const charIndex = findCharIndexAtX(textNode, targetEl, viewX);
-  const pos = spanStart + charIndex;
+  const charIndex = findCharIndexAtX(textNode, viewX, rtl);
+  const pos = mapCharIndexToPm(spanStart, spanEnd, textNode.length, charIndex);
 
-  log('Character position:', { charIndex, spanStart, finalPos: pos });
+  log('Character position:', { charIndex, spanStart, rtl, finalPos: pos });
 
   return pos;
 }
@@ -628,10 +605,12 @@ function findSpanAtX(spanEls: HTMLElement[], viewX: number): HTMLElement | null
   }
 
   let targetSpan: HTMLElement = spanEls[0];
+  let minDist = Infinity;
 
   for (let i = 0; i < spanEls.length; i++) {
     const span = spanEls[i];
     const rect = span.getBoundingClientRect();
+    if (!isVisibleRect(rect)) continue;
     if (viewX >= rect.left && viewX <= rect.right) {
       log('findSpanAtX: Found containing element at index', i, {
         tag: span.tagName,
@@ -642,8 +621,9 @@ function findSpanAtX(spanEls: HTMLElement[], viewX: number): HTMLElement | null
       });
       return span;
     }
-    // Track nearest element to the right if none contain X
-    if (viewX > rect.right) {
+    const dist = Math.min(Math.abs(viewX - rect.left), Math.abs(viewX - rect.right));
+    if (dist < minDist) {
+      minDist = dist;
       targetSpan = span;
     }
   }
@@ -660,61 +640,68 @@ function findSpanAtX(spanEls: HTMLElement[], viewX: number): HTMLElement | null
 /**
  * Finds the character index in a text node closest to a given X coordinate.
  *
- * Uses binary search with `document.createRange()` to efficiently find the
- * character boundary nearest to the target X position. This provides accurate
- * click-to-character mapping even with variable-width fonts, ligatures, and
- * letter-spacing.
+ * Uses `document.caretPositionFromPoint` / `document.caretRangeFromPoint` as
+ * the primary strategy, which correctly handles RTL, bidi, and contextual
+ * shaping (Arabic ligatures, etc.). Falls back to a binary search with
+ * per-character Range rects when the caret API is unavailable or returns a
+ * result outside the target text node.
  *
  * @param textNode - The Text node containing the characters
- * @param container - The element containing the text node (span or anchor, for position reference)
  * @param targetX - The target X coordinate in viewport space
+ * @param rtl - Whether the containing line has RTL direction
  * @returns Character index (0-based) within the text node
- *
- * @example
- * ```typescript
- * const textNode = element.firstChild as Text;
- * const charIndex = findCharIndexAtX(textNode, element, 150);
- * // charIndex might be 5 if the click was near the 5th character
- * ```
  */
-function findCharIndexAtX(textNode: Text, container: HTMLElement, targetX: number): number {
+function findCharIndexAtX(textNode: Text, targetX: number, rtl: boolean): number {
   const text = textNode.textContent ?? '';
-  const baseLeft = container.getBoundingClientRect().left;
+  if (text.length === 0) return 0;
+
+  const container = textNode.parentElement;
+  if (!container) return 0;
+  const containerRect = container.getBoundingClientRect();
+  const targetY = containerRect.top + containerRect.height / 2;
+
+  // Strategy 1: Browser caret API — handles bidi / shaping natively.
+  const caretIndex = caretOffsetFromPoint(targetX, targetY, textNode);
+  if (caretIndex != null) {
+    log('findCharIndexAtX: caret API returned', caretIndex);
+    return caretIndex;
+  }
+
+  // Strategy 2: Binary search using cumulative range rects.
+  // For LTR the boundary edge is the right side of Range(0, i); for RTL it is the left side.
+  // Using the actual rect edges avoids subpixel alignment issues with the container.
+  log('findCharIndexAtX: falling back to range binary search, rtl =', rtl);
   const range = document.createRange();
 
-  // Binary search for the first character where measured X >= target X
+  const measureX = (i: number): number => {
+    if (i <= 0) {
+      return rtl ? containerRect.right : containerRect.left;
+    }
+    range.setStart(textNode, 0);
+    range.setEnd(textNode, i);
+    const r = range.getBoundingClientRect();
+    return rtl ? r.left : r.right;
+  };
+
   let lo = 0;
   let hi = text.length;
 
   while (lo < hi) {
     const mid = Math.floor((lo + hi) / 2);
-    range.setStart(textNode, 0);
-    range.setEnd(textNode, mid);
-    const w = range.getBoundingClientRect().width;
-    const x = baseLeft + w;
-    if (x < targetX) {
+    const x = measureX(mid);
+    if (rtl ? x > targetX : x < targetX) {
       lo = mid + 1;
     } else {
       hi = mid;
     }
   }
 
-  // lo is the first index where measured X >= click X
-  // Compare with previous boundary to find nearest
   const index = Math.max(0, Math.min(text.length, lo));
-
-  const measureAt = (i: number): number => {
-    range.setStart(textNode, 0);
-    range.setEnd(textNode, i);
-    return baseLeft + range.getBoundingClientRect().width;
-  };
-
-  const xAt = measureAt(index);
+  const xAt = measureX(index);
   const distAt = Math.abs(xAt - targetX);
 
-  // Check if previous boundary is closer
   if (index > 0) {
-    const xPrev = measureAt(index - 1);
+    const xPrev = measureX(index - 1);
     const distPrev = Math.abs(xPrev - targetX);
     if (distPrev < distAt) {
       return index - 1;
@@ -723,3 +710,31 @@ function findCharIndexAtX(textNode: Text, container: HTMLElement, targetX: numbe
 
   return index;
 }
+
+/**
+ * Uses the browser's native caret-from-point API to find a character offset
+ * within a specific text node. Returns null when the API is unavailable or
+ * reports a node other than the expected text node.
+ */
+function caretOffsetFromPoint(x: number, y: number, expectedNode: Text): number | null {
+  // Firefox / spec-track: caretPositionFromPoint
+  const doc = document as Document & {
+    caretPositionFromPoint?(x: number, y: number): { offsetNode: Node; offset: number } | null;
+  };
+  if (typeof doc.caretPositionFromPoint === 'function') {
+    const cp = doc.caretPositionFromPoint(x, y);
+    if (cp && cp.offsetNode === expectedNode) {
+      return cp.offset;
+    }
+  }
+
+  // WebKit / Blink: caretRangeFromPoint
+  if (typeof doc.caretRangeFromPoint === 'function') {
+    const r = doc.caretRangeFromPoint(x, y);
+    if (r && r.startContainer === expectedNode) {
+      return r.startOffset;
+    }
+  }
+
+  return null;
+}