Skip to content

SD-1403 - fix: tab stop breaking line prematurely #1720

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
chittolinag wants to merge 1 commit into
base: main
Choose a base branch
from
+75 −34
Open

SD-1403 - fix: tab stop breaking line prematurely #1720

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 8 additions & 2 deletions packages/layout-engine/layout-bridge/src/remeasure.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,9 +381,13 @@ const markerFontString = (run?: MarkerRun): string => {
* // (96px = 1 inch at 96dpi, 48px = 0.5 inch default interval)
* ```
*/
const sanitizeIndentValue = (value: number | undefined): number =>
typeof value === 'number' && Number.isFinite(value) ? value : 0;

const buildTabStopsPx = (indent?: ParagraphIndent, tabs?: TabStop[], tabIntervalTwips?: number): TabStopPx[] => {
const indentLeftPx = sanitizeIndentValue(indent?.left);
const paragraphIndentTwips = {
left: pxToTwips(Math.max(0, indent?.left ?? 0)),
left: pxToTwips(indentLeftPx),
right: pxToTwips(Math.max(0, indent?.right ?? 0)),
firstLine: pxToTwips(Math.max(0, indent?.firstLine ?? 0)),
hanging: pxToTwips(Math.max(0, indent?.hanging ?? 0)),
Expand All @@ -395,8 +399,10 @@ const buildTabStopsPx = (indent?: ParagraphIndent, tabs?: TabStop[], tabInterval
paragraphIndent: paragraphIndentTwips,
});

const leftShiftTwips = paragraphIndentTwips.left ?? 0;

return stops.map((stop: TabStop) => ({
pos: twipsToPx(stop.pos),
pos: twipsToPx(Math.max(0, stop.pos - leftShiftTwips)),
val: stop.val,
leader: stop.leader,
}));
Expand Down
46 changes: 46 additions & 0 deletions packages/layout-engine/measuring/dom/src/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import type {
Measure,
DrawingMeasure,
DrawingBlock,
TabRun,
ParagraphBlock,
} from '@superdoc/contracts';

const expectParagraphMeasure = (measure: Measure): ParagraphMeasure => {
Expand Down Expand Up @@ -207,11 +209,13 @@ describe('measureBlock', () => {
indent: { left: 0, firstLine: 48 },
wordLayout: {
indentLeftPx: 0,
firstLineIndentMode: true,
// Intentionally omit top-level textStartPx to simulate partial/legacy producers.
marker: {
markerText: '(a)',
markerBoxWidthPx: 24,
gutterWidthPx: 8,
markerX: 0,
textStartX,
run: {
fontFamily: 'Times New Roman',
Expand Down Expand Up @@ -1387,6 +1391,48 @@ describe('measureBlock', () => {
}
});

it('keeps default tab width independent of paragraph left indent', async () => {
const contentWidth = 500;
const createBlock = (indentLeft?: number): FlowBlock => ({
kind: 'paragraph',
id: `tab-indent-${indentLeft ?? 0}`,
runs: [
{
text: 'Label',
fontFamily: 'Arial',
fontSize: 12,
},
{
kind: 'tab',
text: '\t',
tabIndex: 0,
} as TabRun,
{
text: 'Value goes here',
fontFamily: 'Arial',
fontSize: 12,
},
],
attrs: {
...(indentLeft != null ? { indent: { left: indentLeft } } : {}),
tabIntervalTwips: 720,
},
});

const baseBlock = createBlock(0) as ParagraphBlock;
const indentedBlock = createBlock(4320 / 15) as ParagraphBlock;

expectParagraphMeasure(await measureBlock(baseBlock, contentWidth));
expectParagraphMeasure(await measureBlock(indentedBlock, contentWidth));

const baseTab = baseBlock.runs[1] as TabRun;
const indentedTab = indentedBlock.runs[1] as TabRun;

expect(baseTab.width).toBeGreaterThan(0);
expect(indentedTab.width).toBeGreaterThan(0);
expect(Math.abs(indentedTab.width! - baseTab.width!)).toBeLessThan(0.001);
});

it('handles multiple tabs in a row', async () => {
const block: FlowBlock = {
kind: 'paragraph',
Expand Down
53 changes: 21 additions & 32 deletions packages/layout-engine/measuring/dom/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1159,14 +1159,10 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P

// Advance to next tab stop using the same logic as inline "\t" handling
const originX = currentLine.width;
// Use first-line effective indent (accounts for hanging) on first line, body indent otherwise
const effectiveIndent = lines.length === 0 ? indentLeft + rawFirstLineOffset : indentLeft;
const absCurrentX = currentLine.width + effectiveIndent;
const { target, nextIndex, stop } = getNextTabStopPx(absCurrentX, tabStops, tabStopCursor);
const { target, nextIndex, stop } = getNextTabStopPx(currentLine.width, tabStops, tabStopCursor);
tabStopCursor = nextIndex;
const maxAbsWidth = currentLine.maxWidth + effectiveIndent;
const clampedTarget = Math.min(target, maxAbsWidth);
const tabAdvance = Math.max(0, clampedTarget - absCurrentX);
const clampedTarget = Math.min(target, currentLine.maxWidth);
const tabAdvance = Math.max(0, clampedTarget - currentLine.width);
Comment on lines 1161 to +1165
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Jan 12, 2026

Choose a reason for hiding this comment

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

P2 Badge Account for first-line indent when resolving tab stops

Tab advancement now uses currentLine.width directly, but currentLine.width is measured from the first-line text start (which includes firstLine - hanging), while buildTabStopsPx only shifts by indentLeft. On paragraphs with a non‑zero first-line or hanging indent, this means the first line’s tab positions are effectively offset by the first-line indent: the tab stop chosen is too far right (and can skip a nearer stop), so tab leaders and aligned text shift by the first-line offset. This is a regression from the prior effectiveIndent path that added rawFirstLineOffset for line 0; you’ll see it on first lines that include tabs.

Useful? React with 👍 / 👎.

currentLine.width = roundValue(currentLine.width + tabAdvance);
// Persist measured tab width on the TabRun for downstream consumers/tests
(run as TabRun & { width?: number }).width = tabAdvance;
Expand All @@ -1178,9 +1174,8 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P
// Emit leader decoration if requested
if (stop && stop.leader && stop.leader !== 'none') {
const leaderStyle: 'heavy' | 'dot' | 'hyphen' | 'underscore' | 'middleDot' = stop.leader;
const relativeTarget = clampedTarget - effectiveIndent;
const from = Math.min(originX, relativeTarget);
const to = Math.max(originX, relativeTarget);
const from = Math.min(originX, clampedTarget);
const to = Math.max(originX, clampedTarget);
if (!currentLine.leaders) currentLine.leaders = [];
currentLine.leaders.push({ from, to, style: leaderStyle });
}
Expand All @@ -1196,26 +1191,25 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P

if (groupMeasure.totalWidth > 0) {
// Calculate the aligned starting X position based on total group width
const relativeTarget = clampedTarget - effectiveIndent;
let groupStartX: number;
if (stop.val === 'end') {
// Right-align: position so right edge of group is at tab stop
groupStartX = Math.max(0, relativeTarget - groupMeasure.totalWidth);
groupStartX = Math.max(0, clampedTarget - groupMeasure.totalWidth);
} else if (stop.val === 'center') {
// Center-align: position so center of group is at tab stop
groupStartX = Math.max(0, relativeTarget - groupMeasure.totalWidth / 2);
groupStartX = Math.max(0, clampedTarget - groupMeasure.totalWidth / 2);
} else {
// Decimal-align: position so decimal point is at tab stop
const beforeDecimal = groupMeasure.beforeDecimalWidth ?? groupMeasure.totalWidth;
groupStartX = Math.max(0, relativeTarget - beforeDecimal);
groupStartX = Math.max(0, clampedTarget - beforeDecimal);
}

// Set up active tab group for subsequent run processing
activeTabGroup = {
measure: groupMeasure,
startX: groupStartX,
currentX: groupStartX,
target: relativeTarget,
target: clampedTarget,
val: stop.val,
};

Expand All @@ -1228,7 +1222,7 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P
pendingTabAlignment = null;
} else {
// For start-aligned tabs, use the existing pendingTabAlignment mechanism
pendingTabAlignment = { target: clampedTarget - effectiveIndent, val: stop.val };
pendingTabAlignment = { target: clampedTarget, val: stop.val };
}
} else {
pendingTabAlignment = null;
Expand Down Expand Up @@ -2081,14 +2075,10 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P
};
}
const originX = currentLine.width;
// Use first-line effective indent (accounts for hanging) on first line, body indent otherwise
const effectiveIndent = lines.length === 0 ? indentLeft + rawFirstLineOffset : indentLeft;
const absCurrentX = currentLine.width + effectiveIndent;
const { target, nextIndex, stop } = getNextTabStopPx(absCurrentX, tabStops, tabStopCursor);
const { target, nextIndex, stop } = getNextTabStopPx(currentLine.width, tabStops, tabStopCursor);
tabStopCursor = nextIndex;
const maxAbsWidth = currentLine.maxWidth + effectiveIndent;
const clampedTarget = Math.min(target, maxAbsWidth);
const tabAdvance = Math.max(0, clampedTarget - absCurrentX);
const clampedTarget = Math.min(target, currentLine.maxWidth);
const tabAdvance = Math.max(0, clampedTarget - currentLine.width);
currentLine.width = roundValue(currentLine.width + tabAdvance);

currentLine.maxFontInfo = updateMaxFontInfo(currentLine.maxFontSize, currentLine.maxFontInfo, run);
Expand All @@ -2098,17 +2088,16 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P
charPosInRun += 1;
if (stop) {
validateTabStopVal(stop);
pendingTabAlignment = { target: clampedTarget - effectiveIndent, val: stop.val };
pendingTabAlignment = { target: clampedTarget, val: stop.val };
} else {
pendingTabAlignment = null;
}

// Emit leader decoration if requested
if (stop && stop.leader && stop.leader !== 'none' && stop.leader !== 'middleDot') {
const leaderStyle: 'heavy' | 'dot' | 'hyphen' | 'underscore' = stop.leader;
const relativeTarget = clampedTarget - effectiveIndent;
const from = Math.min(originX, relativeTarget);
const to = Math.max(originX, relativeTarget);
const from = Math.min(originX, clampedTarget);
const to = Math.max(originX, clampedTarget);
if (!currentLine.leaders) currentLine.leaders = [];
currentLine.leaders.push({ from, to, style: leaderStyle });
}
Expand Down Expand Up @@ -3183,24 +3172,24 @@ const resolveIndentHanging = (item: ListBlock['items'][number]): number => {
* Converts indent from px→twips, calls engine with twips, converts result twips→px.
*/
const buildTabStopsPx = (indent?: ParagraphIndent, tabs?: TabStop[], tabIntervalTwips?: number): TabStopPx[] => {
// Convert indent from pixels to twips for the engine
const indentLeftPx = sanitizeIndent(indent?.left);
const paragraphIndentTwips = {
left: pxToTwips(sanitizePositive(indent?.left)),
left: pxToTwips(indentLeftPx),
right: pxToTwips(sanitizePositive(indent?.right)),
firstLine: pxToTwips(sanitizePositive(indent?.firstLine)),
hanging: pxToTwips(sanitizePositive(indent?.hanging)),
};

// Engine works in twips (tabs already in twips from PM adapter)
const stops = computeTabStops({
explicitStops: tabs ?? [],
defaultTabInterval: tabIntervalTwips ?? DEFAULT_TAB_INTERVAL_TWIPS,
paragraphIndent: paragraphIndentTwips,
});

// Convert resulting tab stops from twips to pixels for measurement
const leftShiftTwips = paragraphIndentTwips.left ?? 0;

return stops.map((stop) => ({
pos: twipsToPx(stop.pos),
pos: twipsToPx(Math.max(0, stop.pos - leftShiftTwips)),
val: stop.val,
leader: stop.leader,
}));
Expand Down