Skip to content

fix(pagination): split oversized tables whose only merges are horizontal - #808

Open
karlei-irving wants to merge 1 commit into
JSv4:mainfrom
karlei-irving:fix/split-oversized-colspan-tables
Open

karlei-irving wants to merge 1 commit into
JSv4:mainfrom
karlei-irving:fix/split-oversized-colspan-tables

Conversation

@karlei-irving

Copy link
Copy Markdown

Fixes #807.

The problem

trySplitSimpleOversizedTable rejects any table containing [colspan]:

table.querySelector("table, [rowspan], [colspan], [data-footnote-id]")

A rejected oversized table takes the whole-block overflow fallback: all of its rows land in a single page box whose content area clips, so everything past the first page-height is rendered but invisible. The shape this hurts most is a legal/business staple — a multi-page table whose section-header rows span all columns (w:gridSpan). On the document that surfaced this (landscape page, 25-row table, 7 colspan header rows, zero rowspan), 17 rows were silently hidden; Word paginates the same file to 6 pages.

Why the colspan reject is unnecessary

A colspan is horizontal merging, contained entirely within a single row — and this splitter only ever cuts between rows, so a row-boundary split can never break one. createSimpleTableFragment already clones the colgroup and whole rows (row.cloneNode(true)), so colspans travel intact with their cells, and every fragment is still re-measured with the existing bail-out to the old behavior if any fragment doesn't fit.

[rowspan] stays rejected: a vertical merge genuinely can cross a row-boundary split, and cloning rows cannot make that correct.

Changes

  • npm/src/pagination.ts: drop [colspan] from the reject selector; update the fragment-builder doc comment to say which merges are unsafe (vertical) and why horizontal ones are fine.
  • npm/tests/docxodus.spec.ts:
    • New test: an oversized table whose only merges are horizontal splits at row boundaries, all rows preserved in order, colspan cells intact (built by widening a row of the existing HW002-Table14.docx fixture to a full-width header row).
    • The existing merged-cells regression test keeps its fixture and all assertions — its table carries both rowspan and colspan, and the rowspan alone keeps the conservative fallback — retitled to "vertically merged cells" to say what it now pins.

Validation

  • tsc --noEmit clean.
  • Built pagination.bundle.js from this branch and drove it headless (Chrome) over the real document's converted HTML: 6 pages (matching Word), 5 table fragments, all 25 rows present in source order, all 7 colspan cells intact.
  • We've been running the equivalent one-line change as a patch on the published 12.6.2 package in production use without regressions.

trySplitSimpleOversizedTable rejected any table containing [colspan],
sending it down the whole-block overflow fallback: every row lands in
one page box whose content clips, so everything past the first
page-height is rendered but invisible. A colspan is horizontal merging,
contained entirely within a single row — and this splitter only ever
cuts between rows, so a row-boundary split can never break one.
createSimpleTableFragment already clones the colgroup and whole rows,
so colspans travel intact with their cells.

The shape this hurts most is a legal/business staple: a multi-page
table whose section-header rows span all columns (w:gridSpan). On the
reporting document (landscape, 25 rows, 7 colspan header rows, zero
rowspan), 17 rows were silently hidden; Word paginates the same file
to 6 pages. With this change the engine produces 6 pages with every
row visible, verified against that document.

[rowspan] stays rejected — a vertical merge genuinely can cross a
row-boundary split. The existing merged-cells regression test keeps
its fixture and assertions (its table carries both rowspan and
colspan, and the rowspan alone keeps the conservative fallback); it is
retitled to say what it now pins. New test: a colspan-only oversized
table splits at row boundaries with all rows preserved.

Fixes JSv4#807

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pagination: oversized tables with colspan (but no rowspan) are clipped instead of split at row boundaries

1 participant