Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 55 additions & 2 deletions packages/layout-engine/layout-bridge/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,28 @@ const sumLineHeights = (measure: ParagraphMeasure, fromLine: number, toLine: num
* @param geometryHelper - Optional PageGeometryHelper for accurate Y calculations (recommended)
* @returns Array of selection rectangles in container space
*/
/**
* SD-3328: an empty paragraph / blank line that the selection passes through is a
* zero-width slice (`pmStart === pmEnd`). `findLinesIntersectingRange` only yields
* such a line when `from < pos < to`, so it is genuinely spanned and must be
* highlighted. Emit a content-width band so the selection highlight stays
* continuous across the blank line — the same as selecting any text. Without this
* the band shows a gap and the highlight appears to "disappear" while a drag
* crosses a blank line (reported in body paragraphs and inside table cells).
*/
function pushEmptyLineSelectionBand(
rects: Rect[],
opts: { x: number; y: number; width: number; height: number; pageIndex: number },
): void {
rects.push({
x: opts.x,
y: opts.y,
width: Math.max(1, opts.width),
height: opts.height,
pageIndex: opts.pageIndex,
});
}

Comment on lines +585 to +606

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pushEmptyLineSelectionBand got inserted between the selectionToRects jsdoc and its signature, so the @param block is now orphaned from the exported function. moving the helper (and its doc) above the selectionToRects doc block fixes it.

while you're there: the helper only clamps width and pushes, but the emptyLineOffset math — the part most likely to drift — is copy-pasted at both call sites. consider moving that into the helper (pass measure/index/startLine), or just inlining the push.

export function selectionToRects(
layout: Layout,
blocks: FlowBlock[],
Expand Down Expand Up @@ -623,7 +645,19 @@ export function selectionToRects(
if (range.pmStart == null || range.pmEnd == null) return;
const sliceFrom = Math.max(range.pmStart, from);
const sliceTo = Math.min(range.pmEnd, to);
if (sliceFrom >= sliceTo) return;
if (sliceFrom >= sliceTo) {
// SD-3328: blank line spanned by the selection — see pushEmptyLineSelectionBand.
const emptyLineOffset =
lineHeightBeforeIndex(measure, index) - lineHeightBeforeIndex(measure, fragment.fromLine);
pushEmptyLineSelectionBand(rects, {
x: fragment.x,
y: fragment.y + emptyLineOffset + pageTopY,
width: fragment.width,
height: line.lineHeight,
pageIndex,
});
return;
}

// Convert PM positions to character offsets properly
// (accounts for gaps in PM positions between runs)
Expand Down Expand Up @@ -963,7 +997,26 @@ export function selectionToRects(
if (range.pmStart == null || range.pmEnd == null) return;
const sliceFrom = Math.max(range.pmStart, from);
const sliceTo = Math.min(range.pmEnd, to);
if (sliceFrom >= sliceTo) return;
if (sliceFrom >= sliceTo) {
// SD-3328: blank line spanned by the selection — see pushEmptyLineSelectionBand.
const emptyLineOffset =
lineHeightBeforeIndex(info.measure, index) - lineHeightBeforeIndex(info.measure, info.startLine);
pushEmptyLineSelectionBand(rects, {
x: fragment.x + contentOffsetX + cellX + padding.left,
y:
fragment.y +
contentOffsetY +
rowOffset +
blockTopCursor +
effectiveSpacingBeforePx +
emptyLineOffset +
pageTopY,
width: cellMeasure.width - padding.left - padding.right,
height: line.lineHeight,
pageIndex,
});
return;
}

const charOffsetFrom = pmPosToCharOffset(info.block, line, sliceFrom);
const charOffsetTo = pmPosToCharOffset(info.block, line, sliceTo);
Expand Down
13 changes: 10 additions & 3 deletions packages/layout-engine/layout-bridge/src/position-hit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -981,11 +981,18 @@ export function clickToPositionGeometry(
}
}

// Fallback: return first position in the cell
// Fallback: return first position in the cell.
// SD-3328: an EMPTY paragraph (blank line / spacer between bullets) has no runs,
// so `firstRun` is undefined and the old code fell through to `return null`. A null
// hit aborts the drag (EditorInputManager #handleDragSelectionAt early-return) and
// freezes/collapses the in-progress selection while the pointer is over the blank
// line. Derive the paragraph's own PM start from its attrs so an empty cell paragraph
// always resolves to a valid forward position inside the cell, never null.
const firstRun = cellBlock.runs?.[0];
if (firstRun && firstRun.pmStart != null) {
const fallbackPos = firstRun?.pmStart ?? blockPmRangeFromAttrs(cellBlock).pmStart;
if (fallbackPos != null) {
return {
pos: firstRun.pmStart,
pos: fallbackPos,
layoutEpoch,
blockId: tableHit.fragment.blockId,
pageIndex,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type {
Measure,
Line,
ParaFragment,
ParagraphAttrs,
TableBlock,
TableMeasure,
TableFragment,
Expand Down Expand Up @@ -143,6 +144,68 @@ describe('clickToPosition', () => {
expect(result?.column).toBe(1);
});

it('resolves an EMPTY cell paragraph to its PM position instead of null (SD-3328)', () => {
// An empty paragraph (blank line / spacer) inside a table cell has no runs and no
// measured lines, so findLineIndexAtY returns null. Before the fix the cell fallback
// (`cellBlock.runs[0].pmStart`) was undefined and the hit resolved to null — which
// aborts an in-progress drag and collapses the selection. The hit must resolve to the
// empty paragraph's own PM position (from its attrs) so dragging across a blank line
// keeps extending the selection.
const emptyCellPara: FlowBlock = {
kind: 'paragraph',
id: 'empty-cell-para',
runs: [],
attrs: { pmStart: 100, pmEnd: 101 } as unknown as ParagraphAttrs,
};

const tableBlock: TableBlock = {
kind: 'table',
id: 'empty-cell-table',
rows: [{ id: 'row-0', cells: [{ id: 'cell-0-0', blocks: [emptyCellPara] }] }],
};

const emptyCellMeasure: Measure = { kind: 'paragraph', lines: [], totalHeight: 20 };

const tableMeasure: TableMeasure = {
kind: 'table',
rows: [
{
cells: [
{ blocks: [emptyCellMeasure], paragraph: emptyCellMeasure, width: 320, height: 28, gridColumnStart: 0, colSpan: 1, rowSpan: 1 },
],
height: 28,
},
],
columnWidths: [320],
totalWidth: 320,
totalHeight: 28,
};

const tableFragment: TableFragment = {
kind: 'table',
blockId: 'empty-cell-table',
fromRow: 0,
toRow: 1,
x: 30,
y: 40,
width: 320,
height: 28,
pmStart: 100,
pmEnd: 101,
};

const layout: Layout = {
pageSize: { w: 600, h: 800 },
pages: [{ number: 1, margins: { top: 0, right: 0, bottom: 0, left: 0 }, fragments: [tableFragment] }],
};

const result = clickToPosition(layout, [tableBlock], [tableMeasure], { x: 120, y: 54 });

expect(result).not.toBeNull();
expect(result?.pos).toBe(100);
expect(result?.blockId).toBe('empty-cell-table');
});

it('falls back to visual x when a table fragment has no columnIndex', () => {
// Legacy fragments without columnIndex should still resolve a column via fragment.x.
const cellParagraph: FlowBlock = {
Expand Down
95 changes: 95 additions & 0 deletions packages/layout-engine/layout-bridge/test/mock-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,101 @@ export const tableLayout: Layout = {
],
};

// Table cell with an EMPTY paragraph between two text paragraphs (SD-3328).
// PM layout: p1 "Table text" [2,12), empty para inside pos 14, p3 "More text" [16,26).
// A selection from 2..26 passes through all three lines; the empty line is a
// zero-width slice (pmStart === pmEnd === 14) that the rect builder used to skip.
const tableEmptyParaLineP1 = { fromRun: 0, fromChar: 0, toRun: 0, toChar: 10, width: 80, ascent: 10, descent: 4, lineHeight: TABLE_CELL_LINE_HEIGHT } as const;
const tableEmptyParaLineEmpty = { fromRun: 0, fromChar: 0, toRun: 0, toChar: 0, width: 0, ascent: 10, descent: 4, lineHeight: TABLE_CELL_LINE_HEIGHT } as const;
const tableEmptyParaLineP3 = { fromRun: 0, fromChar: 0, toRun: 0, toChar: 9, width: 70, ascent: 10, descent: 4, lineHeight: TABLE_CELL_LINE_HEIGHT } as const;

export const tableEmptyParaBlock: FlowBlock = {
kind: 'table',
id: 'table-empty-para',
rows: [
{
id: 'row-0',
cells: [
{
id: 'cell-0',
attrs: { padding: { top: 2, bottom: 2, left: 4, right: 4 } },
blocks: [
{ kind: 'paragraph', id: 'p1', runs: [{ text: 'Table text', fontFamily: 'Arial', fontSize: 14, pmStart: 2, pmEnd: 12 }] },
{ kind: 'paragraph', id: 'p-empty', runs: [{ text: '', fontFamily: 'Arial', fontSize: 14, pmStart: 14, pmEnd: 14 }] },
{ kind: 'paragraph', id: 'p3', runs: [{ text: 'More text', fontFamily: 'Arial', fontSize: 14, pmStart: 16, pmEnd: 26 }] },
],
},
],
},
],
};

export const tableEmptyParaMeasure: Measure = {
kind: 'table',
rows: [
{
height: TABLE_CELL_LINE_HEIGHT * 3 + 4,
cells: [
{
width: 120,
height: TABLE_CELL_LINE_HEIGHT * 3 + 4,
gridColumnStart: 0,
blocks: [
{ kind: 'paragraph', lines: [tableEmptyParaLineP1], totalHeight: TABLE_CELL_LINE_HEIGHT },
{ kind: 'paragraph', lines: [tableEmptyParaLineEmpty], totalHeight: TABLE_CELL_LINE_HEIGHT },
{ kind: 'paragraph', lines: [tableEmptyParaLineP3], totalHeight: TABLE_CELL_LINE_HEIGHT },
],
},
],
},
],
columnWidths: [120],
totalWidth: 120,
totalHeight: TABLE_CELL_LINE_HEIGHT * 3 + 4,
};

export const tableEmptyParaLayout: Layout = {
pageSize: { w: 400, h: 500 },
pages: [
{
number: 1,
fragments: [
{ kind: 'table' as const, blockId: 'table-empty-para', fromRow: 0, toRow: 1, x: 30, y: 60, width: 120, height: TABLE_CELL_LINE_HEIGHT * 3 + 4 },
],
},
],
};

// Body paragraphs with an EMPTY paragraph between two text paragraphs (SD-3328).
// p1 "First line" [1,11), empty paragraph inside pos 13, p3 "Third line" [15,25).
// A selection 1..25 passes through all three; the empty line is a zero-width slice
// that the body rect builder used to skip, leaving a gap in the highlight band.
export const bodyEmptyParaBlocks: FlowBlock[] = [
{ kind: 'paragraph', id: 'body-p1', runs: [{ text: 'First line', fontFamily: 'Arial', fontSize: 16, pmStart: 1, pmEnd: 11 }] },
{ kind: 'paragraph', id: 'body-empty', runs: [{ text: '', fontFamily: 'Arial', fontSize: 16, pmStart: 13, pmEnd: 13 }] },
{ kind: 'paragraph', id: 'body-p3', runs: [{ text: 'Third line', fontFamily: 'Arial', fontSize: 16, pmStart: 15, pmEnd: 25 }] },
];

export const bodyEmptyParaMeasures: Measure[] = [
{ kind: 'paragraph', lines: [{ fromRun: 0, fromChar: 0, toRun: 0, toChar: 10, width: 80, ascent: 12, descent: 4, lineHeight: 20 }], totalHeight: 20 },
{ kind: 'paragraph', lines: [{ fromRun: 0, fromChar: 0, toRun: 0, toChar: 0, width: 0, ascent: 12, descent: 4, lineHeight: 20 }], totalHeight: 20 },
{ kind: 'paragraph', lines: [{ fromRun: 0, fromChar: 0, toRun: 0, toChar: 10, width: 80, ascent: 12, descent: 4, lineHeight: 20 }], totalHeight: 20 },
];

export const bodyEmptyParaLayout: Layout = {
pageSize: { w: 400, h: 500 },
pages: [
{
number: 1,
fragments: [
{ kind: 'para', blockId: 'body-p1', fromLine: 0, toLine: 1, x: 30, y: 40, width: 300, pmStart: 1, pmEnd: 11 },
{ kind: 'para', blockId: 'body-empty', fromLine: 0, toLine: 1, x: 30, y: 60, width: 300, pmStart: 13, pmEnd: 13 },
{ kind: 'para', blockId: 'body-p3', fromLine: 0, toLine: 1, x: 30, y: 80, width: 300, pmStart: 15, pmEnd: 25 },
],
},
],
};

// Table cell spacing.before — selectionToRects tests (effective spacing, absorption, partial row)
export const TABLE_SPACING_BEFORE = 12;
export const TABLE_SPACING_FRAGMENT_Y = 50;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ import {
tableLayout,
tableBlock,
tableMeasure,
tableEmptyParaLayout,
tableEmptyParaBlock,
tableEmptyParaMeasure,
bodyEmptyParaLayout,
bodyEmptyParaBlocks,
bodyEmptyParaMeasures,
tableSpacingBeforeBlock,
tableSpacingBeforeMeasure,
tableSpacingBeforeLayout,
Expand Down Expand Up @@ -74,6 +80,37 @@ describe('selectionToRects', () => {
expect(rects[0].x).toBeGreaterThan(tableLayout.pages[0].fragments[0].x);
});

it('highlights an empty paragraph row spanned by a table-cell selection (SD-3328)', () => {
// Selection 2..26 passes through p1 -> empty paragraph -> p3. The empty line is a
// zero-width slice; before the fix the builder skipped it, leaving a gap in the
// highlight band (the reported "selection highlight disappears over empty space").
const rects = selectionToRects(tableEmptyParaLayout, [tableEmptyParaBlock], [tableEmptyParaMeasure], 2, 26);

// One rect per line: text, empty, text — the empty row must NOT be dropped.
expect(rects).toHaveLength(3);

// The middle (empty) row sits vertically between the two text rows and is visible.
const sorted = [...rects].sort((a, b) => a.y - b.y);
const emptyRect = sorted[1];
expect(emptyRect.y).toBeGreaterThan(sorted[0].y);
expect(emptyRect.y).toBeLessThan(sorted[2].y);
expect(emptyRect.width).toBeGreaterThan(1);
});

it('highlights a blank line between body paragraphs spanned by a selection (SD-3328)', () => {
// Selection 1..25 passes through p1 -> empty paragraph -> p3 in body text.
// The blank line must be highlighted just like inside a table cell.
const rects = selectionToRects(bodyEmptyParaLayout, bodyEmptyParaBlocks, bodyEmptyParaMeasures, 1, 25);

expect(rects).toHaveLength(3);

const sorted = [...rects].sort((a, b) => a.y - b.y);
const emptyRect = sorted[1];
expect(emptyRect.y).toBeGreaterThan(sorted[0].y);
expect(emptyRect.y).toBeLessThan(sorted[2].y);
expect(emptyRect.width).toBeGreaterThan(1);
});

it('accounts for visual-only prefix runs when mapping PM selections to X coordinates', () => {
const blockWithoutMarker: FlowBlock = {
kind: 'paragraph',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,13 @@ import {
getFirstTextPosition as getFirstTextPositionFromHelper,
registerPointerClick as registerPointerClickFromHelper,
} from '../input/ClickSelectionUtilities.js';
import { calculateExtendedSelection } from '../selection/SelectionHelpers.js';
import { calculateExtendedSelection, selectionCollapsesAcrossTableCells } from '../selection/SelectionHelpers.js';
import {
shouldUseCellSelection as shouldUseCellSelectionFromHelper,
getCellPosFromTableHit as getCellPosFromTableHitFromHelper,
getTablePosFromHit as getTablePosFromHitFromHelper,
hitTestTable as hitTestTableFromHelper,
resolveCrossCellSelection,
} from '../tables/TableSelectionUtilities.js';
import { debugLog } from '../selection/SelectionDebug.js';
import { DOM_CLASS_NAMES, buildAnnotationSelector, DRAGGABLE_SELECTOR } from '@superdoc/dom-contract';
Expand Down Expand Up @@ -2592,12 +2593,44 @@ export class EditorInputManager {
return;
}

// Text selection mode
const anchor = this.#dragAnchor!;
const head = hit.pos;

// SD-3328: Cross-cell selection from the resolved PM positions. The geometry trigger above
// (#hitTestTable) can miss empty cell paragraphs and similar spots, leaving #cellAnchor unset,
// so a drag across cells falls to the text path — where prosemirror-tables collapses it
// (forward) or the guard freezes it (backward). Deriving the cells from the resolved positions
// is reliable: when the anchor and head land in different cells of the same table, select the
// cell range directly, the way Word and Google Docs do, regardless of the flaky geometry hit.
if (!useActiveSurfaceHitTest) {
const crossCell = resolveCrossCellSelection(editor.state.doc, anchor, head);
if (crossCell) {
try {
const tr = editor.state.tr.setSelection(
CellSelection.create(editor.state.doc, crossCell.anchorCellPos, crossCell.headCellPos),
);
editor.view?.dispatch(tr);
this.#callbacks.scheduleSelectionUpdate?.();
return;
} catch (error) {
// Fall through to text selection if the cell range cannot be built.
console.warn('[SELECTION] Failed to create cross-cell CellSelection during drag:', error);
}
}
}

// Text selection mode
const { selAnchor, selHead } = this.#calculateExtendedSelection(anchor, head, this.#dragExtensionMode);

// SD-3328: When dragging a body selection into (or through) a table, prosemirror-tables'
// normalization collapses a TextSelection whose head lands at the start of a cell block
// (an empty cell paragraph is always at parentOffset 0), rewriting e.g. [44, 2026] to
// [44, 49]. Detect that frame and keep the last good selection instead of dispatching a
// doomed one — the selection resumes extending as soon as the head moves into cell text.
if (selectionCollapsesAcrossTableCells(editor.state.doc, selAnchor, selHead)) {
return;
}

try {
const tr = editor.state.tr.setSelection(TextSelection.create(editor.state.doc, selAnchor, selHead));
editor.view?.dispatch(tr);
Expand Down
Loading
Loading