diff --git a/devtools/visual-testing/pnpm-lock.yaml b/devtools/visual-testing/pnpm-lock.yaml index 9c960fa4f6..7ac085f4a3 100644 --- a/devtools/visual-testing/pnpm-lock.yaml +++ b/devtools/visual-testing/pnpm-lock.yaml @@ -2072,7 +2072,7 @@ packages: resolution: {integrity: sha512-l63NF9y/cLROq/yqKXSLtcMeeyOfnSQlfMSlzFt/K73oIaD8DGaQWd7Z34X9GPiKqP5rbSh84Hl4bOlLcjiSrQ==} superdoc@file:../../packages/superdoc/superdoc.tgz: - resolution: {integrity: sha512-jBc73CsYGAxZJwWjMnPPLh4HhvRFv1uwKA/6KGgGOqV3RVaC0xFzRtMJsCt4ffj1YQgC6SSdxKyH3wKcLwsiUw==, tarball: file:../../packages/superdoc/superdoc.tgz} + resolution: {integrity: sha512-fq7xlmIhxeo8oE9pKOefs9PLoSE1MB1ZN1Qt4HLUkNezv/u3UjbR9jYQKl7fiw1doH2w6h7afgXSYY0/Rhj9Ng==, tarball: file:../../packages/superdoc/superdoc.tgz} version: 1.11.0 peerDependencies: '@hocuspocus/provider': ^2.13.6 diff --git a/devtools/visual-testing/tests/interactions/stories/tables/autofit-table-docx-rendering.ts b/devtools/visual-testing/tests/interactions/stories/tables/autofit-table-docx-rendering.ts new file mode 100644 index 0000000000..0a5d7b6bac --- /dev/null +++ b/devtools/visual-testing/tests/interactions/stories/tables/autofit-table-docx-rendering.ts @@ -0,0 +1,48 @@ +import { defineStory } from '@superdoc-testing/helpers'; + +const WAIT_LONG_MS = 800; + +/** + * SD-1895: Auto-layout tables from DOCX should fill page width + * + * Uses the actual document from the SD-1895 bug report which has + * auto-layout tables with column widths defined by cell size. + * Before the fix, columns rendered at their raw measurement widths + * leaving unused space. After the fix, columns scale up to fill + * the available page width. + */ +export default defineStory({ + name: 'autofit-table-docx-rendering', + description: 'SD-1895: Auto-layout tables from DOCX should fill page width', + tickets: ['SD-1895'], + startDocument: 'tables/SD-1895-autofit-issue.docx', + layout: true, + hideCaret: true, + hideSelection: true, + + async run(page, helpers): Promise { + const { step, waitForStable, milestone } = helpers; + + await step('Wait for document to load', async () => { + await page.waitForSelector('.superdoc-page', { timeout: 30_000 }); + await waitForStable(WAIT_LONG_MS); + await milestone('page1-autofit-table', 'Auto-layout table should fill page width'); + }); + + await step('Scroll to page 2 if present', async () => { + const hasPage2 = await page.evaluate(() => { + const pages = document.querySelectorAll('.superdoc-page'); + if (pages.length > 1) { + const container = document.querySelector('.harness-main'); + pages[1].scrollIntoView({ block: 'start' }); + return true; + } + return false; + }); + if (hasPage2) { + await waitForStable(WAIT_LONG_MS); + await milestone('page2-autofit-table', 'Table on page 2 should also fill page width'); + } + }); + }, +}); diff --git a/devtools/visual-testing/tests/interactions/stories/tables/autofit-table-fill-width.ts b/devtools/visual-testing/tests/interactions/stories/tables/autofit-table-fill-width.ts new file mode 100644 index 0000000000..2d489af583 --- /dev/null +++ b/devtools/visual-testing/tests/interactions/stories/tables/autofit-table-fill-width.ts @@ -0,0 +1,107 @@ +import { defineStory } from '@superdoc-testing/helpers'; + +const WAIT_MS = 400; +const WAIT_LONG_MS = 800; + +/** + * SD-1895: Auto-layout tables should fill page width + * + * Tables inserted via the editor use auto-layout mode. With the fix, + * auto-layout tables scale their column widths UP to fill the available + * page width (matching Word behavior) rather than leaving unused space. + * + * This test inserts tables with different column counts and verifies + * they render cleanly filling the page width. + */ +export default defineStory({ + name: 'autofit-table-fill-width', + description: 'Verify auto-layout tables fill page width with proportional columns', + tickets: ['SD-1895'], + startDocument: null, + layout: true, + hideCaret: true, + hideSelection: true, + + async run(_page, helpers): Promise { + const { step, focus, type, press, waitForStable, milestone, executeCommand } = helpers; + + await step('Insert a 4-column table', async () => { + await focus(); + await type('Table with 4 columns:'); + await press('Enter'); + await executeCommand('insertTable', { rows: 3, cols: 4, withHeaderRow: false }); + await waitForStable(WAIT_LONG_MS); + }); + + await step('Fill table cells with content', async () => { + // Row 1 + await type('Name'); + await press('Tab'); + await type('Department'); + await press('Tab'); + await type('Role'); + await press('Tab'); + await type('Status'); + // Row 2 + await press('Tab'); + await type('Alice Smith'); + await press('Tab'); + await type('Engineering'); + await press('Tab'); + await type('Senior Developer'); + await press('Tab'); + await type('Active'); + // Row 3 + await press('Tab'); + await type('Bob Johnson'); + await press('Tab'); + await type('Marketing'); + await press('Tab'); + await type('Content Lead'); + await press('Tab'); + await type('Active'); + await waitForStable(WAIT_LONG_MS); + await milestone('four-column-table', 'Table with 4 columns should fill page width'); + }); + + await step('Add paragraph and insert 6-column table', async () => { + // Move cursor after table + await press('ArrowDown'); + await press('ArrowDown'); + await waitForStable(WAIT_MS); + await press('Enter'); + await type('Table with 6 columns:'); + await press('Enter'); + await executeCommand('insertTable', { rows: 2, cols: 6, withHeaderRow: false }); + await waitForStable(WAIT_LONG_MS); + }); + + await step('Fill 6-column table', async () => { + await type('Col 1'); + await press('Tab'); + await type('Col 2'); + await press('Tab'); + await type('Col 3'); + await press('Tab'); + await type('Col 4'); + await press('Tab'); + await type('Col 5'); + await press('Tab'); + await type('Col 6'); + await press('Tab'); + await type('Data A'); + await press('Tab'); + await type('Data B'); + await press('Tab'); + await type('Data C'); + await press('Tab'); + await type('Data D'); + await press('Tab'); + await type('Data E'); + await press('Tab'); + await type('Data F'); + await waitForStable(WAIT_LONG_MS); + await milestone('six-column-table', 'Both tables should fill page width with proportional columns'); + }); + }, +}); diff --git a/devtools/visual-testing/tests/interactions/stories/tables/percent-width-table-overflow.ts b/devtools/visual-testing/tests/interactions/stories/tables/percent-width-table-overflow.ts new file mode 100644 index 0000000000..8c9313821e --- /dev/null +++ b/devtools/visual-testing/tests/interactions/stories/tables/percent-width-table-overflow.ts @@ -0,0 +1,44 @@ +import { defineStory } from '@superdoc-testing/helpers'; + +const WAIT_LONG_MS = 800; + +/** + * SD-1859: Percent-width tables should not overflow page bounds in mixed orientation docs + * + * Uses the actual document from the SD-1859 bug report: NuraBio document + * which has a percent-width table in a document with mixed portrait/landscape sections. + * Before the fix, the table overflowed the right edge of portrait pages because + * column widths were computed using landscape dimensions but rendered in portrait. + * After the fix, column widths are rescaled to fit the current page's content area. + */ +export default defineStory({ + name: 'percent-width-table-overflow', + description: 'SD-1859: Percent-width tables should fit within portrait page bounds', + tickets: ['SD-1859'], + startDocument: 'tables/SD-1859-mixed-orientation.docx', + layout: true, + hideCaret: true, + hideSelection: true, + + async run(page, helpers): Promise { + const { step, waitForStable, milestone } = helpers; + + await step('Wait for document to load', async () => { + await page.waitForSelector('.superdoc-page', { timeout: 30_000 }); + await waitForStable(WAIT_LONG_MS); + await milestone('page1-table', 'Page 1 table should fit within page bounds'); + }); + + await step('Scroll to see the table on page 2', async () => { + await page.evaluate(() => { + const container = document.querySelector('.harness-main'); + const scrollTarget = container?.querySelector('.superdoc-page:nth-child(2)'); + if (scrollTarget) { + scrollTarget.scrollIntoView({ block: 'start' }); + } + }); + await waitForStable(WAIT_LONG_MS); + await milestone('page2-table', 'Table should not overflow right edge of portrait page'); + }); + }, +}); diff --git a/packages/layout-engine/contracts/src/index.ts b/packages/layout-engine/contracts/src/index.ts index 77bd061260..e7aa16147c 100644 --- a/packages/layout-engine/contracts/src/index.ts +++ b/packages/layout-engine/contracts/src/index.ts @@ -1584,6 +1584,9 @@ export type TableFragment = { metadata?: TableFragmentMetadata; pmStart?: number; pmEnd?: number; + /** Per-fragment column widths, rescaled when table is clamped to section width. + * When set, the renderer uses these instead of measure.columnWidths. */ + columnWidths?: number[]; }; export type ImageFragment = { diff --git a/packages/layout-engine/layout-bridge/src/incrementalLayout.ts b/packages/layout-engine/layout-bridge/src/incrementalLayout.ts index 82433951d4..a3efaba015 100644 --- a/packages/layout-engine/layout-bridge/src/incrementalLayout.ts +++ b/packages/layout-engine/layout-bridge/src/incrementalLayout.ts @@ -1597,6 +1597,28 @@ export async function incrementalLayout( tableX += indent; tableWidth = Math.max(0, tableWidth - indent); } + // Rescale column widths when table was clamped to section width. + // This happens in mixed-orientation docs where measurement uses the + // widest section but rendering is per-section (SD-1859). + let fragmentColumnWidths: number[] | undefined; + if ( + tableWidthRaw > tableWidth && + measure.columnWidths && + measure.columnWidths.length > 0 && + tableWidthRaw > 0 + ) { + const scale = tableWidth / tableWidthRaw; + fragmentColumnWidths = measure.columnWidths.map((w: number) => Math.max(1, Math.round(w * scale))); + const scaledSum = fragmentColumnWidths.reduce((a: number, b: number) => a + b, 0); + const target = Math.round(tableWidth); + if (scaledSum !== target && fragmentColumnWidths.length > 0) { + fragmentColumnWidths[fragmentColumnWidths.length - 1] = Math.max( + 1, + fragmentColumnWidths[fragmentColumnWidths.length - 1] + (target - scaledSum), + ); + } + } + page.fragments.push({ kind: 'table', blockId: range.blockId, @@ -1606,6 +1628,7 @@ export async function incrementalLayout( y: cursorY, width: tableWidth, height: Math.max(0, measure.totalHeight ?? 0), + columnWidths: fragmentColumnWidths, }); cursorY += getRangeRenderHeight(range); return; diff --git a/packages/layout-engine/layout-bridge/test/resolveMeasurementConstraints.test.ts b/packages/layout-engine/layout-bridge/test/resolveMeasurementConstraints.test.ts index 21bb7e85f8..8b5b7b3e5f 100644 --- a/packages/layout-engine/layout-bridge/test/resolveMeasurementConstraints.test.ts +++ b/packages/layout-engine/layout-bridge/test/resolveMeasurementConstraints.test.ts @@ -451,4 +451,56 @@ describe('resolveMeasurementConstraints', () => { expect(result.measurementHeight).toBe(20); }); }); + + describe('mixed-orientation documents (SD-1859)', () => { + it('takes max width across portrait and landscape sections', () => { + // First section: portrait (612 x 792) + const options: LayoutOptions = { + pageSize: { w: 612, h: 792 }, + margins: { top: 72, right: 72, bottom: 72, left: 72 }, + }; + + // Second section: landscape (792 x 612) + const blocks: FlowBlock[] = [ + { + kind: 'sectionBreak', + id: 'sb-1', + pageSize: { w: 792, h: 612 }, + margins: { top: 72, right: 72, bottom: 72, left: 72 }, + } as SectionBreakBlock, + ]; + + const result = resolveMeasurementConstraints(options, blocks); + + // Portrait content width: 612 - 144 = 468 + // Landscape content width: 792 - 144 = 648 + // Should take MAX: 648 (landscape width) + expect(result.measurementWidth).toBe(648); + }); + + it('takes max width across sections with different margins', () => { + // First section: narrow margins (content width = 612 - 100 = 512) + const options: LayoutOptions = { + pageSize: { w: 612, h: 792 }, + margins: { top: 72, right: 50, bottom: 72, left: 50 }, + }; + + // Second section: wider margins (content width = 612 - 200 = 412) + const blocks: FlowBlock[] = [ + { + kind: 'sectionBreak', + id: 'sb-1', + pageSize: { w: 612, h: 792 }, + margins: { top: 72, right: 100, bottom: 72, left: 100 }, + } as SectionBreakBlock, + ]; + + const result = resolveMeasurementConstraints(options, blocks); + + // First section content width: 612 - 100 = 512 + // Second section content width: 612 - 200 = 412 + // Should take MAX: 512 (first section is wider) + expect(result.measurementWidth).toBe(512); + }); + }); }); diff --git a/packages/layout-engine/layout-engine/src/layout-table.test.ts b/packages/layout-engine/layout-engine/src/layout-table.test.ts index 9a286b75c2..1133939451 100644 --- a/packages/layout-engine/layout-engine/src/layout-table.test.ts +++ b/packages/layout-engine/layout-engine/src/layout-table.test.ts @@ -2,9 +2,9 @@ * Tests for table layout with column boundary metadata generation */ -import { describe, it, expect } from 'vitest'; +import type { BlockId, TableAttrs, TableBlock, TableFragment, TableMeasure } from '@superdoc/contracts'; +import { describe, expect, it } from 'vitest'; import { layoutTableBlock } from './layout-table.js'; -import type { TableBlock, TableMeasure, TableFragment, BlockId, TableAttrs } from '@superdoc/contracts'; /** * Creates a dummy table fragment for test scenarios where prior page content is needed. @@ -3167,4 +3167,117 @@ describe('layoutTableBlock', () => { } }); }); + + describe('column width rescaling (SD-1859)', () => { + it('should rescale column widths when table is wider than section content width', () => { + // Simulate a table measured at landscape width (700px) but rendered in + // a portrait section (450px). Column widths should be rescaled to fit. + const block = createMockTableBlock(2); + const measure = createMockTableMeasure([250, 200, 250], [30, 30]); + // measure.totalWidth = 700 + + const fragments: TableFragment[] = []; + const mockPage = { fragments }; + + layoutTableBlock({ + block, + measure, + columnWidth: 450, // Portrait section width (narrower than table) + ensurePage: () => ({ + page: mockPage, + columnIndex: 0, + cursorY: 0, + contentBottom: 1000, + }), + advanceColumn: (state) => state, + columnX: () => 0, + }); + + expect(fragments).toHaveLength(1); + const fragment = fragments[0]; + + // Fragment width should be clamped to section width + expect(fragment.width).toBe(450); + + // Column widths should be rescaled proportionally + expect(fragment.columnWidths).toBeDefined(); + expect(fragment.columnWidths!.length).toBe(3); + + // Sum of rescaled column widths should equal fragment width + const sum = fragment.columnWidths!.reduce((a, b) => a + b, 0); + expect(sum).toBe(450); + + // Proportions should be maintained (250:200:250 → ~161:129:161) + expect(fragment.columnWidths![0]).toBeGreaterThan(fragment.columnWidths![1]); + expect(fragment.columnWidths![0]).toBeCloseTo(fragment.columnWidths![2], -1); + }); + + it('should not set fragment columnWidths when table fits within section width', () => { + const block = createMockTableBlock(2); + const measure = createMockTableMeasure([100, 150, 100], [30, 30]); + // measure.totalWidth = 350 + + const fragments: TableFragment[] = []; + const mockPage = { fragments }; + + layoutTableBlock({ + block, + measure, + columnWidth: 450, // Section is wider than table + ensurePage: () => ({ + page: mockPage, + columnIndex: 0, + cursorY: 0, + contentBottom: 1000, + }), + advanceColumn: (state) => state, + columnX: () => 0, + }); + + expect(fragments).toHaveLength(1); + // No rescaling needed — columnWidths should be undefined + expect(fragments[0].columnWidths).toBeUndefined(); + }); + + it('should rescale column widths on paginated table fragments', () => { + // Table that splits across pages should have rescaled column widths on each fragment + const block = createMockTableBlock(4); + const measure = createMockTableMeasure([300, 300], [200, 200, 200, 200]); + // totalWidth = 600, each row = 200px + + const fragments: TableFragment[] = []; + let pageIndex = 0; + + layoutTableBlock({ + block, + measure, + columnWidth: 400, // Narrower than table + ensurePage: () => ({ + page: { fragments }, + columnIndex: 0, + cursorY: 0, + contentBottom: 500, // Only fits ~2 rows per page + }), + advanceColumn: (state) => { + pageIndex++; + return { + ...state, + cursorY: 0, + contentBottom: 500, + }; + }, + columnX: () => 0, + }); + + // Should have multiple fragments (table paginated) + expect(fragments.length).toBeGreaterThanOrEqual(1); + + // Every fragment should have rescaled column widths + for (const fragment of fragments) { + expect(fragment.columnWidths).toBeDefined(); + const sum = fragment.columnWidths!.reduce((a, b) => a + b, 0); + expect(sum).toBe(400); + } + }); + }); }); diff --git a/packages/layout-engine/layout-engine/src/layout-table.ts b/packages/layout-engine/layout-engine/src/layout-table.ts index 4fe6a0233a..f0d8da01d4 100644 --- a/packages/layout-engine/layout-engine/src/layout-table.ts +++ b/packages/layout-engine/layout-engine/src/layout-table.ts @@ -168,6 +168,39 @@ function resolveTableFrame( return applyTableIndent(baseX, width, tableIndent); } +/** + * Rescales column widths when a table is clamped to fit a narrower section. + * + * In mixed-orientation documents, tables are measured at the widest section's + * content width but may render in narrower sections. When the measured total + * width exceeds the fragment width, column widths must be proportionally + * rescaled so cells don't overflow the fragment container (SD-1859). + * + * @returns Rescaled column widths if clamping occurred, undefined otherwise. + */ +function rescaleColumnWidths( + measureColumnWidths: number[] | undefined, + measureTotalWidth: number, + fragmentWidth: number, +): number[] | undefined { + if ( + !measureColumnWidths || + measureColumnWidths.length === 0 || + measureTotalWidth <= fragmentWidth || + measureTotalWidth <= 0 + ) { + return undefined; + } + const scale = fragmentWidth / measureTotalWidth; + const scaled = measureColumnWidths.map((w) => Math.max(1, Math.round(w * scale))); + const scaledSum = scaled.reduce((a, b) => a + b, 0); + const target = Math.round(fragmentWidth); + if (scaledSum !== target && scaled.length > 0) { + scaled[scaled.length - 1] = Math.max(1, scaled[scaled.length - 1] + (target - scaledSum)); + } + return scaled; +} + /** * Calculate minimum width for a table column. * @@ -300,6 +333,7 @@ function calculateFragmentHeight( type SplitPointResult = { endRow: number; // Exclusive row index (next row after last included) partialRow: PartialRowInfo | null; // Null for row-boundary splits, PartialRowInfo for mid-row splits + forcePageBreak?: boolean; // When true, force a page break after this fragment (rowspan-aware clean break) }; /** @@ -864,6 +898,13 @@ function findSplitPoint( let accumulatedHeight = 0; let lastFitRow = startRow; // Last row that fit completely + // Rowspan-aware splitting: track the farthest row reached by any active rowspan + // and the last boundary where no rowspan crosses (a "clean" break point). + // When the standard break point splits a rowspan group, prefer the clean break + // to avoid continuation cells and match Word's behavior. + let maxRowspanEnd = startRow; + let lastCleanFitRow = startRow; + for (let i = startRow; i < block.rows.length; i++) { const row = block.rows[i]; const rowMeasure = measure.rows[i]; @@ -873,11 +914,26 @@ function findSplitPoint( cantSplit = true; } + // Track the farthest rowspan extent from this row's cells + if (rowMeasure) { + for (const cellMeasure of rowMeasure.cells) { + const rs = cellMeasure.rowSpan ?? 1; + if (rs > 1) { + maxRowspanEnd = Math.max(maxRowspanEnd, i + rs); + } + } + } + // Check if this row fits completely if (accumulatedHeight + rowHeight <= availableHeight) { // Row fits completely accumulatedHeight += rowHeight; lastFitRow = i + 1; // Next row index (exclusive) + + // A boundary is "clean" if no active rowspan crosses it + if (maxRowspanEnd <= i + 1) { + lastCleanFitRow = i + 1; + } } else { // Row doesn't fit completely const remainingHeight = availableHeight - accumulatedHeight; @@ -895,6 +951,10 @@ function findSplitPoint( if (lastFitRow === startRow) { return { endRow: startRow, partialRow: null }; } + // Prefer a clean break point that avoids splitting rowspan groups + if (maxRowspanEnd > lastFitRow && lastCleanFitRow > startRow) { + return { endRow: lastCleanFitRow, partialRow: null, forcePageBreak: true }; + } // Break before the cantSplit row return { endRow: lastFitRow, partialRow: null }; } @@ -915,7 +975,10 @@ function findSplitPoint( } } - // Can't fit any content from this row - break before it + // Can't fit any content from this row - prefer clean break if available + if (maxRowspanEnd > lastFitRow && lastCleanFitRow > startRow) { + return { endRow: lastCleanFitRow, partialRow: null, forcePageBreak: true }; + } return { endRow: lastFitRow, partialRow: null }; } } @@ -981,6 +1044,7 @@ function layoutMonolithicTable(context: TableLayoutContext): void { width, height, metadata, + columnWidths: rescaleColumnWidths(context.measure.columnWidths, context.measure.totalWidth, width), }; applyTableFragmentPmRange(fragment, context.block, context.measure); state.page.fragments.push(fragment); @@ -1120,6 +1184,7 @@ export function layoutTableBlock({ width, height, metadata, + columnWidths: rescaleColumnWidths(measure.columnWidths, measure.totalWidth, width), }; applyTableFragmentPmRange(fragment, block, measure); state.page.fragments.push(fragment); @@ -1148,11 +1213,25 @@ export function layoutTableBlock({ } } + // If repeated headers would prevent a cantSplit row from fitting, skip header repetition. + // Word does not split cantSplit rows just because repeated headers eat up space. + if (repeatHeaderCount > 0 && !pendingPartialRow) { + const bodyRow = block.rows[currentRow]; + const bodyRowHeight = measure.rows[currentRow]?.height || 0; + const bodyCantSplit = bodyRow?.attrs?.tableRowProperties?.cantSplit === true; + const spaceWithHeaders = availableHeight - headerHeight; + if (bodyCantSplit && bodyRowHeight > spaceWithHeaders && bodyRowHeight <= availableHeight) { + repeatHeaderCount = 0; + } + } + // Adjust available height for header repetition const availableForBody = repeatHeaderCount > 0 ? availableHeight - headerHeight : availableHeight; // Calculate full page height (for detecting over-tall rows) - const fullPageHeight = state.contentBottom; // Assumes content starts at y=0 + // This is the actual usable content area height, accounting for top margin. + // The ?? 0 handles test fixtures that may not set topMargin. + const fullPageHeight = state.contentBottom - (state.topMargin ?? 0); // Handle pending partial row continuation if (pendingPartialRow !== null) { @@ -1206,6 +1285,7 @@ export function layoutTableBlock({ repeatHeaderCount, partialRow: continuationPartialRow, metadata: generateFragmentMetadata(measure, rowIndex, rowIndex + 1, repeatHeaderCount), + columnWidths: rescaleColumnWidths(measure.columnWidths, measure.totalWidth, width), }; applyTableFragmentPmRange(fragment, block, measure); @@ -1236,7 +1316,13 @@ export function layoutTableBlock({ // Normal row processing const bodyStartRow = currentRow; - const { endRow, partialRow } = findSplitPoint(block, measure, bodyStartRow, availableForBody, fullPageHeight); + const { endRow, partialRow, forcePageBreak } = findSplitPoint( + block, + measure, + bodyStartRow, + availableForBody, + fullPageHeight, + ); // If no rows fit and page has content, advance if (endRow === bodyStartRow && partialRow === null && state.page.fragments.length > 0) { @@ -1269,6 +1355,7 @@ export function layoutTableBlock({ repeatHeaderCount, partialRow: forcedPartialRow, metadata: generateFragmentMetadata(measure, bodyStartRow, forcedEndRow, repeatHeaderCount), + columnWidths: rescaleColumnWidths(measure.columnWidths, measure.totalWidth, width), }; applyTableFragmentPmRange(fragment, block, measure); @@ -1310,6 +1397,7 @@ export function layoutTableBlock({ repeatHeaderCount, partialRow: partialRow || undefined, metadata: generateFragmentMetadata(measure, bodyStartRow, endRow, repeatHeaderCount), + columnWidths: rescaleColumnWidths(measure.columnWidths, measure.totalWidth, width), }; applyTableFragmentPmRange(fragment, block, measure); @@ -1326,6 +1414,13 @@ export function layoutTableBlock({ } isTableContinuation = true; + + // If findSplitPoint chose a clean rowspan boundary (earlier than the standard break), + // force a page break so the remaining rows start on the next page instead of + // continuing to fill the current page with another fragment. + if (forcePageBreak && currentRow < block.rows.length) { + state = advanceColumn(state); + } } } diff --git a/packages/layout-engine/measuring/dom/src/index.test.ts b/packages/layout-engine/measuring/dom/src/index.test.ts index dd35aa5b11..7b39ecd8a1 100644 --- a/packages/layout-engine/measuring/dom/src/index.test.ts +++ b/packages/layout-engine/measuring/dom/src/index.test.ts @@ -7,6 +7,7 @@ import type { Measure, DrawingMeasure, DrawingBlock, + TableMeasure, } from '@superdoc/contracts'; const expectParagraphMeasure = (measure: Measure): ParagraphMeasure => { @@ -3428,6 +3429,145 @@ describe('measureBlock', () => { }); }); + describe('autofit tables with colspan should not truncate grid columns', () => { + const makeCell = (id: string) => ({ + id, + blocks: [ + { + kind: 'paragraph' as const, + id: `para-${id}`, + runs: [{ text: 'Text', fontFamily: 'Arial', fontSize: 12 }], + }, + ], + }); + + it('preserves all 4 grid columns when max physical cells is 3 but colspans sum to 4', async () => { + // Reproduces SD-1797: table with 4-column grid where no row has 4 physical cells. + // Row patterns: 2 cells (span 3+1), 3 cells (span 1+2+1) + // maxCellCount must be 4 (from colspan sums), not 3 (from physical cell count) + const block: FlowBlock = { + kind: 'table', + id: 'autofit-colspan-table', + rows: [ + { + id: 'row-0', + cells: [ + { ...makeCell('c-0-0'), colSpan: 3 }, + { ...makeCell('c-0-1'), colSpan: 1 }, + ], + }, + { + id: 'row-1', + cells: [ + { ...makeCell('c-1-0'), colSpan: 1 }, + { ...makeCell('c-1-1'), colSpan: 2 }, + { ...makeCell('c-1-2'), colSpan: 1 }, + ], + }, + ], + columnWidths: [172, 13, 128, 310], // 4 grid columns from w:tblGrid + }; + + const measure = await measureBlock(block, { maxWidth: 800 }); + expect(measure.kind).toBe('table'); + if (measure.kind !== 'table') throw new Error('expected table measure'); + + // All 4 column widths should be preserved (not truncated to 3) + expect(measure.columnWidths).toHaveLength(4); + expect(measure.columnWidths).toEqual([172, 13, 128, 310]); + expect(measure.totalWidth).toBe(623); + + // Row 0: 2 cells spanning 3+1 = both cells measured + expect(measure.rows[0].cells).toHaveLength(2); + + // Row 1: 3 cells spanning 1+2+1 = all 3 cells measured + expect(measure.rows[1].cells).toHaveLength(3); + }); + + it('does not drop rightmost cell when colspan exhausts truncated grid', async () => { + // 4-column grid, rows with span patterns [2,2] and [3,1] + // Without the fix, grid gets truncated to 2 columns (max physical cells = 2), + // and the second cell in span [3,1] rows is dropped + const block: FlowBlock = { + kind: 'table', + id: 'autofit-colspan-table-2', + rows: [ + { + id: 'row-0', + cells: [ + { ...makeCell('c-0-0'), colSpan: 2 }, + { ...makeCell('c-0-1'), colSpan: 2 }, + ], + }, + { + id: 'row-1', + cells: [ + { ...makeCell('c-1-0'), colSpan: 3 }, + { ...makeCell('c-1-1'), colSpan: 1 }, + ], + }, + ], + columnWidths: [100, 50, 100, 300], // 4 grid columns + }; + + const measure = await measureBlock(block, { maxWidth: 800 }); + expect(measure.kind).toBe('table'); + if (measure.kind !== 'table') throw new Error('expected table measure'); + + // Grid should not be truncated + expect(measure.columnWidths).toHaveLength(4); + + // Both cells in each row must be present + expect(measure.rows[0].cells).toHaveLength(2); + expect(measure.rows[1].cells).toHaveLength(2); + + // Cell widths should correctly sum their spanned columns + // Row 0 cell 0: cols 0+1 = 100+50 = 150 + expect(measure.rows[0].cells[0].width).toBe(150); + // Row 0 cell 1: cols 2+3 = 100+300 = 400 + expect(measure.rows[0].cells[1].width).toBe(400); + // Row 1 cell 0: cols 0+1+2 = 100+50+100 = 250 + expect(measure.rows[1].cells[0].width).toBe(250); + // Row 1 cell 1: col 3 = 300 + expect(measure.rows[1].cells[1].width).toBe(300); + }); + + it('handles single-cell full-span row correctly', async () => { + const block: FlowBlock = { + kind: 'table', + id: 'autofit-fullspan-table', + rows: [ + { + id: 'row-0', + cells: [{ ...makeCell('c-0-0'), colSpan: 4 }], + }, + { + id: 'row-1', + cells: [ + { ...makeCell('c-1-0'), colSpan: 1 }, + { ...makeCell('c-1-1'), colSpan: 2 }, + { ...makeCell('c-1-2'), colSpan: 1 }, + ], + }, + ], + columnWidths: [100, 50, 100, 300], + }; + + const measure = await measureBlock(block, { maxWidth: 800 }); + expect(measure.kind).toBe('table'); + if (measure.kind !== 'table') throw new Error('expected table measure'); + + expect(measure.columnWidths).toHaveLength(4); + + // Full-span row: 1 cell spanning all 4 columns + expect(measure.rows[0].cells).toHaveLength(1); + expect(measure.rows[0].cells[0].width).toBe(550); // 100+50+100+300 + + // 3-cell row: all cells present + expect(measure.rows[1].cells).toHaveLength(3); + }); + }); + describe('scaleColumnWidths behavior', () => { it('scales column widths proportionally when exceeding target', async () => { const block: FlowBlock = { @@ -4530,7 +4670,7 @@ describe('measureBlock', () => { }); describe('table cell measurement with spacing.after', () => { - it('should add spacing.after to content height for all paragraphs', async () => { + it('should add spacing.after to content height for non-last paragraphs', async () => { const table: FlowBlock = { kind: 'table', id: 'table-spacing', @@ -4570,17 +4710,19 @@ describe('measureBlock', () => { const block0Measure = cellMeasure.blocks[0]; const block1Measure = cellMeasure.blocks[1]; - // Content height should include both paragraph heights and both spacing.after values + // Content height should include first paragraph's spacing.after but NOT the last paragraph's. + // In Word, the last paragraph's spacing.after is absorbed by the cell's bottom padding. // First paragraph: height + 10px spacing - // Last paragraph: height + 20px spacing (even though it's the last paragraph) + // Last paragraph: height only (spacing.after=20 is skipped) expect(block0Measure.kind).toBe('paragraph'); expect(block1Measure.kind).toBe('paragraph'); const para0Height = block0Measure.kind === 'paragraph' ? block0Measure.totalHeight : 0; const para1Height = block1Measure.kind === 'paragraph' ? block1Measure.totalHeight : 0; - // Cell height includes: para0Height + 10 + para1Height + 20 + padding (default 2 top + 2 bottom) - const expectedCellHeight = para0Height + 10 + para1Height + 20 + 4; + // Cell height includes: para0Height + 10 + para1Height + padding (default 2 top + 2 bottom) + // Last paragraph's spacing.after (20) is NOT included + const expectedCellHeight = para0Height + 10 + para1Height + 4; expect(cellMeasure.height).toBe(expectedCellHeight); }); @@ -4635,10 +4777,11 @@ describe('measureBlock', () => { const para1Height = block1.kind === 'paragraph' ? block1.totalHeight : 0; const para2Height = block2.kind === 'paragraph' ? block2.totalHeight : 0; - // Only positive spacing should be added - // Zero and negative spacing should not be added - // Cell height = para0 + para1 + para2 + 15 (positive spacing) + 4 (padding) - const expectedCellHeight = para0Height + para1Height + para2Height + 15 + 4; + // Only positive spacing should be added, and not for the last paragraph. + // Zero and negative spacing should not be added. + // para-2 is the last paragraph so its spacing.after (15) is skipped. + // Cell height = para0 + para1 + para2 + 4 (padding) + const expectedCellHeight = para0Height + para1Height + para2Height + 4; expect(cellMeasure.height).toBe(expectedCellHeight); }); @@ -4831,7 +4974,8 @@ describe('measureBlock', () => { const cellMeasure = measure.rows[0].cells[0]; // Should handle mixed block types correctly - // Paragraphs should have spacing.after applied, image should not + // Non-last paragraphs should have spacing.after applied, image should not + // Last paragraph's spacing.after is skipped expect(cellMeasure.blocks).toHaveLength(3); expect(cellMeasure.blocks[0].kind).toBe('paragraph'); expect(cellMeasure.blocks[1].kind).toBe('image'); @@ -4845,9 +4989,99 @@ describe('measureBlock', () => { const imageHeight = block1.kind === 'image' ? block1.height : 0; const para1Height = block2.kind === 'paragraph' ? block2.totalHeight : 0; - // Cell height = para0 + 10 + image + para1 + 5 + 4 (padding) - const expectedCellHeight = para0Height + 10 + imageHeight + para1Height + 5 + 4; + // Cell height = para0 + 10 + image + para1 + 4 (padding) + // Last paragraph's spacing.after (5) is NOT included + const expectedCellHeight = para0Height + 10 + imageHeight + para1Height + 4; expect(cellMeasure.height).toBe(expectedCellHeight); }); }); + + describe('table column count with rowspan', () => { + it('should preserve all grid columns when rows have fewer physical cells due to rowspan', async () => { + // Simulates PCI table structure: 4 grid columns, but some rows have only 2-3 physical cells + // because rowspan cells from above occupy grid slots. + const table: FlowBlock = { + kind: 'table', + id: 'table-rowspan-cols', + attrs: {}, + columnWidths: [170, 15, 130, 310], // 4 grid columns, sum = 625 + rows: [ + { + id: 'row-0', + cells: [ + { + id: 'cell-0-0', + colSpan: 2, + attrs: {}, + blocks: [{ kind: 'paragraph', id: 'p-0-0', runs: [{ text: 'A', fontFamily: 'Arial', fontSize: 12 }] }], + }, + { + id: 'cell-0-1', + colSpan: 2, + attrs: {}, + blocks: [{ kind: 'paragraph', id: 'p-0-1', runs: [{ text: 'B', fontFamily: 'Arial', fontSize: 12 }] }], + }, + ], + }, + { + id: 'row-1', + cells: [ + { + id: 'cell-1-0', + colSpan: 1, + rowSpan: 2, + attrs: {}, + blocks: [{ kind: 'paragraph', id: 'p-1-0', runs: [{ text: 'C', fontFamily: 'Arial', fontSize: 12 }] }], + }, + { + id: 'cell-1-1', + colSpan: 2, + attrs: {}, + blocks: [{ kind: 'paragraph', id: 'p-1-1', runs: [{ text: 'D', fontFamily: 'Arial', fontSize: 12 }] }], + }, + { + id: 'cell-1-2', + colSpan: 1, + attrs: {}, + blocks: [{ kind: 'paragraph', id: 'p-1-2', runs: [{ text: 'E', fontFamily: 'Arial', fontSize: 12 }] }], + }, + ], + }, + { + // Row 2: only 2 physical cells because col 0 is occupied by row-1 rowSpan=2 + id: 'row-2', + cells: [ + { + id: 'cell-2-0', + colSpan: 2, + attrs: {}, + blocks: [{ kind: 'paragraph', id: 'p-2-0', runs: [{ text: 'F', fontFamily: 'Arial', fontSize: 12 }] }], + }, + { + id: 'cell-2-1', + colSpan: 1, + attrs: {}, + blocks: [{ kind: 'paragraph', id: 'p-2-1', runs: [{ text: 'G', fontFamily: 'Arial', fontSize: 12 }] }], + }, + ], + }, + ], + }; + + const measure = await measureBlock(table, 625); + expect(measure.kind).toBe('table'); + const tableMeasure = measure as TableMeasure; + + // All 4 grid columns must be preserved (not truncated to 3 based on max physical cell count) + expect(tableMeasure.columnWidths).toHaveLength(4); + expect(tableMeasure.columnWidths[0]).toBe(170); + expect(tableMeasure.columnWidths[1]).toBe(15); + expect(tableMeasure.columnWidths[2]).toBe(130); + expect(tableMeasure.columnWidths[3]).toBe(310); + + // Total width should match page width + const totalWidth = tableMeasure.columnWidths.reduce((a: number, b: number) => a + b, 0); + expect(totalWidth).toBe(625); + }); + }); }); diff --git a/packages/layout-engine/measuring/dom/src/index.ts b/packages/layout-engine/measuring/dom/src/index.ts index 20f39c3daf..9dba53923b 100644 --- a/packages/layout-engine/measuring/dom/src/index.ts +++ b/packages/layout-engine/measuring/dom/src/index.ts @@ -2524,8 +2524,11 @@ async function measureTableBlock(block: TableBlock, constraints: MeasureConstrai return scaled; }; - // Determine actual column count from table structure - const maxCellCount = Math.max(1, Math.max(...block.rows.map((r) => r.cells.length))); + // Determine actual column count from table structure (accounting for colspan) + const maxCellCount = Math.max( + 1, + Math.max(...block.rows.map((r) => r.cells.reduce((sum, cell) => sum + (cell.colSpan ?? 1), 0))), + ); // Effective target width: use resolvedTableWidth if set (from percentage or explicit px), // but never exceed maxWidth (available column space) @@ -2577,10 +2580,20 @@ async function measureTableBlock(block: TableBlock, constraints: MeasureConstrai columnWidths = columnWidths.slice(0, maxCellCount); } - // Scale proportionally if total width exceeds effective target width + // Scale proportionally to fit effective target width. + // Auto-layout tables in Word fill the available page width, so we scale + // both up (when grid widths are smaller) and down (when they exceed it). const totalWidth = columnWidths.reduce((a, b) => a + b, 0); if (totalWidth > effectiveTargetWidth) { columnWidths = scaleColumnWidths(columnWidths, effectiveTargetWidth); + } else if (totalWidth < effectiveTargetWidth && effectiveTargetWidth > 0 && totalWidth > 0) { + const scale = effectiveTargetWidth / totalWidth; + columnWidths = columnWidths.map((w) => Math.max(1, Math.round(w * scale))); + const scaledSum = columnWidths.reduce((a, b) => a + b, 0); + if (scaledSum !== effectiveTargetWidth && columnWidths.length > 0) { + const diff = effectiveTargetWidth - scaledSum; + columnWidths[columnWidths.length - 1] = Math.max(1, columnWidths[columnWidths.length - 1] + diff); + } } } } else { @@ -2701,9 +2714,11 @@ async function measureTableBlock(block: TableBlock, constraints: MeasureConstrai contentHeight += blockHeight; - // Add paragraph spacing.after to content height for all paragraphs. - // Word applies spacing.after even to the last paragraph in a cell, creating space at the bottom. - if (block.kind === 'paragraph') { + // Add paragraph spacing.after to content height for non-last paragraphs. + // In Word, the last paragraph's spacing.after is absorbed by the cell's bottom padding + // and doesn't add extra height beyond the cell margin. + const isLastBlock = blockIndex === cellBlocks.length - 1; + if (block.kind === 'paragraph' && !isLastBlock) { const spacingAfter = (block as ParagraphBlock).attrs?.spacing?.after; if (typeof spacingAfter === 'number' && spacingAfter > 0) { contentHeight += spacingAfter; diff --git a/packages/layout-engine/painters/dom/src/table/renderTableCell.test.ts b/packages/layout-engine/painters/dom/src/table/renderTableCell.test.ts index 644c0f0083..cdb3fd34b3 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableCell.test.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableCell.test.ts @@ -591,12 +591,13 @@ describe('renderTableCell', () => { const firstParaWrapper = paraWrappers[0] as HTMLElement; const secondParaWrapper = paraWrappers[1] as HTMLElement; - // Both paragraphs should have margin-bottom for spacing.after + // First paragraph should have margin-bottom, last paragraph should NOT + // (last paragraph's spacing.after is absorbed by cell bottom padding) expect(firstParaWrapper.style.marginBottom).toBe('10px'); - expect(secondParaWrapper.style.marginBottom).toBe('20px'); + expect(secondParaWrapper.style.marginBottom).toBe(''); }); - it('should apply spacing.after even to the last paragraph', () => { + it('should NOT apply spacing.after to the last paragraph', () => { const lastPara: ParagraphBlock = { kind: 'paragraph', id: 'para-last', @@ -645,9 +646,9 @@ describe('renderTableCell', () => { const contentElement = cellElement.firstElementChild as HTMLElement; const paraWrapper = contentElement.firstElementChild as HTMLElement; - // Last paragraph should still have margin-bottom applied - // This matches Word's behavior - expect(paraWrapper.style.marginBottom).toBe('15px'); + // Last paragraph should NOT have margin-bottom applied + // In Word, the last paragraph's spacing.after is absorbed by the cell's bottom padding + expect(paraWrapper.style.marginBottom).toBe(''); }); it('should only apply margin-bottom when spacing.after > 0', () => { @@ -721,8 +722,8 @@ describe('renderTableCell', () => { expect(wrapper1.style.marginBottom).toBe(''); expect(wrapper2.style.marginBottom).toBe(''); - // Positive spacing should have margin-bottom - expect(wrapper3.style.marginBottom).toBe('10px'); + // Last paragraph's spacing.after is skipped (absorbed by cell bottom padding) + expect(wrapper3.style.marginBottom).toBe(''); }); it('should handle paragraphs without spacing.after attribute', () => { @@ -919,8 +920,9 @@ describe('renderTableCell', () => { const fullContent = fullCell.firstElementChild as HTMLElement; const fullWrapper = fullContent.firstElementChild as HTMLElement; - // Full render SHOULD apply spacing.after - expect(fullWrapper.style.marginBottom).toBe('15px'); + // Full render of last paragraph should NOT apply spacing.after + // (last paragraph's spacing.after is absorbed by cell bottom padding) + expect(fullWrapper.style.marginBottom).toBe(''); }); }); diff --git a/packages/layout-engine/painters/dom/src/table/renderTableCell.ts b/packages/layout-engine/painters/dom/src/table/renderTableCell.ts index c694c4d7cf..e9d7aeafb1 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableCell.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableCell.ts @@ -509,6 +509,8 @@ type TableCellRenderDependencies = { tableSdt?: SdtMetadata | null; /** Table indent in pixels (applied to table fragment positioning) */ tableIndent?: number; + /** Computed cell width from rescaled columnWidths (overrides cellMeasure.width when present) */ + cellWidth?: number; /** Starting line index for partial row rendering (inclusive) */ fromLine?: number; /** Ending line index for partial row rendering (exclusive), -1 means render to end */ @@ -594,6 +596,7 @@ export const renderTableCell = (deps: TableCellRenderDependencies): TableCellRen applySdtDataset, tableSdt, tableIndent, + cellWidth, fromLine, toLine, } = deps; @@ -609,7 +612,7 @@ export const renderTableCell = (deps: TableCellRenderDependencies): TableCellRen cellEl.style.position = 'absolute'; cellEl.style.left = `${x}px`; cellEl.style.top = `${y}px`; - cellEl.style.width = `${cellMeasure.width}px`; + cellEl.style.width = `${cellWidth ?? cellMeasure.width}px`; cellEl.style.height = `${rowHeight}px`; cellEl.style.boxSizing = 'border-box'; // Cell clips all overflow - no scrollbars, content just gets clipped at boundaries @@ -728,7 +731,8 @@ export const renderTableCell = (deps: TableCellRenderDependencies): TableCellRen const globalFromLine = fromLine ?? 0; const globalToLine = toLine === -1 || toLine === undefined ? totalLines : toLine; - const contentWidthPx = Math.max(0, cellMeasure.width - paddingLeft - paddingRight); + const effectiveCellWidth = cellWidth ?? cellMeasure.width; + const contentWidthPx = Math.max(0, effectiveCellWidth - paddingLeft - paddingRight); const contentHeightPx = Math.max(0, rowHeight - paddingTop - paddingBottom); const paragraphTopById = new Map(); let flowCursorY = 0; @@ -1018,9 +1022,10 @@ export const renderTableCell = (deps: TableCellRenderDependencies): TableCellRen flowCursorY += renderedHeight; - // Apply paragraph spacing.after as margin-bottom for all paragraphs. - // Word applies spacing.after even to the last paragraph in a cell, creating space at the bottom. - if (renderedEntireBlock) { + // Apply paragraph spacing.after as margin-bottom for non-last paragraphs. + // In Word, the last paragraph's spacing.after is absorbed by the cell's bottom padding. + const isLastBlock = i === Math.min(blockMeasures.length, cellBlocks.length) - 1; + if (renderedEntireBlock && !isLastBlock) { const spacingAfter = (block as ParagraphBlock).attrs?.spacing?.after; if (typeof spacingAfter === 'number' && spacingAfter > 0) { paraWrapper.style.marginBottom = `${spacingAfter}px`; diff --git a/packages/layout-engine/painters/dom/src/table/renderTableFragment.test.ts b/packages/layout-engine/painters/dom/src/table/renderTableFragment.test.ts index f5f8fe7c9d..c5d9a27426 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableFragment.test.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableFragment.test.ts @@ -572,6 +572,144 @@ describe('renderTableFragment', () => { }); }); + describe('cell width rescaling (SD-1859)', () => { + it('should use fragment.columnWidths for cell widths when present', () => { + // Simulates a mixed-orientation doc: table measured at landscape width (432px per col) + // but rendered in portrait where fragment.columnWidths rescales to 312px per col. + const block: TableBlock = { + kind: 'table', + id: 'test-table-1' as BlockId, + rows: [ + { + id: 'row-1' as BlockId, + cells: [ + { + id: 'cell-1-1' as BlockId, + paragraph: { + kind: 'paragraph', + id: 'para-1-1' as BlockId, + runs: [], + }, + }, + { + id: 'cell-1-2' as BlockId, + paragraph: { + kind: 'paragraph', + id: 'para-1-2' as BlockId, + runs: [], + }, + }, + ], + }, + ], + }; + + const measure: TableMeasure = { + kind: 'table', + rows: [ + { + cells: [ + { + paragraph: { kind: 'paragraph', lines: [], totalHeight: 20 }, + width: 432, + height: 20, + gridColumnStart: 0, + colSpan: 1, + }, + { + paragraph: { kind: 'paragraph', lines: [], totalHeight: 20 }, + width: 432, + height: 20, + gridColumnStart: 1, + colSpan: 1, + }, + ], + height: 20, + }, + ], + columnWidths: [432, 432], + totalWidth: 864, + totalHeight: 20, + }; + + // Fragment with rescaled column widths (portrait: 624px total) + const fragment: TableFragment = { + kind: 'table', + blockId: 'test-table-1' as BlockId, + fromRow: 0, + toRow: 1, + x: 0, + y: 0, + width: 624, + height: 20, + columnWidths: [312, 312], // rescaled from [432, 432] + }; + + blockLookup.set(fragment.blockId, { block, measure }); + + const element = renderTableFragment({ + doc, + fragment, + context, + blockLookup, + renderLine: (_block, _line, _ctx, _lineIndex, _isLastLine) => doc.createElement('div'), + applyFragmentFrame: () => {}, + applySdtDataset: () => {}, + applyStyles: () => {}, + }); + + // Find rendered cell elements (absolutely positioned divs inside container) + const cells = element.querySelectorAll('div[style*="position: absolute"]'); + expect(cells.length).toBeGreaterThanOrEqual(2); + + // Cell 1: should be at x=0, width=312 (not 432) + const cell1 = cells[0]; + expect(cell1.style.left).toBe('0px'); + expect(cell1.style.width).toBe('312px'); + + // Cell 2: should be at x=312, width=312 (not 432) + const cell2 = cells[1]; + expect(cell2.style.left).toBe('312px'); + expect(cell2.style.width).toBe('312px'); + }); + + it('should fall back to cellMeasure.width when fragment.columnWidths is absent', () => { + const block = createTestTableBlock(); + const measure = createTestTableMeasure(); + // Fragment without columnWidths — should use measure.columnWidths + const fragment: TableFragment = { + kind: 'table', + blockId: 'test-table-1' as BlockId, + fromRow: 0, + toRow: 1, + x: 0, + y: 0, + width: 100, + height: 20, + // no columnWidths + }; + + blockLookup.set(fragment.blockId, { block, measure }); + + const element = renderTableFragment({ + doc, + fragment, + context, + blockLookup, + renderLine: (_block, _line, _ctx, _lineIndex, _isLastLine) => doc.createElement('div'), + applyFragmentFrame: () => {}, + applySdtDataset: () => {}, + applyStyles: () => {}, + }); + + const cells = element.querySelectorAll('div[style*="position: absolute"]'); + expect(cells.length).toBeGreaterThanOrEqual(1); + + // Should use measure.columnWidths[0] = 100 + expect(cells[0].style.width).toBe('100px'); + }); + }); + describe('boundary segment logic', () => { it('should create segments for cells with varying rowspan', () => { // Create a table with mixed rowspans: diff --git a/packages/layout-engine/painters/dom/src/table/renderTableFragment.ts b/packages/layout-engine/painters/dom/src/table/renderTableFragment.ts index 9f3a8383c9..95982356f2 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableFragment.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableFragment.ts @@ -13,6 +13,7 @@ import { DOM_CLASS_NAMES } from '../constants.js'; import type { FragmentRenderContext, BlockLookup } from '../renderer.js'; import { renderTableRow } from './renderTableRow.js'; import { applySdtContainerStyling, type SdtBoundaryOptions } from '../utils/sdt-helpers.js'; +import { applyBorder, borderValueToSpec } from './border-utils.js'; type ApplyStylesFn = (el: HTMLElement, styles: Partial) => void; @@ -161,6 +162,9 @@ export const renderTableFragment = (deps: TableRenderDependencies): HTMLElement const block = lookup.block as TableBlock; const measure = lookup.measure as TableMeasure; + // Use per-fragment rescaled column widths when available (SD-1859: mixed-orientation docs + // where measurement width differs from section width). Falls back to measured widths. + const effectiveColumnWidths = fragment.columnWidths ?? measure.columnWidths; const tableBorders = block.attrs?.borders; const tableIndentValue = (block.attrs?.tableIndent as { width?: unknown } | null | undefined)?.width; const tableIndent = typeof tableIndentValue === 'number' && Number.isFinite(tableIndentValue) ? tableIndentValue : 0; @@ -187,7 +191,7 @@ export const renderTableFragment = (deps: TableRenderDependencies): HTMLElement // When a table splits across pages, each fragment only renders a subset of rows // (repeated headers + body rows from fromRow to toRow). Segments must match // exactly the rendered rows so resize handles don't overflow the fragment. - const columnCount = measure.columnWidths.length; + const columnCount = effectiveColumnWidths.length; // boundarySegments[colIndex] = array of {fromRow, toRow, y, height} segments where this boundary exists const boundarySegments: Array> = []; @@ -330,7 +334,7 @@ export const renderTableFragment = (deps: TableRenderDependencies): HTMLElement row: block.rows[r], totalRows: block.rows.length, tableBorders, - columnWidths: measure.columnWidths, + columnWidths: effectiveColumnWidths, allRowHeights, tableIndent, context, @@ -346,6 +350,108 @@ export const renderTableFragment = (deps: TableRenderDependencies): HTMLElement } } + // Render rowspan continuation cells ("ghost cells") + // When a table continues from a previous fragment, some grid columns in the + // first body rows may be occupied by rowspan cells that started on a previous page. + // Create empty cells to maintain table structure and borders (matching Word behavior). + if (fragment.continuesFromPrev && fragment.fromRow > 0) { + const repeatCount = fragment.repeatHeaderCount ?? 0; + + for (let r = repeatCount; r < fragment.fromRow; r++) { + const srcRowMeasure = measure.rows[r]; + if (!srcRowMeasure) continue; + + for (let ci = 0; ci < srcRowMeasure.cells.length; ci++) { + const srcCellMeasure = srcRowMeasure.cells[ci]; + const rowSpan = srcCellMeasure.rowSpan ?? 1; + if (rowSpan <= 1) continue; + + const spanEndRow = r + rowSpan; + if (spanEndRow <= fragment.fromRow) continue; + + // This cell's rowspan extends into this fragment's body rows + const gridCol = srcCellMeasure.gridColumnStart ?? 0; + const colSpan = srcCellMeasure.colSpan ?? 1; + + // Calculate x position (sum of columns before gridCol) + let ghostX = 0; + for (let i = 0; i < gridCol && i < effectiveColumnWidths.length; i++) { + ghostX += effectiveColumnWidths[i]; + } + + // Calculate width (sum of spanned columns) + let ghostWidth = 0; + for (let i = gridCol; i < gridCol + colSpan && i < effectiveColumnWidths.length; i++) { + ghostWidth += effectiveColumnWidths[i]; + } + + // Calculate height: from fromRow to min(spanEndRow, toRow) + const effectiveEnd = Math.min(spanEndRow, fragment.toRow); + let ghostHeight = 0; + for (let ri = fragment.fromRow; ri < effectiveEnd; ri++) { + ghostHeight += allRowHeights[ri] ?? 0; + } + + if (ghostWidth <= 0 || ghostHeight <= 0) continue; + + // Create ghost cell + const ghostDiv = doc.createElement('div'); + ghostDiv.style.position = 'absolute'; + ghostDiv.style.left = `${ghostX}px`; + ghostDiv.style.top = `${y}px`; + ghostDiv.style.width = `${ghostWidth}px`; + ghostDiv.style.height = `${ghostHeight}px`; + ghostDiv.style.boxSizing = 'border-box'; + ghostDiv.style.overflow = 'hidden'; + + // Resolve borders for the ghost cell + const srcCell = block.rows[r]?.cells?.[ci]; + const cellBordersAttr = srcCell?.attrs?.borders; + const hasExplicitBorders = + cellBordersAttr && + (cellBordersAttr.top !== undefined || + cellBordersAttr.right !== undefined || + cellBordersAttr.bottom !== undefined || + cellBordersAttr.left !== undefined); + const isFirstCol = gridCol === 0; + const isLastCol = gridCol + colSpan >= effectiveColumnWidths.length; + + if (hasExplicitBorders && tableBorders) { + // Use cell's borders, with table top border for continuation + applyBorder(ghostDiv, 'Top', cellBordersAttr.top ?? borderValueToSpec(tableBorders.top)); + applyBorder( + ghostDiv, + 'Left', + cellBordersAttr.left ?? borderValueToSpec(isFirstCol ? tableBorders.left : tableBorders.insideV), + ); + applyBorder( + ghostDiv, + 'Right', + cellBordersAttr.right ?? borderValueToSpec(isLastCol ? tableBorders.right : tableBorders.insideV), + ); + if (effectiveEnd <= fragment.toRow && spanEndRow <= fragment.toRow) { + applyBorder(ghostDiv, 'Bottom', cellBordersAttr.bottom ?? borderValueToSpec(tableBorders.insideH)); + } + } else if (tableBorders) { + // Resolve from table borders + applyBorder(ghostDiv, 'Top', borderValueToSpec(tableBorders.top)); + applyBorder(ghostDiv, 'Left', borderValueToSpec(isFirstCol ? tableBorders.left : tableBorders.insideV)); + applyBorder(ghostDiv, 'Right', borderValueToSpec(isLastCol ? tableBorders.right : tableBorders.insideV)); + if (effectiveEnd <= fragment.toRow && spanEndRow <= fragment.toRow) { + applyBorder(ghostDiv, 'Bottom', borderValueToSpec(tableBorders.insideH)); + } + } + + // Apply cell background if present + if (srcCell?.attrs?.background) { + ghostDiv.style.backgroundColor = srcCell.attrs.background; + } + + container.appendChild(ghostDiv); + } + } + } + // Render body rows (fromRow to toRow) for (let r = fragment.fromRow; r < fragment.toRow; r += 1) { const rowMeasure = measure.rows[r]; @@ -368,7 +474,7 @@ export const renderTableFragment = (deps: TableRenderDependencies): HTMLElement row: block.rows[r], totalRows: block.rows.length, tableBorders, - columnWidths: measure.columnWidths, + columnWidths: effectiveColumnWidths, allRowHeights, tableIndent, context, diff --git a/packages/layout-engine/painters/dom/src/table/renderTableRow.ts b/packages/layout-engine/painters/dom/src/table/renderTableRow.ts index cc8cf221fe..e154874717 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableRow.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableRow.ts @@ -323,6 +323,16 @@ export const renderTableRow = (deps: TableRowRenderDependencies): void => { const fromLine = partialRow?.fromLineByCell?.[cellIndex]; const toLine = partialRow?.toLineByCell?.[cellIndex]; + // Compute cell width from rescaled columnWidths (SD-1859: mixed-orientation docs + // where cellMeasure.width may reflect landscape measurement but the fragment renders + // in portrait). The columnWidths array is already rescaled by the layout engine. + const colSpan = cellMeasure.colSpan ?? 1; + const gridStart = cellMeasure.gridColumnStart ?? cellIndex; + let computedCellWidth = 0; + for (let i = gridStart; i < gridStart + colSpan && i < columnWidths.length; i++) { + computedCellWidth += columnWidths[i]; + } + // Never use default borders - cells are either explicitly styled or borderless // This prevents gray borders on cells with borders={} (intentionally borderless) const { cellElement } = renderTableCell({ @@ -342,6 +352,7 @@ export const renderTableRow = (deps: TableRowRenderDependencies): void => { fromLine, toLine, tableIndent, + cellWidth: computedCellWidth > 0 ? computedCellWidth : undefined, }); container.appendChild(cellElement); diff --git a/packages/layout-engine/pm-adapter/src/converters/table.test.ts b/packages/layout-engine/pm-adapter/src/converters/table.test.ts index 4d9afab225..db6ba705e9 100644 --- a/packages/layout-engine/pm-adapter/src/converters/table.test.ts +++ b/packages/layout-engine/pm-adapter/src/converters/table.test.ts @@ -1414,7 +1414,7 @@ describe('table converter', () => { expect(tableBlock.columnWidths).toEqual([100, 150]); }); - it('Priority 2/3 interplay: should prefer colwidth when userEdited is false even if grid present', () => { + it('Priority 2/3 interplay: should prefer grid over colwidth when userEdited is false', () => { const node: PMNode = { type: 'table', attrs: { @@ -1457,13 +1457,15 @@ describe('table converter', () => { const tableBlock = result as TableBlock; // When userEdited is false and both grid and colwidth are present, - // colwidth (Priority 2) takes precedence over grid (Priority 3) + // grid (Priority 2) takes precedence over colwidth (Priority 3). + // Grid values represent actual column positions and sum to the page width. expect(tableBlock.columnWidths).toBeDefined(); expect(tableBlock.columnWidths).toHaveLength(2); - expect(tableBlock.columnWidths).toEqual([50, 100]); + // 1440 twips = 96px, 2880 twips = 192px + expect(tableBlock.columnWidths).toEqual([96, 192]); }); - it('Priority 3: should use grid when no colwidth present', () => { + it('Priority 2: should use grid when no colwidth present', () => { const node: PMNode = { type: 'table', attrs: { @@ -1507,7 +1509,7 @@ describe('table converter', () => { expect(tableBlock.columnWidths![1]).toBeCloseTo(192, 1); }); - it('Priority 4: should auto-calculate when no width attributes', () => { + it('Priority 4: should leave columnWidths undefined when no width attributes', () => { const node: PMNode = { type: 'table', attrs: { diff --git a/packages/layout-engine/pm-adapter/src/converters/table.ts b/packages/layout-engine/pm-adapter/src/converters/table.ts index 00e944e4fe..f4d20c2760 100644 --- a/packages/layout-engine/pm-adapter/src/converters/table.ts +++ b/packages/layout-engine/pm-adapter/src/converters/table.ts @@ -782,15 +782,16 @@ export function tableNodeToBlock( }; /** - * Column width priority hierarchy (per plan Phase 3): + * Column width priority hierarchy: * 1. User-edited grid (userEdited flag + grid attribute) - * 2. PM colwidth attributes (fallback for PM-native edits) - * 3. Original OOXML grid (untouched documents) + * 2. Original OOXML grid (untouched documents — grid values sum to page width) + * 3. PM colwidth attributes (fallback for PM-native edits or missing grid) * 4. Auto-calculate from content (no explicit widths) * - * When both grid and colwidth are present: - * - If userEdited=true: use grid (Priority 1) - * - Otherwise: use colwidth (Priority 2) over grid (Priority 3) + * Grid values (from w:tblGrid) represent actual column positions on the page and + * sum to exactly the content width. Cell colwidth values may be scaled up from tcW + * (cell width hints) during import and require down-scaling in the measuring code, + * which introduces proportion changes that make columns narrower than they should be. */ // Priority 1: User-edited grid (preserves resize operations) @@ -811,7 +812,22 @@ export function tableNodeToBlock( } } - // Priority 2: PM colwidth attributes (higher priority than grid when userEdited !== true) + // Priority 2: Original OOXML grid (grid values are authoritative for column positions) + if (!columnWidths && Array.isArray(node.attrs?.grid) && node.attrs.grid.length > 0) { + columnWidths = (node.attrs.grid as Array<{ col?: number } | null | undefined>) + .filter((col): col is { col?: number } => col != null && typeof col === 'object') + .map((col) => { + const twips = typeof col.col === 'number' ? col.col : 0; + return twips > 0 ? twipsToPixels(twips) : 0; + }) + .filter((width: number) => width > 0); + + if (columnWidths.length === 0) { + columnWidths = undefined; + } + } + + // Priority 3: PM colwidth attributes (fallback when no grid is available) if (!columnWidths && Array.isArray(node.content) && node.content.length > 0) { const firstRow = node.content[0]; if (firstRow && isTableRowNode(firstRow) && Array.isArray(firstRow.content) && firstRow.content.length > 0) { @@ -832,21 +848,6 @@ export function tableNodeToBlock( } } - // Priority 3: Original OOXML grid (fallback when no colwidth) - if (!columnWidths && Array.isArray(node.attrs?.grid) && node.attrs.grid.length > 0) { - columnWidths = (node.attrs.grid as Array<{ col?: number } | null | undefined>) - .filter((col): col is { col?: number } => col != null && typeof col === 'object') - .map((col) => { - const twips = typeof col.col === 'number' ? col.col : 0; - return twips > 0 ? twipsToPixels(twips) : 0; - }) - .filter((width: number) => width > 0); - - if (columnWidths.length === 0) { - columnWidths = undefined; - } - } - // Priority 4: Auto-calculate from content (columnWidths remains undefined) // Extract floating table anchor/wrap properties diff --git a/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.js b/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.js index 6ad889fbbc..00ccd33189 100644 --- a/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.js +++ b/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.js @@ -70,6 +70,7 @@ export function handleTableCellNode({ } if (widthType) attributes['widthType'] = widthType; + const cellOwnWidth = width; // tcW-derived width (before grid fallback) if (!width && columnWidth) width = columnWidth; if (width) { attributes['colwidth'] = [width]; @@ -80,13 +81,22 @@ export function handleTableCellNode({ if (colspan > 1 && hasDefaultColWidths) { let colwidth = []; + // When cell has its own tcW width that exceeds the grid span total, + // distribute tcW proportionally across grid columns to match Word behavior. + // Only scale UP (tcW > grid), not down — smaller tcW is just a minimum. + const gridSpanTotal = defaultColWidths + .slice(columnIndex, columnIndex + colspan) + .reduce((sum, w) => sum + (w || 0), 0); + const shouldScale = cellOwnWidth && gridSpanTotal > 0 && cellOwnWidth > gridSpanTotal + 1; for (let i = 0; i < colspan; i++) { let colwidthValue = defaultColWidths[columnIndex + i]; let defaultColwidth = 100; if (typeof colwidthValue !== 'undefined') { - colwidth.push(colwidthValue); + colwidth.push( + shouldScale ? Math.round(colwidthValue * (cellOwnWidth / gridSpanTotal) * 1000) / 1000 : colwidthValue, + ); } else { colwidth.push(defaultColwidth); } diff --git a/packages/super-editor/src/tests/data/pci-table.docx b/packages/super-editor/src/tests/data/pci-table.docx new file mode 100644 index 0000000000..6668f2fb18 Binary files /dev/null and b/packages/super-editor/src/tests/data/pci-table.docx differ diff --git a/packages/super-editor/src/tests/data/table-autofit-colspan.docx b/packages/super-editor/src/tests/data/table-autofit-colspan.docx new file mode 100644 index 0000000000..6668f2fb18 Binary files /dev/null and b/packages/super-editor/src/tests/data/table-autofit-colspan.docx differ diff --git a/packages/super-editor/src/tests/data/table.docx b/packages/super-editor/src/tests/data/table.docx new file mode 100644 index 0000000000..677a0fa25b Binary files /dev/null and b/packages/super-editor/src/tests/data/table.docx differ diff --git a/packages/super-editor/src/tests/regression/table-autofit-colspan.test.js b/packages/super-editor/src/tests/regression/table-autofit-colspan.test.js new file mode 100644 index 0000000000..611561bb68 --- /dev/null +++ b/packages/super-editor/src/tests/regression/table-autofit-colspan.test.js @@ -0,0 +1,57 @@ +import { describe, it, expect } from 'vitest'; +import { initTestEditor, loadTestDataForEditorTests } from '@tests/helpers/helpers.js'; + +const findFirstTable = (doc) => { + let tableNode = null; + doc.descendants((node) => { + if (!tableNode && node.type?.name === 'table') { + tableNode = node; + return false; + } + return true; + }); + return tableNode; +}; + +describe('SD-1797: autofit tables with colspan should not drop columns', () => { + it('preserves all grid columns when rows use colspan patterns', async () => { + const { docx, media, mediaFiles, fonts } = await loadTestDataForEditorTests('table-autofit-colspan.docx'); + const { editor } = await initTestEditor({ content: docx, media, mediaFiles, fonts, isHeadless: true }); + + try { + const table = findFirstTable(editor.state.doc); + expect(table).toBeDefined(); + + // The table has a 4-column grid + const grid = Array.isArray(table.attrs?.grid) ? table.attrs.grid : []; + expect(grid.length).toBe(4); + + // Verify no row has more than 3 physical cells + // (this is the condition that triggers the bug — physical cells < grid columns) + let maxPhysicalCells = 0; + table.forEach((row) => { + let cellCount = 0; + row.forEach(() => { + cellCount++; + }); + maxPhysicalCells = Math.max(maxPhysicalCells, cellCount); + }); + expect(maxPhysicalCells).toBeLessThan(grid.length); + + // The key assertion: all cells should have valid colwidth arrays with positive values + // If the bug is present, cells in the last grid column would be missing or have zero width + let allColwidthsValid = true; + table.forEach((row) => { + row.forEach((cell) => { + const colwidth = cell.attrs?.colwidth; + if (!colwidth || !Array.isArray(colwidth) || colwidth.some((w) => w <= 0)) { + allColwidthsValid = false; + } + }); + }); + expect(allColwidthsValid).toBe(true); + } finally { + editor.destroy(); + } + }); +}); diff --git a/tests/visual/tests/rendering/table-fixes.spec.ts b/tests/visual/tests/rendering/table-fixes.spec.ts new file mode 100644 index 0000000000..9436774809 --- /dev/null +++ b/tests/visual/tests/rendering/table-fixes.spec.ts @@ -0,0 +1,35 @@ +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { test } from '../fixtures/superdoc.js'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const DOCS_DIR = path.resolve(__dirname, '../../test-data/rendering'); + +const SD_1859_PATH = path.join(DOCS_DIR, 'SD-1859-mixed-orientation.docx'); +const SD_1895_PATH = path.join(DOCS_DIR, 'SD-1895-autofit-issue.docx'); +const SD_1797_PATH = path.join(DOCS_DIR, 'table-autofit-colspan.docx'); + +// SD-1859: Percent-width table in mixed portrait/landscape document +// Table measured at landscape width but rendered in portrait — cells should not overflow +test('@rendering SD-1859 percent-width table fits within portrait page bounds', async ({ superdoc }) => { + test.skip(!fs.existsSync(SD_1859_PATH), 'Test document not available'); + await superdoc.loadDocument(SD_1859_PATH); + await superdoc.screenshotPages('rendering/sd-1859-mixed-orientation'); +}); + +// SD-1895: Auto-layout table from DOCX should fill page width +// Grid columns should scale up proportionally to fill available content area +test('@rendering SD-1895 autofit table fills page width', async ({ superdoc }) => { + test.skip(!fs.existsSync(SD_1895_PATH), 'Test document not available'); + await superdoc.loadDocument(SD_1895_PATH); + await superdoc.screenshotPages('rendering/sd-1895-autofit-table'); +}); + +// SD-1797: Autofit table with colspan should not drop columns +// Rows with rowspan continuations have fewer physical cells — all grid columns must be preserved +test('@rendering SD-1797 autofit table with colspan preserves all columns', async ({ superdoc }) => { + test.skip(!fs.existsSync(SD_1797_PATH), 'Test document not available'); + await superdoc.loadDocument(SD_1797_PATH); + await superdoc.screenshotPages('rendering/sd-1797-autofit-colspan'); +});