Skip to content

Commit f778c4a

Browse files
chittolinagchittolinacaio-pizzol
authored
SD-2447 - fix: tab stop computation and alignment for TOC styles (#2731)
* fix: toc not loading on paragraphs * fix: tab stop computation and alignment for TOC styles * fix: tests * fix: don't decode every child twice Co-authored-by: Caio Pizzol <97641911+caio-pizzol@users.noreply.github.com> * fix: build * refactor: removed unused code * chore: small code tweaks * chore: small code tweaks * chore: removed unused code * fix(layout-engine): tab-alignment parity, TOC schema invariant, regression tests Follow-ups from PR review: - measuring/dom: sync sequentialTabIndex with explicit tabIndex values (mirror remeasure.ts). - Document getAlignmentStopForOrdinal as a Word-compat heuristic, not ECMA 17.3.3.32 behavior. - tableOfContents encoder: normalize every non-paragraph child into its own paragraph so the schema invariant (paragraph*) holds for mixed inline + paragraph input, not just the all-inline case. - Add regression tests: 3-tabs + 2-alignment-stops asymmetric case, mixed start+end stops preserving legacy defaults-after-rightmost behavior, tightened TOC leader assertion. * test(behavior): add TOC tab-alignment regression for SD-2447 Playwright spec that loads a TOC1 fixture and asserts leader endpoints align, page numbers right-justify, and leader start positions vary with title length (proving the first tab falls on the default grid, not the right stop). Fixture is a Word-native 14KB DOCX with a right-aligned dot-leader tab stop on TOC1 style — the same paragraph shape as the reported Keyper document. --------- Co-authored-by: Gabriel Chittolina <gabrielchittolina1@gmail.com> Co-authored-by: Caio Pizzol <97641911+caio-pizzol@users.noreply.github.com> Co-authored-by: Caio Pizzol <caio@harbourshare.com>
1 parent ff220a9 commit f778c4a

11 files changed

Lines changed: 381 additions & 51 deletions

File tree

packages/layout-engine/contracts/src/engines/tabs.test.ts

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -161,11 +161,49 @@ describe('engines-tabs computeTabStops', () => {
161161
expect(stops.find((stop) => stop.pos === 340)).toBeDefined();
162162
// Explicit stop at 709 should be preserved
163163
expect(stops.find((stop) => stop.pos === 709)).toBeDefined();
164-
// First default should be at 709 + 720 = 1429
164+
// First default should align with Word's 0.5" grid offset from leftIndent (709 + 720 = 1429).
165165
expect(stops.find((stop) => stop.pos === 1429)).toBeDefined();
166-
// No default at 720 (before leftIndent, and no explicit stop there)
166+
// No duplicate default at 720 because explicit stop at 709 occupies that slot
167167
expect(stops.filter((stop) => stop.pos === 720).length).toBe(0);
168168
});
169+
170+
it('still generates default start tabs before explicit right tabs (TOC regression)', () => {
171+
const stops = computeTabStops({
172+
explicitStops: [{ val: 'end', pos: 10593, leader: 'dot' }], // TOC1 style tab
173+
defaultTabInterval: 720,
174+
paragraphIndent: { left: 454, hanging: 454 }, // first line begins near 0"
175+
});
176+
177+
const firstDefault = stops.find((stop) => stop.val === 'start' && stop.leader === 'none');
178+
expect(firstDefault).toBeDefined();
179+
expect(firstDefault?.pos).toBe(720); // Word default 0.5" tab stop
180+
expect(firstDefault!.pos).toBeLessThan(10593);
181+
expect(stops.find((stop) => stop.val === 'end' && stop.pos === 10593)).toBeDefined();
182+
});
183+
184+
it('preserves legacy defaults-after-rightmost behavior when a start stop is present', () => {
185+
// Paragraphs with a start-aligned explicit stop (e.g. signature lines, invoice
186+
// headers) must keep the pre-fix behavior: defaults begin after the rightmost
187+
// explicit stop, not from zero. Regression guard for the hasStartAlignedExplicit
188+
// branch added alongside the TOC fix.
189+
const explicitStops = [
190+
{ val: 'start' as const, pos: 500, leader: 'none' as const },
191+
{ val: 'end' as const, pos: 5000, leader: 'dot' as const },
192+
];
193+
const stops = computeTabStops({
194+
explicitStops,
195+
defaultTabInterval: 720,
196+
paragraphIndent: { left: 0 },
197+
});
198+
199+
const explicitPositions = new Set(explicitStops.map((s) => s.pos));
200+
// No *default* (non-explicit) stop should appear between 0 and the rightmost
201+
// explicit stop (5000). Explicit stops themselves are allowed.
202+
const generatedBelowEnd = stops.filter((stop) => stop.pos < 5000 && !explicitPositions.has(stop.pos));
203+
expect(generatedBelowEnd).toHaveLength(0);
204+
// Defaults should resume at 5720 (5000 + 720 interval).
205+
expect(stops.find((stop) => stop.val === 'start' && stop.pos === 5720)).toBeDefined();
206+
});
169207
});
170208

