Repository navigation
test(docx): hold a composed table to the row that holds it, and record the measurement - #992
Merged
Merged
Conversation
…d the measurement A block the layout places on one page is held in Word by two writers that know nothing of each other: keepOnOnePage chains the block's paragraphs, so each row of a table in it stays on the page of the row under it, and breakRowsWhereTheLayoutDoes gives every row the layout placed w:cantSplit, so no row breaks across the page mid-line. Neither alone is enough. A table composed in a cell is reached by neither. The chain reads a cell's own paragraphs, which are not those of a table in that cell, and the layout places no row for such a table, so the table writer gives its rows nothing. What holds it is the row it stands in. Nothing in the backend's tests asserted anything about a composed table's paragraphs, so neither the absence nor the reason it costs nothing was written down anywhere. aTableComposedInACellIsHeldByTheRowRoundIt asserts the two absences together with that row, so the reason stands beside the gap, and names where it would begin to cost something: a row the layout did not place, which carries no w:cantSplit of its own. aTableInsideAKeptBlockIsChainedRowByRow asked only for the chain half of the pair, where the keepWithNext path was already held to both (DocxOneRowTableKeptWithTheNextTest). It now asks for both. The recipe and the capability matrix said that the keep chains a cell's own paragraphs and not the rows of a table in it, and left it there. They now finish it — the table writer holds those rows, a composed table is held by the row round it — and carry the measurement: nine shapes at the foot of a page, in two positions each, eighteen documents, each whole in Word 16.0.20527 and LibreOffice 24.2 with no group and no row broken. The editors differ only over which page holds the whole block: LibreOffice sets rows taller, a three-row table's 33.9pt apart against the page's 21.0, and moved it to the next page for seven of the nine that fit at the foot; Word did the same for the band of two layers. No engine change: every byte of every exported document is as it was.
… row-pitch attribution Three corrections to the sentences this change adds. The rule "every row the layout placed carries w:cantSplit" was stated unconditionally, with its exceptions sitting a few sentences away in the same documents: letTheOneRowTablesKeptWithTheNextBreak takes it back off a one-row body table of paragraphs kept with what follows, and the gate in writePanelPiece never writes it for a panel asking only to be kept with the next. Both exceptions are now in the sentence, and the 56 one-row tables of the corpus that carry keeps without it are named in all three documents rather than only in the pull request. The reason given for w:cantSplit was "it keeps a row of several paragraphs from breaking mid-line". A cell of a plain table cannot hold several paragraphs - lines in a cell are one paragraph with line breaks - so the shape named was unreachable, and inside a kept block the chain's w:keepLines already holds a single paragraph's lines together. What w:cantSplit adds is a row the chain cannot reach inside: a cell of more than one paragraph or of a table, which only a composed cell can hold, and any table outside a kept block, where no chain is written at all. The measured shape is renamed from several paragraphs to several lines, which is what the fixture held. The 33.9pt against 21.0 row pitch was measured on one shape, a three-row table, and was made the cause of all seven LibreOffice page moves. It explains six: the band of two layers holds no such table, and both editors moved that one, from a cause not measured. Also: keep-with-next and the table-cell case added to the w:keepNext description, writePanelPiece named as one of three setCantSplitRow sites, and the cross-reference in the recipe pointed above rather than below.
…hrough its line spacing A plain table cell is a paragraph per line, not one paragraph of several lines, when its style sets a line spacing and it holds more than one line (paragraphPerLine in writeCellLines, which DocxCellSeatTest pins, and which the recipe's Tables row already stated). The sentences added here said that only a composed cell can hold several paragraphs, and so left out the one shape on an authored table for which w:cantSplit is load-bearing: the chain gives each of a row's paragraphs w:keepLines, which holds that paragraph's own lines, and w:keepNext only where the row is not the block's last unit, so nothing but the row itself keeps two paragraphs of one row on a page. aRowOfSeveralParagraphsIsHeldWholeByItsRow builds that row, asserts the premise that the line spacing makes it two paragraphs, and then asserts that only the row holds them. A fourth sabotage takes w:cantSplit off a placed row and requires this fixture to fail. Two further sentences narrowed to what the code does. The panel gate is not "a panel asking only to be kept with the next never gets w:cantSplit": a row and a shape container pass its instanceof disjuncts whatever they ask, so the block that goes without it is a painted section. And "nothing asserted anything about a composed table's paragraphs" was too wide - DocxComposedCellTest asserts their text; it is their keeps that nothing asserted. Also: the band's page move is no longer implied to be absent from the corpus, which a static classification cannot establish; the count of unclassified blocks states its denominator; the added cantSplit loop on the keepTogether path is described as documenting the pair at the call site rather than as closing a gap, since breakRowsWhereTheLayoutDoes is blind to the keep flags; and the ninth measured shape is named with its table in all four places.
…ule for it No one rule says which rows carry w:cantSplit, and the sentences added here tried twice to give one. Three writers decide it between them: breakRowsWhereTheLayoutDoes gives it to every row it lays out; writePanelPiece gives a block written as a panel its own row where it asks to be kept together, or is a row or a shape container the layout put on one page; and letTheOneRowTablesKeptWithTheNextBreak then takes it off a one-row body table of paragraphs all kept with the next, whichever writer put it there. Two existing fixtures refute the rule that was written instead. A painted section asking nothing at all also goes without it (DocxContainerPaintTest.aCardKeptTogetherOnOnePageIsARowThatDoesNotSplit asserts both sides), so "a painted section asking only to be kept with the next is the one block whose own row goes without it" named neither the gate nor the set. And a row asking only to be kept with the next ends up with no w:cantSplit in the file at all (DocxOneRowTableKeptWithTheNextTest.aRowKeptWithTheNextMayBreakAndHoldsOntoIt asserts the element is absent), because the third writer takes back what the gate gave, so "a row or a shape container getting it whatever it asks" was false of the export. The recipe also claimed the 56 one-row tables carrying keeps without w:cantSplit "were measured whole in both editors". They were not: the measurement is nine shapes, of which one has that gate's shape. The clause is gone, and the set is no longer described as panels and containers kept with the next - a one-row body table of paragraphs is neither. aRowOfSeveralParagraphsIsHeldWholeByItsRow is scoped to what it shows: w:cantSplit is the only thing holding such a row inside a kept block, since outside one no chain is written at all and it holds every row. It now asserts both premises before the rule - that the line spacing made two paragraphs, and that the chain ran in the body - and walks both of the row's paragraphs rather than the first. Also: LibreOffice moved seven of the nine, not six, the recipe having made the row-pitch count the editor's whole total; and one CHANGELOG line re-wrapped to the file's width.
…al conditions Four writers decide whether a row carries w:cantSplit, not three. The account named breakRowsWhereTheLayoutDoes, writePanelPiece and the pass that takes the flag back, and left out writeRow, which gives it to a row written as a table wherever the layout placed that row. That omission made the commonest case in the backend unexplainable: a plain unpainted addRow carries w:cantSplit, and it reaches neither of the two writers the account named - breakRowsWhereTheLayoutDoes sees only a table's rows, and writePanelPiece is reached only through writePaintedRow, which runs only when the row paints. It is also why a row asking only to be kept with the next has no w:cantSplit in the file at all: writeRow gave it one and the removal pass took it back, the panel gate never having run. Three conditions stated more narrowly than the code: - layout.onOnePage gates all three disjuncts of the panel gate, not just the two type tests, so a painted section kept together but taller than a page gets no w:cantSplit either. - A plain cell is a paragraph a line only where its style sets a line spacing AND it holds more than one line; with one line it stays one paragraph. - A block in a table cell is not left without a chain: it gets w:keepLines on every paragraph, and only w:keepNext is withheld. What keeps a table in a cell from being freed is that the removal pass reads the body's tables alone. The local behind the paragraph-a-line rule lives in writeCellContent; writeCellLines, named in the new fixture's javadoc, does not exist. The row-height figure is scoped to the shape it was read off - a three-row table, 33.9pt against the page's 21.0 - rather than offered as the cause of all six moves it covers; the count of 56 one-row tables carrying keeps without w:cantSplit is given as a count, with the shapes that reach it named and no apportionment claimed; and the corpus classification states its denominator again.
… the panel split aTableComposedInACellIsHeldByTheRowRoundIt asserted that a composed table's paragraphs carry no keep, with the surrounding row's w:cantSplit beside it as the reason that costs nothing. But that flag comes from layout.placedRow, not from the chain, so it did not show that keepOnOnePage had run: a regression that stopped the chain would have left both absences green. The fixture now asserts the chain ran, and ran in the body, as its sibling does, and a sabotage that stops keepOnOnePage writing w:keepNext in the body requires it to fail. w:cantSplit has a fifth source the account did not name. After the four writers have decided, a panel the layout breaks over pages is copied whole by DocxTableSplit.splitInItsPanel, which uses copyXml: the second part's panel row and the copied header rows of its inner table carry whatever the first part's were given, judged by no gate of their own. Two sentences narrowed. A row of several paragraphs is the shape a PLAIN table reaches for which w:cantSplit alone holds the row inside a kept block; a cell holding a table is the other, and the fixture for it sits in the same file. And outside a kept block it is keepOnOnePage that writes nothing, not the export - breakRowsWhereTheLayoutDoes writes w:keepNext itself on a repeated header row. The Verification section now says which of the documented shapes the two editors were never shown: shape 2 of the nine set no line spacing, so its cell was one paragraph, and the multi-paragraph row the new fixture builds is read off the written XML alone, as is the keepWithNext boundary beside it. Also: the per-row gate described per row rather than per table, the composed table said to be held by neither the chain nor the table writer rather than by "neither writer", and the unplaced-row citation given as a document the layout refused rather than a table too tall.
The split copy was described on the wrong side of the pass that frees a one-row table, and as reaching no gate at all. Both are wrong. cutThePanelsAtTheirPages runs at DocxSemanticBackend:1408 and letTheOneRowTablesKeptWithTheNextBreak at :1411, and copyAfter ends with insertTable, so the copied panel is in document.getTables() when the pass runs: its row is judged, and survives only because the gate frees cells of paragraphs and a panel's cell holds a table. What no gate reaches is the copied rows of that inner table, the pass reading the body's tables alone. The copy is also not the panel cut's alone - DocxPageMarginSections splits a table the same way - so the account no longer counts the copiers. It says what a split copy carries and where the panel cut sits relative to the pass, and leaves it there. "Outside a kept block w:cantSplit holds every row" is bounded to every row the layout placed: a document the layout refuses leaves no row placed, and DocxTablePaginationTest asserts the flag absent on all of them. The repeated-header carve-out is given its own condition too - the table writer writes that w:keepNext where the table has body rows under its header. The two shapes w:cantSplit alone holds inside a kept block now name the composed cell beside the plain one, as the recipe already did: a cell of several paragraphs is what a plain cell becomes under a line spacing and what a composed cell is given. The matrix's first mention of the removal pass gains the two qualifiers its later one carries, body and all. Also: the premise on the row count gains a description, and three paragraphs reflowed.
The export splits a table in two places, not one: cutThePanelsAtTheirPages cuts a panel at its pages, and DocxPageMarginSections cuts a body table for a page's own margins. Both run before the pass that frees a one-row table, so it sees both copies — but the reason a panel's copy survives is its cell's table, and a margin-split copy's cells hold paragraphs, so that reason was being generalised to a copy it does not cover. The account now names both cuts and gives each its own verdict: a panel's row left alone, its inner table's copied rows never reached, and a body table's copy judged like any other. theSameRowKeptWithTheNextIsHeldByNothing pins the boundary that until now only a javadoc recorded. Kept with the next, the block's last unit takes w:keepNext too, which makes its table one the removal pass frees, so w:cantSplit comes off the row that was the only thing holding its two paragraphs together. A sixth sabotage stops the pass freeing it and requires this fixture to fail; the javadoc beside it no longer restates the measurement it now asserts. Also: the removal pass's first mention in the recipe gains the qualifiers its later one carries, body and all; the 68 multi-row tables are said to be the corpus's; and the copied-flag sentences drop copyXml, which the matrix does not need.
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.
Why
A block the layout places on one page is held together in Word by two writers that know nothing
of each other:
keepOnOnePagechains the block's paragraphs —w:keepLineson each,w:keepNexton all butthe last (the last too where the block is
keepWithNext, and none of it inside a table cell) —so each row of a table in the block stays on the page of the row under it;
breakRowsWhereTheLayoutDoesgives every row the layout placedw:cantSplit, so no row breaksacross the page mid-line.
Neither covers the other. The chain reaches only a cell's own paragraphs, so what
w:cantSplitadds is a row Word cannot end a page inside — a cell of more than one paragraph, which a plain
cell is where its style sets a line spacing and it holds more than one line (
paragraphPerLine),or a cell of a table, or any table outside a kept block, where
keepOnOnePagewrites nothing atall (a repeated header row's own
w:keepNext, written by the table writer, aside). Andw:cantSplitalone would letWord end the page between two rows.
And no one rule says which rows carry
w:cantSplit— four writers decide which rows are givenit, the three
setCantSplitRowsites and the pass that takes it back:breakRowsWhereTheLayoutDoesgives it to each row of a table where the layout placed that row(
layout.placedRowis per row, so a placed table can hold an unplaced row);writeRowgives it to a row written as a table, where the layout placed that row;writePanelPiecegives a block written as a painted panel its own row where the layout putthe block on one page and
node.keepTogether() || node instanceof RowNode || node instanceof ShapeContainerNode— so a painted section asking nothing goes without it(
DocxContainerPaintTest.aCardKeptTogetherOnOnePageIsARowThatDoesNotSplitpins both sides ofthat gate). An unpainted row never reaches this writer at all:
writePaintedRowis the onlyroute in for a row, and it runs only when the row paints;
letTheOneRowTablesKeptWithTheNextBreakthen takes it off a one-row body table ofparagraphs all kept with the next, whichever of the three put it there, because LibreOffice
reads such a table as unsplittable and then keeps it with nothing. That is why a plain row
inside a section asking only to be kept with the next ends up with no
w:cantSplitin the fileat all: being unpainted it got its flag from
writeRow, and this pass took it back(
DocxOneRowTableKeptWithTheNextTest.aRowKeptWithTheNextMayBreakAndHoldsOntoItasserts theelement is absent).
Separately from all four: where the export splits a table (
DocxTableSplit) — a panel cut at itspages, or a body table cut for a page's own margins — the second part is copied with its rows'
flags, so a copied row carries what the original was given rather than anything decided for it.
Both cuts run before the removal pass in the finishing sequence (
DocxPageMarginSections.applyand
cutThePanelsAtTheirPages, thenletTheOneRowTablesKeptWithTheNextBreak), andDocxTableSplit.copyAfterends withinsertTable, so the pass does see each copy. A panel's rowit leaves alone — the gate frees only cells of paragraphs, and a panel's cell holds a table — and
the copied rows of that inner table it never reaches, reading
document.getTables()alone. A bodytable's copy it judges like any other.
56 one-row tables of the corpus carry keeps without
w:cantSplit. The causes were not apportionedbetween them — the count is a count.
A table composed in a cell is reached by neither. The chain reads a cell's own paragraphs,
which are not those of a table in that cell; and the layout places no row for a table composed in
a cell, so the table writer gives its rows nothing either. What holds it is the row it stands in.
Nothing in the backend's tests asserted anything about a composed table's keeps, so
neither the absence nor the reason it costs nothing was written down anywhere — and the recipe and
the matrix stopped at the half-sentence "the keep chains a cell's own paragraphs, not the rows of a
table in it", without saying what holds those rows instead.
On the
keepTogetherpath only the chain half of the pair was asked for, where thekeepWithNextpath was already held to both (
DocxOneRowTableKeptWithTheNextTest.aTableOfTwoRowsKeptWithTheNextStaysWhole).And the keep machinery as a whole had never been put to the two editors at the foot of a page,
which is the one place it can go wrong. (The multi-paragraph row in particular still has not —
see What was not measured below.)
What changed
No engine change — every byte of every exported document is as it was.
DocxKeepTogetherTest.aTableComposedInACellIsHeldByTheRowRoundIt(new) asserts the twoabsences — no keep on the composed table's paragraphs, no
w:cantSpliton its rows — togetherwith the row round it, which carries
w:cantSplitwhere the layout placed it. The reason thegap costs nothing stands beside the gap, and the javadoc names where it would begin to cost
something: a row the layout did not place, the shape
DocxTablePaginationTest.aTableTooTallToLayOutIsLeftFreeToBreakexports.DocxKeepTogetherTest.aRowOfSeveralParagraphsIsHeldWholeByItsRow(new) pins the shape aplain table reaches for which, inside a kept block,
w:cantSplitis the only thing holdingthe row: a cell of several paragraphs, which a plain cell is where its style sets a line spacing
and it holds more than one line. (The other such shape is a cell holding a table, which the
fixture above covers; outside a kept block
keepOnOnePagewrites nothing, so there it holdsevery row.) The chain gives
each of the row's paragraphs
w:keepLinesand — the row being the block's last unit — now:keepNext, so only the row itself keeps the two paragraphs on one page. The fixture assertsboth premises first: that the line spacing made two paragraphs, and that the chain ran in the
body at all. Its genuinely new assertion is
w:keepLineswith now:keepNexton amulti-paragraph cell inside a kept block; the
isCantSplitRow()beside it cannot fail whileDocxTablePaginationTestpasses, and its premise duplicatesDocxCellSeatTest's.DocxKeepTogetherTest.aTableInsideAKeptBlockIsChainedRowByRownow assertsw:cantSplitonevery row as well as the chain, so the
keepTogetherpath asks for the pair as thekeepWithNextpath already does. This documents the pair at the call site rather than closing agap:
breakRowsWhereTheLayoutDoesis blind to the keep flags, so these lines cannot fail whileDocxTablePaginationTest.aRowTheLayoutKeepsWholeIsKeptWholeInWordpasses. The new coverage inthis change is the other fixtures.
docs/recipes/docx-export.mdanddocs/architecture/backend-capability-matrix.mdfinishthat half-sentence and carry the measurement below, with the corpus counts behind it.
CHANGELOG.md— one entry under Tests, one under Documentation.Verification
The measurement. Nine shapes at the foot of a page, in two positions each, exported by the
engine, converted by Word 16.0.20527 and by LibreOffice 24.2, and read back for the page every
part of the block landed on. 18 of 18 stood whole — no group and no table row broken anywhere.
The editors never differed over whether the block was broken, only over which page held it whole;
they agreed with the layout on the page in 11 of the 18:
LibreOffice sets a table's rows taller — a three-row table's 33.9pt apart against the page's 21.0,
read off that one shape, the other five not measured apart — so a block whose table only just fits
at the foot of a page there no longer does and moves to the next one whole: six of the nine. The
band of two layers holds no table and moved in both editors, from a cause not measured — so
the row-height number covers six, not seven, and nothing here explains the band. In its fits-at-the-foot position that is the one shape of the nine whose page
neither editor agreed on; the block itself was whole in both, as all 18 were. What the corpus
classification can and cannot say about it: it counts 338 blocks holding a band or a container, so
the shape is common, but nothing in a static count establishes whether one of them sits at the
very foot of a page, which is the condition that produced the move.
What was not measured. Shape 2 of the nine, "kept block + one row of several lines", set no
line spacing, so by the rule above its cell is one paragraph — the editors were never shown a
row of two paragraphs.
aRowOfSeveralParagraphsIsHeldWholeByItsRowbuilds that row (with a linespacing) and is read off the export alone. So the shape this change calls the one a plain table
reaches for which
w:cantSplitis load-bearing is pinned in the file, not in Word or LibreOffice.The same holds for the
keepWithNextboundary below: both are read off the written XML.The corpus. All 71 documents of the fidelity corpus, every node of every document classified by
its type and its keep flags, and none of the 1116 it found left unclassified:
keepWithNext, 54 shape containers,43 layer stacks, 25
keepTogether), 902 in a cell or a row's column. Layer stacks and shapecontainers are in the count: they are held by
movedWhole(node)rather than by either flag.keepTogether; 17 in cells.on one page, and no row a keep chain reaches, in any of the corpus's 68 tables of more
than one
row, lacks
w:cantSplit.w:cantSplit, from the four writers in Why above.The count was not apportioned between them, and at least three shapes reach it — a painted
section asking nothing or only to be kept with the next, a one-row body table the removal pass
frees, and a row the layout placed nowhere, where
writeRow's own gate goes false.and it stood whole in both editors in both positions. Left as it is, deliberately: of the 20
corpus blocks that hold a table, the 3 in the body all ask to be kept together, and the other 17
are in a cell, where the chain writes
w:keepLinesalone and now:keepNext— and the removalpass reads the body's tables only (
document.getTables()), so a table in a cell is never freed.theSameRowKeptWithTheNextIsHeldByNothing: the same one-row table of several paragraphs, ina block asking only to be kept with the next, is held by nothing. Its paragraphs then carry
w:keepNexttoo, which makes it a tableletTheOneRowTablesKeptWithTheNextBreakfrees, sow:cantSplitcomes off — andw:keepLinesholds only each paragraph's own lines. Read off theexport for both flags on one document:
keepTogether→ 2 paragraphs,cantSplit=true, marks+L-N +L-N;keepWithNext→ 2 paragraphs,cantSplit=false, marks+L+N +L+N. No corpusdocument has that shape, so it is
recorded rather than fixed here.
Tests.
./mvnw -B -ntp test -Dtest=DocxKeepTogetherTest -pl :graph-compose-render-docx -am—Tests run: 18, Failures: 0, Errors: 0(15 before). Owning module from clean,./mvnw -B -ntp clean test -pl :graph-compose-render-docx -am— render-docx 2396, core 823,render-pdf 339, all green (
Tests run: 2396, Failures: 0, Errors: 0, Skipped: 1).Sabotages. Six, each requiring the named fixture to fail rather than the suite to redden, and
each reporting
Tests run: 1, Failures: 1. Rows 3 and 4 break the same production line and arenot independent catches — the second is there to show which fixture covers the new shape:
theSameRowKeptWithTheNextIsHeldByNothingw:keepNextin the bodyaTableComposedInACellIsHeldByTheRowRoundItw:cantSplitnot written on a placed rowaTableInsideAKeptBlockIsChainedRowByRowaRowOfSeveralParagraphsIsHeldWholeByItsRoww:cantSplitwritten on every row, placed or notaTableComposedInACellIsHeldByTheRowRoundItaTableComposedInACellIsHeldByTheRowRoundItThe first is what the two fixtures' "the chain ran, and ran in the body" premises are for: without
them a regression that stopped the chain would leave both absence assertions green.
The script copies each file it touches before its first edit, restores from that copy, and then
checks the bytes against what that run itself saved:
DocxSemanticBackend.javacame backd8959d6dcb71, the digest taken at the start of the run.Gate. Full reactor
clean verifyas CI runs it, the four JDK 17 knowledge checks, core javadocat zero warnings, then install and the examples tests.