Skip to content
Open
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: 57 additions & 0 deletions packages/layout-engine/layout-bridge/src/remeasure.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1781,6 +1781,55 @@ export function remeasureParagraph(
}
let lastMeasuredFontSize = firstRunFontSize ?? 16;

/**
* Word never wraps a line at a trailing space that only precedes an explicit
* break (<w:br/>) or the paragraph end: such spaces are not measured for line
* fitting and hang past the text margin instead of opening a new (empty) line.
* Mirrors spacesHangBeforeBreak in measuring/dom (issue #3946).
*
* Only true line breaks qualify here (unlike the primary measurer's mirror):
* remeasurement treats page/column 'break' runs as zero-width passthroughs
* that do not close the line, so a space before one is mid-line and must keep
* counting toward the fit. Computed lazily in one backward pass —
* suffixHangs[i] answers "do runs i.. contribute no non-space content before
* a line break or the paragraph end?", and tailAllSpaceFrom[i] is the first
* index after run i's last non-space character — so repeated per-character
* lookups stay O(1) regardless of how many trailing-space runs there are.
*/
let trailingSpacesHangCache: { suffixHangs: boolean[]; tailAllSpaceFrom: number[] } | null = null;
const computeTrailingSpacesHangCache = (): { suffixHangs: boolean[]; tailAllSpaceFrom: number[] } => {
const suffixHangs: boolean[] = new Array(runs.length + 1);
const tailAllSpaceFrom: number[] = new Array(runs.length);
suffixHangs[runs.length] = true;
for (let i = runs.length - 1; i >= 0; i -= 1) {
const run = runs[i];
if (isTextRun(run)) {
let lastNonSpace = run.text.length - 1;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: When a paragraph contains a MathRun and an overflowing space triggers trailingSpacesHang, computeTrailingSpacesHangCache throws because MathRun has no text property. Exclude MathRun from isTextRun or from this cache before reading run.text; otherwise incremental remeasurement fails for affected paragraphs.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/layout-engine/layout-bridge/src/remeasure.ts, line 1807:

<comment>When a paragraph contains a MathRun and an overflowing space triggers `trailingSpacesHang`, `computeTrailingSpacesHangCache` throws because MathRun has no `text` property. Exclude MathRun from `isTextRun` or from this cache before reading `run.text`; otherwise incremental remeasurement fails for affected paragraphs.</comment>

<file context>
@@ -1781,6 +1781,55 @@ export function remeasureParagraph(
+    for (let i = runs.length - 1; i >= 0; i -= 1) {
+      const run = runs[i];
+      if (isTextRun(run)) {
+        let lastNonSpace = run.text.length - 1;
+        while (lastNonSpace >= 0 && run.text[lastNonSpace] === ' ') lastNonSpace -= 1;
+        tailAllSpaceFrom[i] = lastNonSpace + 1;
</file context>

while (lastNonSpace >= 0 && run.text[lastNonSpace] === ' ') lastNonSpace -= 1;
tailAllSpaceFrom[i] = lastNonSpace + 1;
} else {
tailAllSpaceFrom[i] = 0;
}
if (isLineBreakRun(run)) {
suffixHangs[i] = true;
continue;
}
if (isVanishedRun(run)) {
suffixHangs[i] = suffixHangs[i + 1];
continue;
}
suffixHangs[i] = isTextRun(run) && tailAllSpaceFrom[i] === 0 ? suffixHangs[i + 1] : false;
}
return { suffixHangs, tailAllSpaceFrom };
};
const trailingSpacesHang = (runIdx: number, fromChar: number): boolean => {
trailingSpacesHangCache ??= computeTrailingSpacesHangCache();
const { suffixHangs, tailAllSpaceFrom } = trailingSpacesHangCache;
const current = runs[runIdx];
if (isTextRun(current) && fromChar < tailAllSpaceFrom[runIdx]) return false;
return suffixHangs[runIdx + 1];
};

while (currentRun < runs.length) {
const isFirstLine = lines.length === 0;
// For first line, reduce available width by textStart/first-line offset (e.g., for in-flow list markers)
Expand Down Expand Up @@ -2030,6 +2079,14 @@ export function remeasureParagraph(
: effectiveMaxWidth - WIDTH_FUDGE_PX;
if (width + w > fitThreshold && width > 0) {
if (ch === ' ') {
// A trailing space right before an explicit break (or the paragraph
// end) never forces a wrap: consume it at zero charged width and keep
// scanning so the break closes this line instead of an empty one.
if (trailingSpacesHang(r, chEnd)) {
endRun = r;
endChar = chEnd;
continue;
}
// The space is only a wrap delimiter. Consume it so the next line
// starts at the following word, but do not charge its width to the
// completed line.
Expand Down
21 changes: 21 additions & 0 deletions packages/layout-engine/layout-bridge/test/remeasure.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1392,6 +1392,27 @@ describe('remeasureParagraph', () => {
expect(measure.lines[2].toRun).toBe(4);
});

it('does not create a spurious empty line when a trailing space overflows before a lineBreak (#3946)', () => {
// 'Helloworld' = 100px at CHAR_WIDTH 10; with the trailing space it is
// 110px. At maxWidth 105 the space overflows the measure, but because it
// only precedes an explicit break it must hang instead of wrapping onto
// its own (visually empty) line — Word renders two lines here.
const block = createBlock([textRun('Helloworld '), { kind: 'lineBreak' } as Run, textRun('Next')]);
const measure = remeasureParagraph(block, 10 * CHAR_WIDTH + 5);

expect(measure.lines).toHaveLength(2);
expect(measure.lines[0].fromRun).toBe(0);
expect(measure.lines[1].fromRun).toBe(2);
});

it('does not wrap a whole-run trailing space that overflows before a lineBreak (#3946)', () => {
const block = createBlock([textRun('Helloworld'), textRun(' '), { kind: 'lineBreak' } as Run, textRun('Next')]);
const measure = remeasureParagraph(block, 10 * CHAR_WIDTH + 5);

expect(measure.lines).toHaveLength(2);
expect(measure.lines[1].fromRun).toBe(3);
});

it('preserves trailing explicit lineBreak as final empty line', () => {
const block = createBlock([textRun('Hello'), { kind: 'lineBreak' } as Run]);
const measure = remeasureParagraph(block, 200);
Expand Down
86 changes: 86 additions & 0 deletions packages/layout-engine/measuring/dom/src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5125,6 +5125,92 @@ describe('measureBlock', () => {
expect(measure.lines[0].toChar).toBe(5);
});

it('does not create a spurious empty line when a trailing space overflows before a hard break (#3946)', async () => {
// Measure the text without the trailing space to find its natural width.
const probe: FlowBlock = {
kind: 'paragraph',
id: 'probe-paragraph',
runs: [{ text: 'Lorem ipsum dolor', fontFamily: 'Arial', fontSize: 16 }],
attrs: {},
};
const probeMeasure = expectParagraphMeasure(await measureBlock(probe, 1000));
const textWidth = probeMeasure.lines[0].width;

// Same text + trailing space + explicit line break + more text, with
// maxWidth chosen so the text fits but text+space slightly exceeds it.
// Word renders TWO lines here: the trailing space hangs past the measure
// instead of wrapping onto its own (visually empty) line.
const block: FlowBlock = {
kind: 'paragraph',
id: 'bug-paragraph',
runs: [
{ text: 'Lorem ipsum dolor ', fontFamily: 'Arial', fontSize: 16 },
{ kind: 'lineBreak' },
{ text: 'Second line', fontFamily: 'Arial', fontSize: 16 },
],
attrs: { alignment: 'justify' },
};
const measure = expectParagraphMeasure(await measureBlock(block, textWidth + 2));

expect(measure.lines.length).toBe(2);
expect(measure.lines[1].fromRun).toBe(2);
});

it('hangs a trailing space before a page/column break run too (#3946)', async () => {
// The run loop closes the current line for every 'break' kind, so a
// space that only precedes a page/column break is line-trailing as well.
const probe: FlowBlock = {
kind: 'paragraph',
id: 'probe3-paragraph',
runs: [{ text: 'Lorem ipsum dolor', fontFamily: 'Arial', fontSize: 16 }],
attrs: {},
};
const probeMeasure = expectParagraphMeasure(await measureBlock(probe, 1000));
const textWidth = probeMeasure.lines[0].width;

const block: FlowBlock = {
kind: 'paragraph',
id: 'bug3-paragraph',
runs: [
{ text: 'Lorem ipsum dolor ', fontFamily: 'Arial', fontSize: 16 },
{ kind: 'break' },
{ text: 'Second line', fontFamily: 'Arial', fontSize: 16 },
],
attrs: {},
};
const measure = expectParagraphMeasure(await measureBlock(block, textWidth + 2));

expect(measure.lines.length).toBe(2);
expect(measure.lines[1].fromRun).toBe(2);
});

it('does not wrap a whole-run trailing space that overflows before a hard break (#3946)', async () => {
const probe: FlowBlock = {
kind: 'paragraph',
id: 'probe2-paragraph',
runs: [{ text: 'Lorem ipsum dolor', fontFamily: 'Arial', fontSize: 16 }],
attrs: {},
};
const probeMeasure = expectParagraphMeasure(await measureBlock(probe, 1000));
const textWidth = probeMeasure.lines[0].width;

const block: FlowBlock = {
kind: 'paragraph',
id: 'bug2-paragraph',
runs: [
{ text: 'Lorem ipsum dolor', fontFamily: 'Arial', fontSize: 16 },
{ text: ' ', fontFamily: 'Arial', fontSize: 16 },
{ kind: 'lineBreak' },
{ text: 'Second line', fontFamily: 'Arial', fontSize: 16 },
],
attrs: { alignment: 'justify' },
};
const measure = expectParagraphMeasure(await measureBlock(block, textWidth + 2));

expect(measure.lines.length).toBe(2);
expect(measure.lines[1].fromRun).toBe(3);
});

it('includes space width when mid-line', async () => {
const blockNoSpace: FlowBlock = {
kind: 'paragraph',
Expand Down
47 changes: 45 additions & 2 deletions packages/layout-engine/measuring/dom/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2760,6 +2760,47 @@ async function measureParagraphBlock(
}
};

/**
* Word never wraps a line at a trailing space that is immediately followed by an
* explicit break (<w:br/>) or the end of the paragraph: trailing spaces are not
* measured for line fitting and simply hang past the text margin. Without this,
* a space that overflows the measure right before a hard break wraps onto its
* own line, and the break then closes that (visually empty) line — producing a
* spurious blank line (three lines instead of two). Returns true when every run
* after `startRunIndex` up to the next explicit break (or the paragraph end)
* contributes no non-space content, i.e. the current space(s) are line-trailing.
*/
// Any 'break' kind counts, not just breakType 'line': the run loop below
// closes the current line for every break run, so a space that only precedes
// a page/column break is line-trailing here too. Computed lazily in one
// backward pass (suffixHangs[i] answers "do runs i.. contribute no non-space
// content before a line-closing break or the paragraph end?"), so repeated
// lookups stay O(1) even with many trailing-space runs.
let spacesHangCache: boolean[] | null = null;
const computeSpacesHangCache = (): boolean[] => {
const suffixHangs: boolean[] = new Array(runsToProcess.length + 1);
suffixHangs[runsToProcess.length] = true;
for (let i = runsToProcess.length - 1; i >= 0; i--) {
const run = runsToProcess[i] as Run;
if (run.kind === 'lineBreak' || run.kind === 'break') {
suffixHangs[i] = true;
continue;
}
if (isVanishedRun(run)) {
suffixHangs[i] = suffixHangs[i + 1];
continue;
}
const isPlainTextRun = !run.kind || run.kind === 'text';
const text = (run as TextRun).text;
suffixHangs[i] = isPlainTextRun && typeof text === 'string' && /^[ ]*$/.test(text) ? suffixHangs[i + 1] : false;
}
return suffixHangs;
};
const spacesHangBeforeBreak = (startRunIndex: number): boolean => {
spacesHangCache ??= computeSpacesHangCache();
return spacesHangCache[startRunIndex + 1];
};

// Per-line-segment tab counts. The heuristic below binds the last N tabs of a
// segment to the last N alignment stops; segments are delimited by explicit
// <w:br/> runs because pPr/tabs apply per line, not per paragraph.
Expand Down Expand Up @@ -3613,7 +3654,8 @@ async function measureParagraphBlock(
const boundarySpacing = resolveBoundarySpacing(currentLine.width, isRunStart, run as TextRun);
if (
currentLine.width + boundarySpacing + spacesWidth > currentLine.maxWidth - WIDTH_FUDGE_PX &&
currentLine.width > 0
currentLine.width > 0 &&
!(isLastSegment && spacesHangBeforeBreak(runIndex))
) {
trimTrailingWrapSpaces(currentLine);
const completedLine: Line = closeLineWithMetrics(currentLine);
Expand Down Expand Up @@ -3733,7 +3775,8 @@ async function measureParagraphBlock(
const boundarySpacing = resolveBoundarySpacing(currentLine.width, isRunStart, run as TextRun);
if (
currentLine.width + boundarySpacing + singleSpaceWidth > currentLine.maxWidth - WIDTH_FUDGE_PX &&
currentLine.width > 0
currentLine.width > 0 &&
!(isLastSegment && wordIndex > lastNonEmptyWordIndex && spacesHangBeforeBreak(runIndex))
) {
// Space doesn't fit - finish current line and start new one with the space
trimTrailingWrapSpaces(currentLine);
Expand Down
Loading