diff --git a/packages/layout-engine/layout-bridge/src/index.ts b/packages/layout-engine/layout-bridge/src/index.ts index 8bf00407d2..6a9f9d3622 100644 --- a/packages/layout-engine/layout-bridge/src/index.ts +++ b/packages/layout-engine/layout-bridge/src/index.ts @@ -957,15 +957,11 @@ export function clickToPosition( // Fallback to geometry-based mapping logClickStage('log', 'geometry-attempt', { trying: 'geometry-based mapping' }); - // 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. + // Use DOM-based page detection when available. elementsFromPoint accurately identifies + // the page element under the pointer, even in edge cases where the geometry-based + // hitTestPage may return the wrong page (e.g., due to virtualization or gaps). let pageHit: PageHit | null = null; - let isContainerPointPageRelative = false; + let domPageRelativeY: number | undefined; if (domContainer != null && clientX != null && clientY != null) { const pageEl = findPageElement(domContainer, clientX, clientY); @@ -973,7 +969,15 @@ export function clickToPosition( 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; + // Compute page-relative Y directly from the page element's DOM position. + // containerPoint.y is in container-space (global layout Y) and cannot be used + // as page-relative Y — subtracting geometry page-top may not match the actual + // DOM page position due to viewport padding, margins, or virtualization offsets. + const pageRect = pageEl.getBoundingClientRect(); + const layoutPageHeight = pageHit.page.size?.h ?? layout.pageSize.h; + const domPageHeight = pageRect.height; + const effectiveZoom = domPageHeight > 0 && layoutPageHeight > 0 ? domPageHeight / layoutPageHeight : 1; + domPageRelativeY = (clientY - pageRect.top) / effectiveZoom; } } } @@ -989,21 +993,15 @@ export function clickToPosition( return null; } - // 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, - }; - } + // Calculate page-relative point. Prefer DOM-derived Y when available (accurate + // regardless of viewport offsets), fall back to geometry subtraction. + const pageTopY = geometryHelper + ? geometryHelper.getPageTop(pageHit.pageIndex) + : calculatePageTopFallback(layout, pageHit.pageIndex); + const pageRelativePoint: Point = { + x: containerPoint.x, + y: domPageRelativeY ?? containerPoint.y - pageTopY, + }; logClickStage('log', 'page-hit', { pageIndex: pageHit.pageIndex, diff --git a/packages/layout-engine/layout-bridge/test/clickToPosition-dom-page.test.ts b/packages/layout-engine/layout-bridge/test/clickToPosition-dom-page.test.ts new file mode 100644 index 0000000000..3988274e18 --- /dev/null +++ b/packages/layout-engine/layout-bridge/test/clickToPosition-dom-page.test.ts @@ -0,0 +1,301 @@ +/** + * @vitest-environment jsdom + * + * Tests for DOM-based page-relative coordinate resolution in clickToPosition. + * + * When the geometry fallback is used (DOM mapping returns null for fragment/line), + * clickToPosition must compute page-relative Y from the actual DOM page element + * position, not from geometry-based page-top calculations. This is critical on + * page 2+ where container-space Y diverges from page-relative Y. + * + * Scenario (SD-2024): A paragraph sits just above a table on page 2. Clicking on + * the paragraph's left margin (where no fragment/line is found by elementsFromPoint) + * triggers the geometry fallback. If container-space Y is used as page-relative Y, + * the click resolves to a position far below the paragraph — inside or past the + * table. The fix computes page-relative Y directly from the page element's + * bounding rect. + */ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { clickToPosition } from '../src/index.ts'; +import type { Layout, FlowBlock, Measure } from '@superdoc/contracts'; + +/** + * Helper: temporarily mock document.elementsFromPoint for the duration of `run()`. + */ +function withMockedElementsFromPoint(elements: Element[], run: () => void): void { + const originalElementsFromPoint = document.elementsFromPoint; + document.elementsFromPoint = (_x: number, _y: number) => elements; + + try { + run(); + } finally { + document.elementsFromPoint = originalElementsFromPoint; + } +} + +describe('clickToPosition: DOM-based page-relative Y on page 2+ (SD-2024)', () => { + let container: HTMLElement; + + // Layout: page 1 has a filler paragraph, page 2 has a paragraph followed by a table. + // The paragraph on page 2 is at y=40 (page-relative), table at y=120. + const paraBlock: FlowBlock = { + kind: 'paragraph', + id: 'page2-para', + runs: [{ text: 'In witness to the commitment', fontFamily: 'Arial', fontSize: 11, pmStart: 100, pmEnd: 128 }], + }; + + const tableBlock: FlowBlock = { + kind: 'table', + id: 'page2-table', + rows: [ + { + id: 'row-0', + cells: [ + { + id: 'cell-0', + blocks: [ + { + kind: 'paragraph', + id: 'cell-para', + runs: [{ text: 'Signature', fontFamily: 'Arial', fontSize: 11, pmStart: 130, pmEnd: 139 }], + }, + ], + attrs: { padding: { top: 2, bottom: 2, left: 4, right: 4 } }, + }, + ], + }, + ], + }; + + const paraMeasure: Measure = { + kind: 'paragraph', + lines: [ + { + fromRun: 0, + fromChar: 0, + toRun: 0, + toChar: 28, + width: 300, + ascent: 10, + descent: 4, + lineHeight: 16, + }, + ], + totalHeight: 16, + }; + + const tableMeasure: Measure = { + kind: 'table', + rows: [ + { + height: 40, + cells: [ + { + width: 200, + height: 40, + gridColumnStart: 0, + blocks: [ + { + kind: 'paragraph', + lines: [ + { + fromRun: 0, + fromChar: 0, + toRun: 0, + toChar: 9, + width: 80, + ascent: 10, + descent: 4, + lineHeight: 16, + }, + ], + totalHeight: 16, + }, + ], + }, + ], + }, + ], + columnWidths: [200], + totalWidth: 200, + totalHeight: 40, + }; + + const page1Para: FlowBlock = { + kind: 'paragraph', + id: 'page1-para', + runs: [{ text: 'Page 1 content here', fontFamily: 'Arial', fontSize: 11, pmStart: 1, pmEnd: 20 }], + }; + + const page1Measure: Measure = { + kind: 'paragraph', + lines: [ + { + fromRun: 0, + fromChar: 0, + toRun: 0, + toChar: 19, + width: 150, + ascent: 10, + descent: 4, + lineHeight: 16, + }, + ], + totalHeight: 16, + }; + + // Page 2 paragraph at y=40, table at y=120 (page-relative) + const layout: Layout = { + pageSize: { w: 600, h: 800 }, + pageGap: 24, + pages: [ + { + number: 1, + fragments: [ + { + kind: 'para', + blockId: 'page1-para', + fromLine: 0, + toLine: 1, + x: 72, + y: 40, + width: 456, + pmStart: 1, + pmEnd: 20, + }, + ], + }, + { + number: 2, + fragments: [ + { + kind: 'para', + blockId: 'page2-para', + fromLine: 0, + toLine: 1, + x: 72, + y: 40, + width: 456, + pmStart: 100, + pmEnd: 128, + }, + { + kind: 'table', + blockId: 'page2-table', + fromRow: 0, + toRow: 1, + x: 72, + y: 120, + width: 200, + height: 40, + }, + ], + }, + ], + }; + + const allBlocks = [page1Para, paraBlock, tableBlock]; + const allMeasures = [page1Measure, paraMeasure, tableMeasure]; + + beforeEach(() => { + container = document.createElement('div'); + container.style.position = 'absolute'; + container.style.left = '0px'; + container.style.top = '0px'; + document.body.appendChild(container); + }); + + afterEach(() => { + document.body.removeChild(container); + }); + + it('resolves click on page 2 paragraph to paragraph position, not table', () => { + // Set up DOM with two page elements. + // Page 1 is at DOM top=0, height=800. + // Page 2 is at DOM top=824 (800 + 24 gap), height=800. + container.innerHTML = ` +
+
+ `; + + const page2El = container.querySelectorAll('.superdoc-page')[1] as HTMLElement; + + // Mock elementsFromPoint to return only the page element (no fragment/line found). + // This simulates clicking on the left margin of the paragraph where no fragment + // DOM element is under the pointer. + withMockedElementsFromPoint([page2El, container], () => { + // clientY = 864 → page 2 top (824) + 40 = paragraph Y on page 2 + // container-space Y = 864 (same as clientY at zoom=1 with no offset) + // Page-relative Y should be 864 - 824 = 40 (where the paragraph is) + const result = clickToPosition( + layout, + allBlocks, + allMeasures, + { x: 72, y: 864 }, // container-space point + container, // DOM container + 72, // clientX + 864, // clientY + ); + + expect(result).not.toBeNull(); + // Should resolve to the paragraph (PM 100-128), NOT the table (PM 130-139) + expect(result!.pos).toBeGreaterThanOrEqual(100); + expect(result!.pos).toBeLessThanOrEqual(128); + expect(result!.blockId).toBe('page2-para'); + expect(result!.pageIndex).toBe(1); + }); + }); + + it('falls back to geometry when no DOM container is provided', () => { + // Without DOM container, uses pure geometry path. + // container-space Y = 864, geometry page 2 top = 824 (800 + 24 gap) + // page-relative Y = 864 - 824 = 40 → should hit the paragraph + const result = clickToPosition( + layout, + allBlocks, + allMeasures, + { x: 72, y: 864 }, // container-space point + // No DOM container — geometry fallback + ); + + expect(result).not.toBeNull(); + expect(result!.pos).toBeGreaterThanOrEqual(100); + expect(result!.pos).toBeLessThanOrEqual(128); + expect(result!.blockId).toBe('page2-para'); + expect(result!.pageIndex).toBe(1); + }); + + it('DOM-based Y handles zoom correctly', () => { + // At 2x zoom, DOM page height is 1600px but layout height is 800. + // Page 2 DOM top = (800*2) + 24*2 = 1648 + // clientY = 1728 → page-relative = (1728 - 1648) / 2 = 40 + container.innerHTML = ` +
+
+ `; + + const page2El = container.querySelectorAll('.superdoc-page')[1] as HTMLElement; + + withMockedElementsFromPoint([page2El, container], () => { + const result = clickToPosition( + layout, + allBlocks, + allMeasures, + { x: 72, y: 864 }, // container-space (layout coordinates, not zoomed) + container, + 144, // clientX at 2x + 1728, // clientY at 2x: page2 DOM top (1648) + 40*2 = 1728 + ); + + expect(result).not.toBeNull(); + // Should still resolve to the paragraph + expect(result!.pos).toBeGreaterThanOrEqual(100); + expect(result!.pos).toBeLessThanOrEqual(128); + expect(result!.blockId).toBe('page2-para'); + }); + }); +}); diff --git a/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts b/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts index 590fadbca7..4dc83caa1e 100644 --- a/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts +++ b/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts @@ -3946,6 +3946,19 @@ export class PresentationEditor extends EventEmitter { return; } + // When dragging across mark boundaries, the selection can briefly land in the + // 2-position structural gap between adjacent runs, producing zero DOM rects for + // one frame. Preserve the last overlay only during active drag to prevent flicker. + // Outside drag (scroll, programmatic changes), zero rects means the DOM is stale + // or virtualized — clearing the overlay is the safer default. + if (domRects.length === 0 && from !== to && this.#editorInputManager?.isDragging) { + debugLog('warn', '[drawSelection] zero rects for non-collapsed selection — preserving last overlay', { + from, + to, + }); + return; + } + try { this.#localSelectionLayer.innerHTML = ''; const isFieldAnnotationSelection = diff --git a/packages/super-editor/src/core/presentation-editor/dom/DomPositionIndex.ts b/packages/super-editor/src/core/presentation-editor/dom/DomPositionIndex.ts index ab34c2a057..ae9dd8194e 100644 --- a/packages/super-editor/src/core/presentation-editor/dom/DomPositionIndex.ts +++ b/packages/super-editor/src/core/presentation-editor/dom/DomPositionIndex.ts @@ -265,14 +265,25 @@ export class DomPositionIndex { * Finds all index entries whose position ranges overlap with the given range. * * @param from - The start of the query range (inclusive) - * @param to - The end of the query range (exclusive) + * @param to - The end of the query range (exclusive by default, inclusive with `boundaryInclusive`) + * @param options - Options controlling boundary behavior * @returns Array of entries that overlap the range, in index order * * @remarks - * An entry overlaps the query range [start, end) if: + * By default, an entry overlaps the query range [start, end) if: * - entry.pmStart < end (entry starts before query range ends) * - entry.pmEnd > start (entry ends after query range starts) * + * When `boundaryInclusive` is true, the overlap condition becomes [start, end]: + * - entry.pmStart <= end (entry starts at or before query range ends) + * - entry.pmEnd >= start (entry ends at or after query range starts) + * + * Use `boundaryInclusive: true` for selection overlay rendering where positions at + * run boundaries (e.g., between two adjacent text runs with different marks) need to + * resolve to adjacent DOM entries. ProseMirror run nodes create a 2-position gap + * between adjacent text spans; inclusive boundaries ensure entries touching the gap + * are found. + * * The algorithm: * 1. Normalizes the range (swaps from/to if necessary) * 2. Uses binary search to find the first potentially overlapping entry @@ -290,7 +301,7 @@ export class DomPositionIndex { * - Zero-length ranges (from === to) return empty array * - Reversed ranges are automatically normalized (from > to is handled) */ - findEntriesInRange(from: number, to: number): DomPositionIndexEntry[] { + findEntriesInRange(from: number, to: number, options?: { boundaryInclusive?: boolean }): DomPositionIndexEntry[] { if (!Number.isFinite(from) || !Number.isFinite(to)) return []; const start = Math.min(from, to); const end = Math.max(from, to); @@ -299,6 +310,8 @@ export class DomPositionIndex { const entries = this.#entries; if (entries.length === 0) return []; + const inclusive = options?.boundaryInclusive === true; + // Find first entry whose pmStart <= start, then adjust forward if it ends before start. let lo = 0; let hi = entries.length; @@ -319,8 +332,8 @@ export class DomPositionIndex { const out: DomPositionIndexEntry[] = []; for (let i = idx; i < entries.length; i += 1) { const entry = entries[i]; - if (entry.pmStart >= end) break; - if (entry.pmEnd <= start) continue; + if (inclusive ? entry.pmStart > end : entry.pmStart >= end) break; + if (inclusive ? entry.pmEnd < start : entry.pmEnd <= start) continue; out.push(entry); } return out; diff --git a/packages/super-editor/src/core/presentation-editor/dom/DomSelectionGeometry.ts b/packages/super-editor/src/core/presentation-editor/dom/DomSelectionGeometry.ts index 6048962fba..59c15cc1d1 100644 --- a/packages/super-editor/src/core/presentation-editor/dom/DomSelectionGeometry.ts +++ b/packages/super-editor/src/core/presentation-editor/dom/DomSelectionGeometry.ts @@ -164,7 +164,11 @@ export function computeSelectionRectsFromDom( if (sliceFrom >= sliceTo) continue; // Identify representative DOM elements for the slice boundaries on this mounted page. - let sliceEntries = options.domPositionIndex.findEntriesInRange(sliceFrom, sliceTo); + // Use boundaryInclusive to include entries whose boundaries touch the slice range. + // This is critical for selections at run boundaries (mark changes) where PM positions + // fall in the 2-position gap between adjacent text spans. + const rangeOpts = { boundaryInclusive: true }; + let sliceEntries = options.domPositionIndex.findEntriesInRange(sliceFrom, sliceTo, rangeOpts); if (sliceEntries.length === 0) { // Nothing mounted for this PM interval on this page (virtualized or empty). continue; @@ -177,7 +181,7 @@ export function computeSelectionRectsFromDom( if (pageEntries.length === 0 && !rebuiltOnce) { options.rebuildDomPositionIndex(); rebuiltOnce = true; - sliceEntries = options.domPositionIndex.findEntriesInRange(sliceFrom, sliceTo); + sliceEntries = options.domPositionIndex.findEntriesInRange(sliceFrom, sliceTo, rangeOpts); pageEntries = filterPageEntries(sliceEntries); } @@ -213,7 +217,7 @@ export function computeSelectionRectsFromDom( if ((!startEntry?.el?.isConnected || !endEntry?.el?.isConnected) && !rebuiltOnce) { options.rebuildDomPositionIndex(); rebuiltOnce = true; - sliceEntries = options.domPositionIndex.findEntriesInRange(sliceFrom, sliceTo); + sliceEntries = options.domPositionIndex.findEntriesInRange(sliceFrom, sliceTo, rangeOpts); pageEntries = filterPageEntries(sliceEntries); if (pageEntries.length === 0) { continue; diff --git a/packages/super-editor/src/core/presentation-editor/pointer-events/EditorInputManager.ts b/packages/super-editor/src/core/presentation-editor/pointer-events/EditorInputManager.ts index 4568e07c42..7b7b1ea9bc 100644 --- a/packages/super-editor/src/core/presentation-editor/pointer-events/EditorInputManager.ts +++ b/packages/super-editor/src/core/presentation-editor/pointer-events/EditorInputManager.ts @@ -507,6 +507,44 @@ export class EditorInputManager { return calculateExtendedSelection(layoutState?.blocks ?? [], anchor, head, mode); } + /** + * When the drag anchor is outside an isolating node (table), prevent the head + * from resolving inside one. If the head is inside a table cell, clamp it to + * just before or after the table boundary (depending on drag direction). + * + * Selections that span PAST a table (anchor before, head after) are allowed — + * only positions resolving INSIDE the table are clamped. + */ + #clampHeadAtIsolatingBoundary(doc: ProseMirrorNode, anchor: number, head: number): number { + const forward = head >= anchor; + + try { + const $head = doc.resolve(head); + // Find the outermost isolating ancestor. Walk from innermost to outermost, + // tracking the shallowest isolating depth. Using the outermost ensures that + // we clamp to just before/after the entire table, not to a boundary between + // cells within the same table. + let isolatingDepth = -1; + for (let d = $head.depth; d > 0; d--) { + const node = $head.node(d); + if (node.type.spec.isolating || node.type.spec.tableRole === 'table') { + isolatingDepth = d; + } + } + + if (isolatingDepth > 0) { + const boundary = forward ? $head.before(isolatingDepth) : $head.after(isolatingDepth); + const near = Selection.near(doc.resolve(boundary), forward ? -1 : 1); + if (near instanceof TextSelection) return near.head; + return anchor; + } + } catch { + /* position resolution failed */ + } + + return head; + } + #shouldUseCellSelection(currentTableHit: TableHitResult | null): boolean { return shouldUseCellSelectionFromHelper(currentTableHit, this.#cellAnchor, this.#cellDragMode); } @@ -1001,12 +1039,23 @@ export class EditorInputManager { this.#dragAnchorPageIndex = hit.pageIndex; this.#pendingMarginClick = this.#callbacks.computePendingMarginClick?.(event.pointerId, x, y) ?? null; - // Check for table cell selection + // Check for table cell selection. + // Verify that the resolved click position is actually inside the table before + // activating cell selection. hitTestTable uses geometry-based coordinates that + // may have small offsets from the DOM, causing false positives for clicks on + // paragraphs near table boundaries. const tableHit = this.#hitTestTable(x, y); if (tableHit) { const tablePos = this.#getTablePosFromHit(tableHit); - if (tablePos !== null) { + const hitIsInsideTable = + tablePos !== null && + doc && + hit.pos >= tablePos && + hit.pos <= tablePos + (doc.nodeAt(tablePos)?.nodeSize ?? 0); + if (tablePos !== null && hitIsInsideTable) { this.#setCellAnchor(tableHit, tablePos); + } else { + this.#clearCellAnchor(); } } else { this.#clearCellAnchor(); @@ -1723,7 +1772,15 @@ export class EditorInputManager { // Text selection mode const anchor = this.#dragAnchor!; - const head = hit.pos; + let head = hit.pos; + + // When the drag started outside a table, prevent the head from entering an isolating + // node (table). If the head resolves inside a table, ProseMirror-tables' appendTransaction + // converts the TextSelection into a CellSelection, causing the anchor to jump. + if (!this.#cellAnchor) { + head = this.#clampHeadAtIsolatingBoundary(doc, anchor, head); + } + const { selAnchor, selHead } = this.#calculateExtendedSelection(anchor, head, this.#dragExtensionMode); try { diff --git a/packages/super-editor/src/core/presentation-editor/tests/DomPositionIndex.test.ts b/packages/super-editor/src/core/presentation-editor/tests/DomPositionIndex.test.ts index 112a06fca4..13ffa0ac7c 100644 --- a/packages/super-editor/src/core/presentation-editor/tests/DomPositionIndex.test.ts +++ b/packages/super-editor/src/core/presentation-editor/tests/DomPositionIndex.test.ts @@ -885,6 +885,76 @@ describe('DomPositionIndex', () => { const result = index.findElementsInRange(3, 12).map((el) => el.textContent); expect(result).toEqual(['first', 'second']); }); + + it('excludes boundary entries by default (half-open semantics)', () => { + // Default behavior: [start, end) half-open range — entries touching the boundary + // at exactly start or end are excluded. This is correct for decorations. + const container = document.createElement('div'); + container.innerHTML = ` +
+ first + second +
+ `; + + const index = new DomPositionIndex(); + index.rebuild(container); + + // Range [5, 7] touches both boundaries but overlaps neither in half-open mode + expect(index.findElementsInRange(5, 7)).toEqual([]); + }); + + it('includes adjacent entries at run boundaries with boundaryInclusive', () => { + // Simulates two adjacent text runs with different marks (e.g., bold → italic). + // ProseMirror run nodes create a 2-position gap between spans (close + open tokens). + // The range [5, 7] spans exactly this gap. With boundaryInclusive, both boundary + // entries are included to prevent selection overlay flicker during drag. + const container = document.createElement('div'); + container.innerHTML = ` +
+ bold + italic +
+ `; + + const index = new DomPositionIndex(); + index.rebuild(container); + + const result = index.findEntriesInRange(5, 7, { boundaryInclusive: true }).map((e) => e.el.textContent); + expect(result).toEqual(['bold', 'italic']); + }); + + it('includes entry whose pmEnd equals query start with boundaryInclusive', () => { + const container = document.createElement('div'); + container.innerHTML = ` +
+ first + second +
+ `; + + const index = new DomPositionIndex(); + index.rebuild(container); + + const result = index.findEntriesInRange(5, 10, { boundaryInclusive: true }).map((e) => e.el.textContent); + expect(result).toEqual(['first', 'second']); + }); + + it('includes entry whose pmStart equals query end with boundaryInclusive', () => { + const container = document.createElement('div'); + container.innerHTML = ` +
+ first + second +
+ `; + + const index = new DomPositionIndex(); + index.rebuild(container); + + const result = index.findEntriesInRange(3, 7, { boundaryInclusive: true }).map((e) => e.el.textContent); + expect(result).toEqual(['first', 'second']); + }); }); describe('edge cases - leafOnly option', () => { diff --git a/packages/super-editor/src/core/presentation-editor/tests/DomSelectionGeometry.test.ts b/packages/super-editor/src/core/presentation-editor/tests/DomSelectionGeometry.test.ts index d8d396d5e3..9e92d7487d 100644 --- a/packages/super-editor/src/core/presentation-editor/tests/DomSelectionGeometry.test.ts +++ b/packages/super-editor/src/core/presentation-editor/tests/DomSelectionGeometry.test.ts @@ -624,6 +624,86 @@ describe('computeSelectionRectsFromDom', () => { }); }); + describe('selection across mark boundaries (SD-2024)', () => { + it('returns rects when selection spans the structural gap between two differently-marked runs', () => { + // Simulates two adjacent text runs with different marks (e.g., bold → italic). + // ProseMirror run nodes occupy 2 positions (open + close tokens), creating a + // gap between the text spans: + // [1..5] positions 5-6 = structural tokens [7..12] + // A selection exactly at the boundary (from=5, to=7) must still find DOM + // entries and produce highlight rects — not return empty and cause flicker. + painterHost.innerHTML = ` +
+
+ bold + italic +
+
+ `; + + const layout = createMockLayout([{ pmStart: 1, pmEnd: 12 }]); + domPositionIndex.rebuild(painterHost); + + const pageEl = painterHost.querySelector('.superdoc-page') as HTMLElement; + pageEl.getBoundingClientRect = vi.fn(() => createRect(0, 0, 612, 792)); + + const mockRange = { + setStart: vi.fn(), + setEnd: vi.fn(), + getClientRects: vi.fn(() => [createRect(40, 20, 60, 16)]), + } as unknown as Range; + + const originalCreateRange = document.createRange; + document.createRange = vi.fn(() => mockRange); + + const options = createOptions(layout); + // from=5 to=7: exactly the structural gap between the two runs + const rects = computeSelectionRectsFromDom(options, 5, 7); + + expect(rects).not.toBe(null); + expect(rects!.length).toBeGreaterThan(0); + + document.createRange = originalCreateRange; + }); + + it('returns rects when selection starts inside one run and ends at the next run boundary', () => { + // Selection from mid-first-run to the start of the second run. + // Without boundaryInclusive, the second span (pmStart=7) would be excluded + // when the selection ends at exactly 7. + painterHost.innerHTML = ` +
+
+ bold + italic +
+
+ `; + + const layout = createMockLayout([{ pmStart: 1, pmEnd: 12 }]); + domPositionIndex.rebuild(painterHost); + + const pageEl = painterHost.querySelector('.superdoc-page') as HTMLElement; + pageEl.getBoundingClientRect = vi.fn(() => createRect(0, 0, 612, 792)); + + const mockRange = { + setStart: vi.fn(), + setEnd: vi.fn(), + getClientRects: vi.fn(() => [createRect(10, 20, 90, 16)]), + } as unknown as Range; + + const originalCreateRange = document.createRange; + document.createRange = vi.fn(() => mockRange); + + const options = createOptions(layout); + const rects = computeSelectionRectsFromDom(options, 3, 7); + + expect(rects).not.toBe(null); + expect(rects!.length).toBeGreaterThan(0); + + document.createRange = originalCreateRange; + }); + }); + describe('multi-page selections', () => { it('computes rects spanning multiple pages', () => { painterHost.innerHTML = ` diff --git a/packages/super-editor/src/core/presentation-editor/tests/selection-isolating-boundary.test.js b/packages/super-editor/src/core/presentation-editor/tests/selection-isolating-boundary.test.js new file mode 100644 index 0000000000..842de3ffff --- /dev/null +++ b/packages/super-editor/src/core/presentation-editor/tests/selection-isolating-boundary.test.js @@ -0,0 +1,209 @@ +/** + * Tests for selection behavior at isolating node boundaries (tables). + * + * When drag-selecting text in a paragraph near a table, the selection head + * must not resolve inside the table. If it does, ProseMirror-tables' + * appendTransaction converts the TextSelection into a CellSelection, + * causing the anchor to jump into the table — visually the selection + * "flickers" or jumps away from where the user started dragging. + * + * These tests verify the behavioral contract: + * 1. A TextSelection from a paragraph position to inside a table should + * be clamped so the head stays outside the table. + * 2. A TextSelection that spans PAST a table (anchor before, head after) + * should be allowed — only heads INSIDE isolating nodes are clamped. + */ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { TextSelection, Selection } from 'prosemirror-state'; +import { initTestEditor } from '@tests/helpers/helpers.js'; + +/** + * Replicates the clamping logic from EditorInputManager.#clampHeadAtIsolatingBoundary. + * Extracted here as a pure function for testability. + */ +function clampHeadAtIsolatingBoundary(doc, anchor, head) { + const forward = head >= anchor; + + try { + const $head = doc.resolve(head); + // Find the outermost isolating ancestor + let isolatingDepth = -1; + for (let d = $head.depth; d > 0; d--) { + const node = $head.node(d); + if (node.type.spec.isolating || node.type.spec.tableRole === 'table') { + isolatingDepth = d; + } + } + + if (isolatingDepth > 0) { + const boundary = forward ? $head.before(isolatingDepth) : $head.after(isolatingDepth); + const near = Selection.near(doc.resolve(boundary), forward ? -1 : 1); + if (near instanceof TextSelection) return near.head; + return anchor; + } + } catch { + /* position resolution failed */ + } + + return head; +} + +// Document structure: +// doc +// paragraph "Before the table" (positions ~1-19) +// table (isolating node) +// tableRow +// tableCell +// paragraph "Cell content" +// tableCell +// paragraph "Cell two" +// paragraph "After the table" (positions after table end) +const docJson = { + type: 'doc', + content: [ + { + type: 'paragraph', + content: [{ type: 'run', content: [{ type: 'text', text: 'Before the table' }] }], + }, + { + type: 'table', + content: [ + { + type: 'tableRow', + content: [ + { + type: 'tableCell', + attrs: { colwidth: [100] }, + content: [ + { + type: 'paragraph', + content: [{ type: 'run', content: [{ type: 'text', text: 'Cell content' }] }], + }, + ], + }, + { + type: 'tableCell', + attrs: { colwidth: [100] }, + content: [ + { + type: 'paragraph', + content: [{ type: 'run', content: [{ type: 'text', text: 'Cell two' }] }], + }, + ], + }, + ], + }, + ], + }, + { + type: 'paragraph', + content: [{ type: 'run', content: [{ type: 'text', text: 'After the table' }] }], + }, + ], +}; + +describe('selection clamping at isolating boundaries (SD-2024)', () => { + let editor; + let doc; + let tableStart; + let tableEnd; + let beforeParaStart; + let afterParaStart; + + beforeAll(() => { + ({ editor } = initTestEditor({ loadFromSchema: true, content: docJson })); + doc = editor.state.doc; + + // Find table boundaries + doc.descendants((node, pos) => { + if (node.type.name === 'table') { + tableStart = pos; + tableEnd = pos + node.nodeSize; + } + }); + + // First paragraph content starts at pos 1+1 = 2 (doc boundary + paragraph boundary) + // But we need a text position — let's find it + doc.descendants((node, pos) => { + if (beforeParaStart == null && node.isText && node.text.includes('Before')) { + beforeParaStart = pos; + } + }); + + // Find text position after table + let pastTable = false; + doc.descendants((node, pos) => { + if (node.type.name === 'table') pastTable = true; + if (afterParaStart == null && pastTable && node.isText && node.text.includes('After')) { + afterParaStart = pos; + } + }); + }); + + afterAll(() => { + editor.destroy(); + }); + + it('document has expected structure: paragraph, table, paragraph', () => { + const topLevelTypes = []; + doc.forEach((node) => topLevelTypes.push(node.type.name)); + expect(topLevelTypes).toEqual(['paragraph', 'table', 'paragraph']); + }); + + it('clamps head when it resolves inside a table (forward drag)', () => { + // Simulate: anchor in first paragraph, head inside a table cell + const anchor = beforeParaStart + 3; // somewhere in "Before the table" + const headInsideTable = tableStart + 5; // inside the table + + const clamped = clampHeadAtIsolatingBoundary(doc, anchor, headInsideTable); + + // Clamped head should be outside the table (before it) + expect(clamped).toBeLessThan(tableStart); + // Should still be a valid text position + expect(clamped).toBeGreaterThan(0); + }); + + it('clamps head when it resolves inside a table (backward drag)', () => { + // Simulate: anchor after table, head inside table (dragging backwards) + const anchor = afterParaStart + 3; + const headInsideTable = tableStart + 5; + + const clamped = clampHeadAtIsolatingBoundary(doc, anchor, headInsideTable); + + // Clamped head should be outside the table (after it) + expect(clamped).toBeGreaterThan(tableEnd - 1); + }); + + it('does NOT clamp head when it is outside the table (after it)', () => { + // Simulate: anchor in first paragraph, head in paragraph after table + // This selection spans PAST the table — should be allowed + const anchor = beforeParaStart + 3; + const headAfterTable = afterParaStart + 3; + + const clamped = clampHeadAtIsolatingBoundary(doc, anchor, headAfterTable); + + // Head should remain unchanged — it's not inside the table + expect(clamped).toBe(headAfterTable); + }); + + it('does NOT clamp head when it is in the same paragraph as anchor', () => { + // Simulate: both anchor and head in the first paragraph + const anchor = beforeParaStart + 1; + const head = beforeParaStart + 10; + + const clamped = clampHeadAtIsolatingBoundary(doc, anchor, head); + + // Head should remain unchanged + expect(clamped).toBe(head); + }); + + it('TextSelection spanning past table includes content after table', () => { + // Verify that ProseMirror allows a TextSelection from before to after a table + const anchor = beforeParaStart + 3; + const head = afterParaStart + 3; + + const sel = TextSelection.create(doc, anchor, head); + expect(sel.from).toBeLessThan(tableStart); + expect(sel.to).toBeGreaterThan(tableEnd - 1); + }); +});