171209
describe('engines-tabs layoutWithTabs', () => {

packages/layout-engine/contracts/src/engines/tabs.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -129,18 +129,18 @@ export function computeTabStops(context: TabContext): TabStop[] {
129129

130130
// Find the rightmost explicit stop (use original stops for this calculation)
131131
const maxExplicit = filteredExplicitStops.reduce((max, stop) => Math.max(max, stop.pos), 0);
132-
const hasExplicit = filteredExplicitStops.length > 0;
133-
134132
// Collect all stops: start with filtered explicit stops
135133
const stops = [...filteredExplicitStops];
134+
const hasStartAlignedExplicit = filteredExplicitStops.some((stop) => stop.val === 'start');
136135

137136
// Generate default stops at regular intervals.
138-
// When explicit stops exist, start after the rightmost explicit or leftIndent.
139-
// When no explicit stops, generate from 0 to ensure we hit multiples that land at/near leftIndent.
140-
// Then filter defaults by leftIndent (body text alignment).
141-
const defaultStart = hasExplicit ? Math.max(maxExplicit, leftIndent) : 0;
137+
// - When no explicit start tabs exist (e.g., TOC paragraphs with only right-aligned tabs),
138+
// seed defaults from the origin so numbering/content still lands on the default grid.
139+
// - Otherwise, preserve legacy behavior: defaults start after the rightmost explicit or left indent.
140+
const seedDefaultsFromZero = !hasStartAlignedExplicit;
141+
const defaultStart = seedDefaultsFromZero ? 0 : Math.max(maxExplicit, leftIndent);
142142
let pos = defaultStart;
143-
const targetLimit = Math.max(defaultStart, leftIndent) + 14400; // 14400 twips = 10 inches
143+
const targetLimit = Math.max(defaultStart, leftIndent, maxExplicit) + 14400; // 14400 twips = 10 inches
144144

145145
while (pos < targetLimit) {
146146
pos += defaultTabInterval;

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

Lines changed: 48 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import type {
55
LineSegment,
66
Run,
77
TextRun,
8+
TabRun,
89
TabStop,
910
ParagraphIndent,
1011
LeaderDecoration,
@@ -781,6 +782,33 @@ const applyTabLayoutToLines = (
781782
indentLeft: number,
782783
rawFirstLineOffset: number,
783784
): void => {
785+
const totalTabRuns = runs.reduce((count, run) => (run.kind === 'tab' ? count + 1 : count), 0);
786+
const alignmentTabStopsPx = tabStops
787+
.map((stop, index) => ({ stop, index }))
788+
.filter(({ stop }) => stop.val === 'end' || stop.val === 'center' || stop.val === 'decimal');
789+
// Word-compat heuristic (not ECMA-376 17.3.3.32): the last N tab characters in a
790+
// paragraph bind to the last N explicit end/center/decimal stops. Needed for TOC
791+
// entries where a right-aligned dot-leader stop coexists with default grid stops.
792+
// Mirrored in measuring/dom/src/index.ts.
793+
const getAlignmentStopForOrdinal = (ordinal: number): { stop: TabStopPx; index: number } | null => {
794+
if (alignmentTabStopsPx.length === 0 || totalTabRuns === 0 || !Number.isFinite(ordinal)) return null;
795+
if (ordinal < 0 || ordinal >= totalTabRuns) return null;
796+
const remainingTabs = totalTabRuns - ordinal - 1;
797+
const targetIndex = alignmentTabStopsPx.length - 1 - remainingTabs;
798+
if (targetIndex < 0 || targetIndex >= alignmentTabStopsPx.length) return null;
799+
return alignmentTabStopsPx[targetIndex];
800+
};
801+
let sequentialTabIndex = 0;
802+
const consumeTabOrdinal = (explicitIndex?: number): number => {
803+
if (typeof explicitIndex === 'number' && Number.isFinite(explicitIndex)) {
804+
sequentialTabIndex = Math.max(sequentialTabIndex, explicitIndex + 1);
805+
return explicitIndex;
806+
}
807+
const ordinal = sequentialTabIndex;
808+
sequentialTabIndex += 1;
809+
return ordinal;
810+
};
811+
784812
lines.forEach((line, lineIndex) => {
785813
let cursorX = 0;
786814
let lineWidth = 0;
@@ -797,11 +825,23 @@ const applyTabLayoutToLines = (
797825
/**
798826
* Processes a tab character, calculating position and handling alignment.
799827
*/
800-
const applyTab = (startRunIndex: number, startChar: number, run?: Run): void => {
828+
const applyTab = (startRunIndex: number, startChar: number, run?: Run, tabOrdinal?: number): void => {
801829
const originX = cursorX;
802830
const absCurrentX = cursorX + effectiveIndent;
803-
const { target, nextIndex, stop } = getNextTabStopPx(absCurrentX, tabStops, tabStopCursor);
804-
tabStopCursor = nextIndex;
831+
let stop: TabStopPx | undefined;
832+
let target: number;
833+
const forcedAlignment =
834+
typeof tabOrdinal === 'number' && Number.isFinite(tabOrdinal) ? getAlignmentStopForOrdinal(tabOrdinal) : null;
835+
if (forcedAlignment && forcedAlignment.stop.pos > absCurrentX + TAB_EPSILON) {
836+
stop = forcedAlignment.stop;
837+
target = forcedAlignment.stop.pos;
838+
tabStopCursor = forcedAlignment.index + 1;
839+
} else {
840+
const next = getNextTabStopPx(absCurrentX, tabStops, tabStopCursor);
841+
stop = next.stop;
842+
target = next.target;
843+
tabStopCursor = next.nextIndex;
844+
}
805845
const clampedTarget = Number.isFinite(maxAbsWidth) ? Math.min(target, maxAbsWidth) : target;
806846
const relativeTarget = clampedTarget - effectiveIndent;
807847
lineWidth = Math.max(lineWidth, relativeTarget);
@@ -853,7 +893,9 @@ const applyTabLayoutToLines = (
853893
const run = runs[runIndex];
854894
if (!run) continue;
855895
if (run.kind === 'tab') {
856-
applyTab(runIndex + 1, 0, run);
896+
const tabRun = run as TabRun;
897+
const ordinal = consumeTabOrdinal(tabRun.tabIndex);
898+
applyTab(runIndex + 1, 0, run, ordinal);
857899
continue;
858900
}
859901

@@ -889,7 +931,8 @@ const applyTabLayoutToLines = (
889931
lineWidth = Math.max(lineWidth, cursorX);
890932
segments.push(segment);
891933
}
892-
applyTab(runIndex, i + 1);
934+
const ordinal = consumeTabOrdinal();
935+
applyTab(runIndex, i + 1, undefined, ordinal);
893936
segmentStart = i + 1;
894937
}
895938

packages/layout-engine/layout-bridge/test/remeasure.test.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,27 @@ describe('remeasureParagraph', () => {
445445
expect(measure.lines[0].segments?.length).toBeGreaterThan(0);
446446
});
447447

448+
it('aligns trailing TOC-style tab to explicit right stop with leader', () => {
449+
const rightStopPx = 300;
450+
const block = createBlock(
451+
[textRun('1.'), tabRun({ tabIndex: 0 }), textRun('Generalities'), tabRun({ tabIndex: 1 }), textRun('5')],
452+
{
453+
tabs: [{ pos: pxToTwips(rightStopPx), val: 'end', leader: 'dot' }],
454+
indent: { left: 30, hanging: 30 },
455+
tabIntervalTwips: DEFAULT_TAB_INTERVAL_TWIPS,
456+
},
457+
);
458+
459+
const measure = remeasureParagraph(block, 800);
460+
expect(measure.lines).toHaveLength(1);
461+
const leaders = measure.lines[0].leaders;
462+
expect(leaders).toBeDefined();
463+
expect(leaders?.length).toBe(1);
464+
const leader = leaders![0];
465+
expect(leader.style).toBe('dot');
466+
expect(leader.to).toBeCloseTo(rightStopPx - CHAR_WIDTH, 0);
467+
});
468+
448469
it('handles tab at various positions within text', () => {
449470
// Tab after some text should advance to next stop after current position
450471
const tabStop: TabStop = { pos: 720, val: 'start' }; // 48px
@@ -481,7 +502,6 @@ describe('remeasureParagraph', () => {
481502
const tabStop: TabStop = { pos: 1440, val: 'start' };
482503
const block = createBlock([textRun('A'), tabRun(), textRun('B')], { tabs: [tabStop] });
483504
const measure = remeasureParagraph(block, 200);
484-
485505
expect(measure.lines).toHaveLength(1);
486506
// Tab should advance to 96px (1 inch)
487507
expect(measure.lines[0].width).toBeGreaterThan(96);

packages/layout-engine/measuring/dom/src/index.test.ts

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1619,6 +1619,77 @@ describe('measureBlock', () => {
16191619
}
16201620
});
16211621

1622+
it('aligns trailing tabs to explicit right stops with dot leaders (TOC regression)', async () => {
1623+
const rightStopTwips = 10593;
1624+
const rightStopPx = rightStopTwips * (96 / 1440); // ~706px
1625+
const block: FlowBlock = {
1626+
kind: 'paragraph',
1627+
id: 'toc-paragraph',
1628+
runs: [
1629+
{ text: '1.', fontFamily: 'Arial', fontSize: 13.333 },
1630+
{ kind: 'tab', text: '\t', tabIndex: 0, pmStart: 2, pmEnd: 3 },
1631+
{ text: 'Generalities', fontFamily: 'Arial', fontSize: 13.333 },
1632+
{ kind: 'tab', text: '\t', tabIndex: 1, pmStart: 15, pmEnd: 16 },
1633+
{ text: '5', fontFamily: 'Arial', fontSize: 13.333 },
1634+
],
1635+
attrs: {
1636+
indent: { left: 30, right: 0, firstLine: 0, hanging: 30 },
1637+
tabs: [{ val: 'end', leader: 'dot', pos: rightStopTwips }],
1638+
},
1639+
};
1640+
1641+
const measure = expectParagraphMeasure(await measureBlock(block, 800));
1642+
expect(measure.lines).toHaveLength(1);
1643+
const line = measure.lines[0];
1644+
expect(line.leaders).toBeDefined();
1645+
expect(line.leaders?.[0]?.style).toBe('dot');
1646+
// Leader must end right before the page number — within ~20px of the right stop
1647+
// (page number "5" is a few px wide, not 100+ px wide).
1648+
expect(line.leaders?.[0]?.to).toBeLessThanOrEqual(rightStopPx);
1649+
expect(line.leaders?.[0]?.to).toBeGreaterThan(rightStopPx - 20);
1650+
// Leader must start AFTER the title text, not at "1." — proves the first tab
1651+
// fell on the default 0.5" grid, not on the end stop.
1652+
expect(line.leaders?.[0]?.from).toBeGreaterThan(100);
1653+
const trailingTab = block.runs[3];
1654+
if (trailingTab.kind === 'tab') {
1655+
expect(trailingTab.width).toBeGreaterThan(50);
1656+
}
1657+
});
1658+
1659+
it('maps three trailing tabs to two explicit alignment stops (asymmetric case)', async () => {
1660+
const centerStopTwips = 5000;
1661+
const endStopTwips = 10000;
1662+
const block: FlowBlock = {
1663+
kind: 'paragraph',
1664+
id: 'asymmetric-tabs',
1665+
runs: [
1666+
{ text: 'A', fontFamily: 'Arial', fontSize: 13.333 },
1667+
{ kind: 'tab', text: '\t', tabIndex: 0, pmStart: 1, pmEnd: 2 },
1668+
{ text: 'B', fontFamily: 'Arial', fontSize: 13.333 },
1669+
{ kind: 'tab', text: '\t', tabIndex: 1, pmStart: 3, pmEnd: 4 },
1670+
{ text: 'C', fontFamily: 'Arial', fontSize: 13.333 },
1671+
{ kind: 'tab', text: '\t', tabIndex: 2, pmStart: 5, pmEnd: 6 },
1672+
{ text: 'D', fontFamily: 'Arial', fontSize: 13.333 },
1673+
],
1674+
attrs: {
1675+
indent: { left: 0, right: 0, firstLine: 0, hanging: 0 },
1676+
tabs: [
1677+
{ val: 'center', pos: centerStopTwips },
1678+
{ val: 'end', pos: endStopTwips },
1679+
],
1680+
},
1681+
};
1682+
1683+
const measure = expectParagraphMeasure(await measureBlock(block, 800));
1684+
expect(measure.lines).toHaveLength(1);
1685+
// Three tabs, two alignment stops: last two tabs bind to center + end.
1686+
// The first tab must NOT bind to either alignment stop — it should fall on the
1687+
// default grid. The last tab ends near the end stop position.
1688+
const lineWidth = measure.lines[0].width;
1689+
const endStopPx = endStopTwips * (96 / 1440);
1690+
expect(lineWidth).toBeCloseTo(endStopPx, 0);
1691+
});
1692+
16221693
it('handles multiple tabs in a row', async () => {
16231694
const block: FlowBlock = {
16241695
kind: 'paragraph',

0 commit comments

Comments
 (0)