fix(pagination): split oversized tables whose only merges are horizontal - #808
Open
karlei-irving wants to merge 1 commit into
Open
karlei-irving wants to merge 1 commit into
karlei-irving wants to merge 1 commit into
Conversation
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #807.
The problem
trySplitSimpleOversizedTablerejects any table containing[colspan]: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.
createSimpleTableFragmentalready 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:HW002-Table14.docxfixture to a full-width header row).Validation
tsc --noEmitclean.pagination.bundle.jsfrom 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.