diff --git a/packages/layout-engine/layout-engine/src/index.test.ts b/packages/layout-engine/layout-engine/src/index.test.ts index e080a72196..b73c27ee11 100644 --- a/packages/layout-engine/layout-engine/src/index.test.ts +++ b/packages/layout-engine/layout-engine/src/index.test.ts @@ -2013,6 +2013,78 @@ describe('layoutDocument', () => { expect(p3Fragment).toBeDefined(); expect(p3Fragment?.width).toBe(210); // Half width = two columns }); + + it('starts new region below tallest column when columns have unequal heights', () => { + // Regression test for SD-1869: when a multi-column section has unequal column + // heights, the next region must start below the TALLEST column, not the last + // column's cursor. Without the maxCursorY fix, the new region would start at + // the shorter column's bottom, overlapping the taller one. + // + // Uses a 3-col → 2-col transition because the layout engine forces a new page + // when reducing to fewer columns than the current column index (guard at + // columnIndexBefore >= newColumns.count). With 3→2, content in col1 + // (columnIndex=1) stays on the same page (1 < 2). + const toThreeColumns: FlowBlock = { + kind: 'sectionBreak', + id: 'sb-to-3col', + type: 'continuous', + columns: { count: 3, gap: 24 }, + margins: {}, + }; + const toTwoColumns: FlowBlock = { + kind: 'sectionBreak', + id: 'sb-to-2col', + type: 'continuous', + columns: { count: 2, gap: 48 }, + margins: {}, + }; + + const blocks: FlowBlock[] = [ + { kind: 'paragraph', id: 'p1', runs: [] }, // single column preamble + toThreeColumns, + { kind: 'paragraph', id: 'p-cols', runs: [] }, // 3 lines → col0 gets 2, col1 gets 1 + toTwoColumns, + { kind: 'paragraph', id: 'p-after', runs: [] }, // must start below tallest column + ]; + + // p-cols: 3 lines of 250px each (750px total) + // Available column height = 720 (page bottom) - 112 (region top) = 608px + // Column 0 fits lines 0+1 (500px), line 2 overflows to column 1 + // Column 0 bottom = 112 + 500 = 612 + // Column 1 bottom = 112 + 250 = 362 + const measures: Measure[] = [ + makeMeasure([40]), // p1 + { kind: 'sectionBreak' }, + makeMeasure([250, 250, 250]), // p-cols: 3 lines, 2 in col0 + 1 in col1 + { kind: 'sectionBreak' }, + makeMeasure([40]), // p-after + ]; + + const options: LayoutOptions = { + pageSize: { w: 612, h: 792 }, + margins: { top: 72, right: 72, bottom: 72, left: 72 }, + }; + + const layout = layoutDocument(blocks, measures, options); + + // Everything should fit on one page + expect(layout.pages.length).toBe(1); + + // p1 at y=72, height=40 → region for 3-col section starts at y=112 + const regionTop = 72 + 40; // 112 + + // Column 0: 2 lines × 250px = 500px → bottom at 112 + 500 = 612 + // Column 1: 1 line × 250px = 250px → bottom at 112 + 250 = 362 + const tallestColumnBottom = regionTop + 500; // 612 + + const page = layout.pages[0]; + const pAfter = page.fragments.find((f) => f.blockId === 'p-after'); + expect(pAfter).toBeDefined(); + + // KEY ASSERTION: p-after must start at or below the tallest column's bottom (612) + // Without the fix, it would start at 362 (column 1's bottom), overlapping column 0 + expect(pAfter!.y).toBeGreaterThanOrEqual(tallestColumnBottom); + }); }); describe('columnBreak with multi-column pages', () => { diff --git a/packages/layout-engine/layout-engine/src/index.ts b/packages/layout-engine/layout-engine/src/index.ts index 8729e109d1..32a2aea6c1 100644 --- a/packages/layout-engine/layout-engine/src/index.ts +++ b/packages/layout-engine/layout-engine/src/index.ts @@ -1451,9 +1451,16 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options // Start a new mid-page region with different column configuration const startMidPageRegion = (state: PageState, newColumns: ColumnLayout): void => { - // Record the boundary at current Y position + // Use the maximum Y reached across all columns so the new region starts + // below ALL column content, not just the current column's cursor position. + // This prevents overlap when a multi-column section's columns have unequal heights. + const regionStartY = Math.max(state.cursorY, state.maxCursorY); + state.cursorY = regionStartY; + state.maxCursorY = regionStartY; + + // Record the boundary at the resolved Y position const boundary: ConstraintBoundary = { - y: state.cursorY, + y: regionStartY, columns: newColumns, }; state.constraintBoundaries.push(boundary); @@ -1465,7 +1472,7 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options layoutLog(`[Layout] *** COLUMNS CHANGED MID-PAGE ***`); layoutLog(` OLD activeColumns: ${JSON.stringify(activeColumns)}`); layoutLog(` NEW activeColumns: ${JSON.stringify(newColumns)}`); - layoutLog(` Current page: ${state.page.number}, cursorY: ${state.cursorY}`); + layoutLog(` Current page: ${state.page.number}, cursorY: ${state.cursorY}, maxCursorY: ${state.maxCursorY}`); // Update activeColumns so subsequent pages use this column configuration activeColumns = cloneColumnLayout(newColumns); @@ -1479,9 +1486,6 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options { left: activeLeftMargin, right: activeRightMargin }, activePageSize.w, ); - - // Note: We do NOT reset cursorY - content continues from current position - // This creates the mid-page region effect }; // Collect anchored drawings mapped to their anchor paragraphs @@ -2129,6 +2133,7 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options } } state.cursorY = tableBottomY; + state.maxCursorY = Math.max(state.maxCursorY, state.cursorY); } continue; } diff --git a/packages/layout-engine/layout-engine/src/layout-drawing.test.ts b/packages/layout-engine/layout-engine/src/layout-drawing.test.ts index 27a5f7f01e..d70415fad2 100644 --- a/packages/layout-engine/layout-engine/src/layout-drawing.test.ts +++ b/packages/layout-engine/layout-engine/src/layout-drawing.test.ts @@ -62,6 +62,7 @@ describe('layoutDrawingBlock', () => { constraintBoundaries: []; activeConstraintIndex: number; trailingSpacing: number; + maxCursorY: number; }; const createMockPageState = (overrides: Record = {}): MockPageState => @@ -76,6 +77,7 @@ describe('layoutDrawingBlock', () => { constraintBoundaries: [], activeConstraintIndex: -1, trailingSpacing: 0, + maxCursorY: 100, ...overrides, }) as MockPageState; diff --git a/packages/layout-engine/layout-engine/src/layout-drawing.ts b/packages/layout-engine/layout-engine/src/layout-drawing.ts index 12596f4da7..1ec149f646 100644 --- a/packages/layout-engine/layout-engine/src/layout-drawing.ts +++ b/packages/layout-engine/layout-engine/src/layout-drawing.ts @@ -140,4 +140,5 @@ export function layoutDrawingBlock({ state.page.fragments.push(fragment); state.cursorY += requiredHeight; + state.maxCursorY = Math.max(state.maxCursorY, state.cursorY); } diff --git a/packages/layout-engine/layout-engine/src/layout-image.ts b/packages/layout-engine/layout-engine/src/layout-image.ts index a1cfdc651e..ee14cae6b1 100644 --- a/packages/layout-engine/layout-engine/src/layout-image.ts +++ b/packages/layout-engine/layout-engine/src/layout-image.ts @@ -86,4 +86,5 @@ export function layoutImageBlock({ state.page.fragments.push(fragment); state.cursorY += requiredHeight; + state.maxCursorY = Math.max(state.maxCursorY, state.cursorY); } diff --git a/packages/layout-engine/layout-engine/src/layout-paragraph.test.ts b/packages/layout-engine/layout-engine/src/layout-paragraph.test.ts index ade89230e7..39220fe3cb 100644 --- a/packages/layout-engine/layout-engine/src/layout-paragraph.test.ts +++ b/packages/layout-engine/layout-engine/src/layout-paragraph.test.ts @@ -60,6 +60,7 @@ const makePageState = (): PageState => ({ trailingSpacing: 0, lastParagraphStyleId: undefined, lastParagraphContextualSpacing: false, + maxCursorY: 50, }); /** @@ -1271,6 +1272,7 @@ describe('layoutParagraphBlock - keepLines', () => { currentState = { ...state, cursorY: 50, // Reset to top of new page + maxCursorY: 50, page: { number: state.page.number + 1, fragments: [] }, trailingSpacing: 0, }; @@ -1420,6 +1422,7 @@ describe('layoutParagraphBlock - keepLines', () => { const advanceColumn = mock((state: PageState) => ({ ...state, cursorY: 50, + maxCursorY: 50, trailingSpacing: 0, page: { number: 2, fragments: [] }, })); diff --git a/packages/layout-engine/layout-engine/src/layout-paragraph.ts b/packages/layout-engine/layout-engine/src/layout-paragraph.ts index 598f1503c7..5bbce31c03 100644 --- a/packages/layout-engine/layout-engine/src/layout-paragraph.ts +++ b/packages/layout-engine/layout-engine/src/layout-paragraph.ts @@ -789,6 +789,7 @@ export function layoutParagraphBlock(ctx: ParagraphLayoutContext, anchors?: Para if (neededSpacingBefore > 0) { state.cursorY += neededSpacingBefore; + state.maxCursorY = Math.max(state.maxCursorY, state.cursorY); if (spacingDebugEnabled) { spacingDebugLog('spacingBefore applied', { blockId: block.id, @@ -907,6 +908,7 @@ export function layoutParagraphBlock(ctx: ParagraphLayoutContext, anchors?: Para state.page.fragments.push(fragment); state.cursorY += borderExpansion.top + fragmentHeight + borderExpansion.bottom; + state.maxCursorY = Math.max(state.maxCursorY, state.cursorY); lastState = state; fromLine = slice.toLine; } @@ -929,6 +931,7 @@ export function layoutParagraphBlock(ctx: ParagraphLayoutContext, anchors?: Para appliedSpacingAfter = 0; } else { targetState.cursorY += spacingAfter; + targetState.maxCursorY = Math.max(targetState.maxCursorY, targetState.cursorY); } targetState.trailingSpacing = appliedSpacingAfter; if (spacingDebugEnabled) { diff --git a/packages/layout-engine/layout-engine/src/layout-table.ts b/packages/layout-engine/layout-engine/src/layout-table.ts index 7f37f86d40..101de88516 100644 --- a/packages/layout-engine/layout-engine/src/layout-table.ts +++ b/packages/layout-engine/layout-engine/src/layout-table.ts @@ -1239,6 +1239,7 @@ function layoutMonolithicTable(context: TableLayoutContext): void { applyTableFragmentPmRange(fragment, context.block, context.measure); state.page.fragments.push(fragment); state.cursorY += height; + state.maxCursorY = Math.max(state.maxCursorY, state.cursorY); } /** @@ -1389,6 +1390,7 @@ export function layoutTableBlock({ applyTableFragmentPmRange(fragment, block, measure); state.page.fragments.push(fragment); state.cursorY += height; + state.maxCursorY = Math.max(state.maxCursorY, state.cursorY); return; } @@ -1554,6 +1556,7 @@ export function layoutTableBlock({ applyTableFragmentPmRange(fragment, block, measure); state.page.fragments.push(fragment); state.cursorY += fragmentHeight; + state.maxCursorY = Math.max(state.maxCursorY, state.cursorY); } const rowComplete = !hasRemainingLinesAfterContinuation; @@ -1668,6 +1671,7 @@ export function layoutTableBlock({ applyTableFragmentPmRange(fragment, block, measure); state.page.fragments.push(fragment); state.cursorY += fragmentHeight; + state.maxCursorY = Math.max(state.maxCursorY, state.cursorY); pendingPartialRow = forcedPartialRow; samePagePartialContinuation = true; isTableContinuation = true; @@ -1717,6 +1721,7 @@ export function layoutTableBlock({ applyTableFragmentPmRange(fragment, block, measure); state.page.fragments.push(fragment); state.cursorY += fragmentHeight; + state.maxCursorY = Math.max(state.maxCursorY, state.cursorY); // Handle partial row tracking if (partialRow && !partialRow.isLastPart) { diff --git a/packages/layout-engine/layout-engine/src/paginator.ts b/packages/layout-engine/layout-engine/src/paginator.ts index a87d43b73c..f09597f3ad 100644 --- a/packages/layout-engine/layout-engine/src/paginator.ts +++ b/packages/layout-engine/layout-engine/src/paginator.ts @@ -20,6 +20,10 @@ export type PageState = { lastParagraphContextualSpacing: boolean; /** Border hash of the last paragraph for between-border group detection. */ lastParagraphBorderHash?: string; + /** Tracks the maximum cursorY reached across all columns on this page. + * Used when starting a mid-page region so the new section begins below + * all column content, not just the current column's cursor. */ + maxCursorY: number; }; export type PaginatorOptions = { @@ -107,6 +111,7 @@ export function createPaginator(opts: PaginatorOptions) { trailingSpacing: 0, lastParagraphStyleId: undefined, lastParagraphContextualSpacing: false, + maxCursorY: topMargin, }; states.push(state); pages.push(state.page); @@ -123,6 +128,8 @@ export function createPaginator(opts: PaginatorOptions) { const advanceColumn = (state: PageState): PageState => { const activeCols = getActiveColumnsForState(state); if (state.columnIndex < activeCols.count - 1) { + // Snapshot max Y before resetting cursor for the next column + state.maxCursorY = Math.max(state.maxCursorY, state.cursorY); state.columnIndex += 1; if (state.activeConstraintIndex >= 0 && state.constraintBoundaries[state.activeConstraintIndex]) { state.cursorY = state.constraintBoundaries[state.activeConstraintIndex].y; diff --git a/packages/layout-engine/measuring/dom/src/index.test.ts b/packages/layout-engine/measuring/dom/src/index.test.ts index c4d26ea236..b46e43fe0a 100644 --- a/packages/layout-engine/measuring/dom/src/index.test.ts +++ b/packages/layout-engine/measuring/dom/src/index.test.ts @@ -1881,6 +1881,91 @@ describe('measureBlock', () => { } } }); + + it('uses surrounding text font size for tab line height, not hardcoded 12', async () => { + // Regression: tab runs previously hardcoded maxFontSize=12, producing + // wrong line heights when the surrounding text used a larger font. + const largeFontBlock: FlowBlock = { + kind: 'paragraph', + id: 'tab-font-size-large', + runs: [ + { text: 'Hello', fontFamily: 'Arial', fontSize: 24 }, + { kind: 'tab', text: '\t', pmStart: 5, pmEnd: 6 }, + { text: 'World', fontFamily: 'Arial', fontSize: 24 }, + ], + attrs: {}, + }; + + const smallFontBlock: FlowBlock = { + kind: 'paragraph', + id: 'tab-font-size-small', + runs: [ + { text: 'Hello', fontFamily: 'Arial', fontSize: 10 }, + { kind: 'tab', text: '\t', pmStart: 5, pmEnd: 6 }, + { text: 'World', fontFamily: 'Arial', fontSize: 10 }, + ], + attrs: {}, + }; + + const largeMeasure = expectParagraphMeasure(await measureBlock(largeFontBlock, 1000)); + const smallMeasure = expectParagraphMeasure(await measureBlock(smallFontBlock, 1000)); + + expect(largeMeasure.lines).toHaveLength(1); + expect(smallMeasure.lines).toHaveLength(1); + + // The large-font paragraph must have a taller line than the small-font one. + // With the old hardcoded 12, both could collapse to similar heights. + expect(largeMeasure.lines[0].lineHeight).toBeGreaterThan(smallMeasure.lines[0].lineHeight); + }); + + it('uses fallback font size when tab is the first run (no preceding text)', async () => { + // When a tab starts a paragraph, lastFontSize should fall back to the + // first text run's font size, not a hardcoded default. + const block: FlowBlock = { + kind: 'paragraph', + id: 'tab-first-run', + runs: [ + { kind: 'tab', text: '\t', pmStart: 0, pmEnd: 1 }, + { text: 'After tab', fontFamily: 'Arial', fontSize: 20 }, + ], + attrs: {}, + }; + + const refBlock: FlowBlock = { + kind: 'paragraph', + id: 'no-tab-ref', + runs: [{ text: 'After tab', fontFamily: 'Arial', fontSize: 20 }], + attrs: {}, + }; + + const measure = expectParagraphMeasure(await measureBlock(block, 1000)); + const refMeasure = expectParagraphMeasure(await measureBlock(refBlock, 1000)); + + expect(measure.lines).toHaveLength(1); + // Line height should match or exceed the reference (same font size drives both) + expect(measure.lines[0].lineHeight).toBeGreaterThanOrEqual(refMeasure.lines[0].lineHeight); + }); + + it('tab-only line inherits font size from following text run', async () => { + // A line that contains only a tab should derive its height from the + // paragraph's font size context, not from a hardcoded 12pt. + const block: FlowBlock = { + kind: 'paragraph', + id: 'tab-only-line', + runs: [{ kind: 'tab', text: '\t', pmStart: 0, pmEnd: 1 }], + attrs: { + // paragraph-level font size hint via a nearby run + }, + }; + + const measure = expectParagraphMeasure(await measureBlock(block, 1000)); + + expect(measure.lines).toHaveLength(1); + // With the fallback font size (default 12 when no runs present), + // the line should still have a reasonable height + expect(measure.lines[0].lineHeight).toBeGreaterThan(0); + expect(measure.lines[0].maxFontSize).toBeGreaterThan(0); + }); }); describe('space-only runs', () => { diff --git a/packages/layout-engine/measuring/dom/src/index.ts b/packages/layout-engine/measuring/dom/src/index.ts index ded7439f9e..83e050abd2 100644 --- a/packages/layout-engine/measuring/dom/src/index.ts +++ b/packages/layout-engine/measuring/dom/src/index.ts @@ -1411,7 +1411,8 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P toRun: runIndex, toChar: 1, width: 0, - maxFontSize: 12, // Default font size for tabs + maxFontSize: lastFontSize, + maxFontInfo: hasSeenTextRun ? undefined : fallbackFontInfo, maxWidth: getEffectiveWidth(lines.length === 0 ? initialAvailableWidth : bodyContentWidth), segments: [], spaceCount: 0, @@ -1432,7 +1433,7 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P // Persist measured tab width on the TabRun for downstream consumers/tests (run as TabRun & { width?: number }).width = tabAdvance; - currentLine.maxFontSize = Math.max(currentLine.maxFontSize, 12); + currentLine.maxFontSize = Math.max(currentLine.maxFontSize, lastFontSize); currentLine.toRun = runIndex; currentLine.toChar = 1; // tab is a single character let currentLeader: LeaderDecoration | null = null; diff --git a/packages/layout-engine/painters/dom/src/index.test.ts b/packages/layout-engine/painters/dom/src/index.test.ts index 2ba2389f57..3a1e8f57bf 100644 --- a/packages/layout-engine/painters/dom/src/index.test.ts +++ b/packages/layout-engine/painters/dom/src/index.test.ts @@ -1787,6 +1787,130 @@ describe('DomPainter', () => { const emptySpan = mount.querySelector('.superdoc-line span.superdoc-empty-run') as HTMLElement | null; expect(emptySpan?.dataset.pmStart).toBe('1'); expect(emptySpan?.dataset.pmEnd).toBe('1'); + // Empty-run must set explicit fontSize so it doesn't inherit fontSize:0 from the line + expect(emptySpan?.style.fontSize).toBe('18px'); + }); + + it('sets fallback fontSize on field annotation without explicit fontSize', () => { + const block: FlowBlock = { + kind: 'paragraph', + id: 'fa-no-fontsize', + runs: [ + { + kind: 'fieldAnnotation', + variant: 'text', + displayLabel: 'Client Name', + fieldId: 'F1', + fieldType: 'text', + fieldColor: '#980043', + pmStart: 0, + pmEnd: 1, + }, + ], + }; + const measure: Measure = { + kind: 'paragraph', + lines: [{ fromRun: 0, fromChar: 0, toRun: 0, toChar: 0, width: 100, ascent: 12, descent: 4, lineHeight: 20 }], + totalHeight: 20, + }; + const testLayout: Layout = { + pageSize: layout.pageSize, + pages: [ + { + number: 1, + fragments: [{ kind: 'para', blockId: 'fa-no-fontsize', fromLine: 0, toLine: 1, x: 10, y: 10, width: 200 }], + }, + ], + }; + const painter = createTestPainter({ blocks: [block], measures: [measure] }); + painter.paint(testLayout, mount); + + const annotation = mount.querySelector('.annotation') as HTMLElement | null; + // Must always set fontSize so it doesn't inherit fontSize:0 from the line. + // Falls back to 16px (browser default) when run has no explicit fontSize. + expect(annotation?.style.fontSize).toBe('16px'); + }); + + it('converts numeric fontSize to pt on field annotation', () => { + const block: FlowBlock = { + kind: 'paragraph', + id: 'fa-numeric-fontsize', + runs: [ + { + kind: 'fieldAnnotation', + variant: 'text', + displayLabel: 'Client Name', + fieldId: 'F1', + fieldType: 'text', + fieldColor: '#980043', + fontSize: 14, + pmStart: 0, + pmEnd: 1, + }, + ], + }; + const measure: Measure = { + kind: 'paragraph', + lines: [{ fromRun: 0, fromChar: 0, toRun: 0, toChar: 0, width: 100, ascent: 12, descent: 4, lineHeight: 20 }], + totalHeight: 20, + }; + const testLayout: Layout = { + pageSize: layout.pageSize, + pages: [ + { + number: 1, + fragments: [ + { kind: 'para', blockId: 'fa-numeric-fontsize', fromLine: 0, toLine: 1, x: 10, y: 10, width: 200 }, + ], + }, + ], + }; + const painter = createTestPainter({ blocks: [block], measures: [measure] }); + painter.paint(testLayout, mount); + + const annotation = mount.querySelector('.annotation') as HTMLElement | null; + // Numeric fontSize is converted to pt units. + expect(annotation?.style.fontSize).toBe('14pt'); + }); + + it('sets explicit fontSize on math run wrapper', () => { + const block: FlowBlock = { + kind: 'paragraph', + id: 'math-block', + runs: [ + { + kind: 'math', + ommlJson: {}, + textContent: 'x+1', + width: 40, + height: 14, + pmStart: 0, + pmEnd: 1, + }, + ], + }; + const measure: Measure = { + kind: 'paragraph', + lines: [{ fromRun: 0, fromChar: 0, toRun: 0, toChar: 0, width: 40, ascent: 10, descent: 4, lineHeight: 18 }], + totalHeight: 18, + }; + const testLayout: Layout = { + pageSize: layout.pageSize, + pages: [ + { + number: 1, + fragments: [{ kind: 'para', blockId: 'math-block', fromLine: 0, toLine: 1, x: 10, y: 10, width: 200 }], + }, + ], + }; + const painter = createTestPainter({ blocks: [block], measures: [measure] }); + painter.paint(testLayout, mount); + + const mathWrapper = mount.querySelector('.sd-math') as HTMLElement | null; + // Must set fontSize so fallback text doesn't inherit fontSize:0 from the line. + // Uses browser default (16px) rather than run.height, which would render tall + // expressions at 80–100px for the plain-text fallback path. + expect(mathWrapper?.style.fontSize).toBe('16px'); }); it('renders image fragments', () => { diff --git a/packages/layout-engine/painters/dom/src/renderer.ts b/packages/layout-engine/painters/dom/src/renderer.ts index 8839e631d2..912c7db70b 100644 --- a/packages/layout-engine/painters/dom/src/renderer.ts +++ b/packages/layout-engine/painters/dom/src/renderer.ts @@ -84,6 +84,7 @@ import { import { assertFragmentPmPositions, assertPmPositions } from './pm-position-validation.js'; import { createRulerElement, ensureRulerStyles, generateRulerDefinitionFromPx } from './ruler/index.js'; import { + BROWSER_DEFAULT_FONT_SIZE, CLASS_NAMES, containerStyles, containerStylesHorizontal, @@ -5228,6 +5229,12 @@ export class DomPainter { // Let browser auto-size to MathML content; estimated dimensions are for layout only wrapper.style.minWidth = `${run.width}px`; wrapper.style.minHeight = `${run.height}px`; + // Restore font-size so the plain-text fallback renders at a reasonable size + // (the line container sets fontSize: 0 to eliminate the CSS strut). MathML + // has its own internal scaling, so this only matters for the textContent + // fallback path. run.height would make tall expressions (fractions, equation + // arrays) render at 80–100px — use the browser default instead. + wrapper.style.fontSize = BROWSER_DEFAULT_FONT_SIZE; wrapper.dataset.layoutEpoch = String(this.layoutEpoch ?? 0); const mathEl = convertOmmlToMathml(run.ommlJson, this.doc); @@ -5724,12 +5731,20 @@ export class DomPainter { } } - // Apply typography to the annotation element + // Apply typography to the annotation element. + // Always set a font-size so the annotation never inherits fontSize: 0 from + // the line container (which zeroes it to eliminate the CSS strut). When the + // run has no explicit fontSize, fall back to BROWSER_DEFAULT_FONT_SIZE (the + // browser default that was previously inherited before the strut fix). if (run.fontFamily) { annotation.style.fontFamily = run.fontFamily; } - if (run.fontSize) { - const fontSize = typeof run.fontSize === 'number' ? `${run.fontSize}pt` : run.fontSize; + { + const fontSize = run.fontSize + ? typeof run.fontSize === 'number' + ? `${run.fontSize}pt` + : run.fontSize + : BROWSER_DEFAULT_FONT_SIZE; annotation.style.fontSize = fontSize; } if (run.textColor) { @@ -5933,6 +5948,9 @@ export class DomPainter { if (lineRange.pmEnd != null) { span.dataset.pmEnd = String(lineRange.pmEnd); } + // Restore font-size so the   remains a visible caret target + // (the line container sets fontSize: 0 to eliminate the CSS strut). + span.style.fontSize = `${line.lineHeight}px`; span.innerHTML = ' '; el.appendChild(span); } diff --git a/packages/layout-engine/painters/dom/src/styles.test.ts b/packages/layout-engine/painters/dom/src/styles.test.ts index d1170bf9f4..bcdd59e772 100644 --- a/packages/layout-engine/painters/dom/src/styles.test.ts +++ b/packages/layout-engine/painters/dom/src/styles.test.ts @@ -1,5 +1,18 @@ import { describe, expect, it } from 'vitest'; -import { ensureSdtContainerStyles } from './styles.js'; +import { ensureSdtContainerStyles, lineStyles } from './styles.js'; + +describe('lineStyles', () => { + it('sets height and lineHeight from the argument', () => { + const styles = lineStyles(24); + expect(styles.height).toBe('24px'); + expect(styles.lineHeight).toBe('24px'); + }); + + it('sets fontSize to 0 to eliminate the CSS strut', () => { + const styles = lineStyles(20); + expect(styles.fontSize).toBe('0'); + }); +}); describe('ensureSdtContainerStyles', () => { it('suppresses structured-content hover backgrounds in viewing mode, including grouped hover', () => { diff --git a/packages/layout-engine/painters/dom/src/styles.ts b/packages/layout-engine/painters/dom/src/styles.ts index 2f8cb1e498..e5bd524427 100644 --- a/packages/layout-engine/painters/dom/src/styles.ts +++ b/packages/layout-engine/painters/dom/src/styles.ts @@ -1,5 +1,12 @@ import { DOM_CLASS_NAMES } from './constants.js'; +/** + * Fallback font-size applied to child elements inside a line container that + * carry no explicit fontSize. Matches the browser default so rendering is + * preserved after the strut-elimination fix (fontSize: '0' on lines). + */ +export const BROWSER_DEFAULT_FONT_SIZE = '16px'; + export const CLASS_NAMES = { container: 'superdoc-layout', page: 'superdoc-page', @@ -90,6 +97,15 @@ export const fragmentStyles: Partial = { export const lineStyles = (lineHeight: number): Partial => ({ lineHeight: `${lineHeight}px`, height: `${lineHeight}px`, + // Eliminate the CSS "strut" created by the inherited font-size (typically + // the browser default 16px). Without this, the strut shifts normal-flow + // inline children down via baseline alignment, while absolutely-positioned + // children (used for tab-aligned segments) are unaffected — causing + // tab-indented first lines to appear shifted up relative to continuation + // lines. All text-bearing child elements set their own explicit font-size; + // elements that don't (empty-run, math wrapper, field annotation wrapper) + // are patched individually in renderer.ts. + fontSize: '0', position: 'relative', display: 'block', whiteSpace: 'pre',