Skip to content

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

Open
JSv4 wants to merge 2 commits into
mainfrom
claude/pr-808-colspan-split
Open

JSv4 wants to merge 2 commits into
mainfrom
claude/pr-808-colspan-split

Conversation

@JSv4

@JSv4 JSv4 commented Sep 23, 2026

Copy link
Copy Markdown
Owner

Supersedes #808 (from a fork, so review changes couldn't be pushed there). Fixes #807. The contributor's commit is carried unchanged with its authorship; one review commit is added on top.

The problem

In the paginated view, a table taller than one page is split between rows so it can flow across pages. The splitter refused any table containing a colspan. A refused table falls back to a single page box that clips its overflow, so every row past the first page-height is rendered but invisible. The shape this hurts most is common in legal documents: a long table whose section-header rows span every column (Word's w:gridSpan). The reporter's document hid 17 of its 25 rows. Word shows it across 6 pages.

Why the change is safe

A colspan merges cells side by side within one row. The splitter only cuts between rows and copies whole rows into each page fragment, so it can never cut through a colspan. Vertical merges (rowspan) genuinely can span a cut, so tables with them still take the old fallback, along with nested tables and footnotes. Each fragment is still measured, and the old fallback is used if any fragment doesn't fit.

Review findings

  • Column alignment across page fragments. I checked the one real risk: a continuation fragment whose first row is a full-width header row. I used a probe on real converter output (every row but one in seven made full-width, with a page break forced onto a spanning row).
    • Tables with the <colgroup> the converter emits from Word's column grid kept identical column widths in every fragment.
    • Only a fixed-layout table with no colgroup lost its column widths in that fragment (columns went equal-width). The converter omits the colgroup only when a grid column has a missing or zero width, and the old behaviour for such a table was hiding rows, so this isn't a blocker. It's noted here rather than guarded against, because a guard would bring the clipping back.
  • Test tightened. The new test only checked that some colspan survived. That would also pass if the header row were duplicated across fragments or rendered with the wrong span. It now asserts exactly one spanning cell, spanning every column.
  • CHANGELOG. Added the missing ### Fixed entry, which the repo requires for bug fixes.

Validation

  • TDD check: with [colspan] put back into the reject selector, the new test fails (1 table fragment where more than 1 is expected). With the fix it passes. The rowspan and plain-table tests pass either way.
  • All 76 pagination-related Playwright tests pass locally (Chromium): the pagination groups in docxodus.spec.ts plus every pagination-*.spec.ts.
  • tsc --noEmit is clean.
  • fix(pagination): split oversized tables whose only merges are horizontal #808's own CI was green on every job.

🤖 Generated with Claude Code

https://claude.ai/code/session_01EuVfv22yFAxchW5H2F7Sk9


Generated by Claude Code

karlei-irving and others added 2 commits September 22, 2026 18:56
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 #807
… entry

The colspan test asserted only that some [colspan] survived, which would also
pass if the header row were duplicated across fragments or rendered with the
wrong span. It now asserts exactly one spanning cell, spanning every column.
Adds the missing CHANGELOG entry for issue #807.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EuVfv22yFAxchW5H2F7Sk9

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

3 participants