fix(doc-codec): cascade row/table-level borders onto cells with none - #1000
Draft
Mearman wants to merge 19 commits into
Draft
fix(doc-codec): cascade row/table-level borders onto cells with none#1000Mearman wants to merge 19 commits into
Mearman wants to merge 19 commits into
Conversation
Mearman
force-pushed
the
fix/doc-codec-row-table-border-read
branch
from
September 5, 2026 14:44
86ae82a to
ccef1b6
Compare
Adds readTableBordersOperand/readTableBordersOperand80, decoding the six-field row/table border cascade ([MS-DOC] 2.9.302/2.9.303: brcTop, brcLeft, brcBottom, brcRight, brcHorizontalInside, brcVerticalInside) into a TableBordersSet. Neither sprm's operand was previously read at all, so a table whose borders were stated only this way carried no border information downstream.
sprmTTableBorders/sprmTTableBorders80 state one border set for a whole row "unless modified by other Sprms applied to the cells" -- an explicit, order-independent fallback beneath TC80/sprmTSetBrc, unlike every other sgc-5 sprm this module folds in ordinary grpprl order. That precedence needs a cell's position in the whole table (its first/last row, its own first/last physical cell), which a single row's own grpprl never carries, so this only captures the row's own six-field operand on TableRowDefinition.rowBorders, unresolved; table/read.ts applies it once every row is known. sprmTSetShdTable's own text carries none of that exception, so its whole-row background is read as an ordinary sprm here, folded in grpprl order like every other shading sprm already in this module. sprmTCellShdStyle is deliberately not read: its own text and its neighbouring "Style"-suffixed sprms both place it inside a table STYLE's own definition, never a row's own direct-formatting grpprl this module walks, and this package does not read table styles at all.
A table decorated only through sprmTTableBorders/sprmTTableBorders80 -- the ordinary way ECMA-376's own tblBorders/tcBorders precedence applies a table-wide default that per-cell borders can override -- previously read with no cell borders at all, since only TC80 and sprmTSetBrc were ever resolved onto a cell's own borders. applyRowLevelBorderCascade fills in whichever cells and sides those leave undefined once every row of the table is known: brcTop/brcBottom only for the table's own first/last row, brcLeft/brcRight only for a row's own first/last physical cell (resolved through any trailing horzMerge continuation so a legacy-encoded merge's anchor still reaches the row's real edge), and brcHorizontalInside/brcVerticalInside everywhere else. A cell's own TC80/sprmTSetBrc border, once resolved, is never overridden by the cascade, matching [MS-DOC]'s own stated precedence for these sprms.
Removes the "Row- and table-level borders" scope-table row now that sprmTTableBorders/sprmTTableBorders80 and sprmTSetShdTable are read, adds a Cell decoration paragraph explaining the cascade and its precedence, and names sprmTCellShdStyle specifically as the one sprm from that same [MS-DOC] 2.6.3 neighbourhood still genuinely out of scope, since it belongs to a table style's own definition rather than a row's direct formatting.
… the row cascade applyBrcToCell folded a NilBrc-named side into cellBordersFrom, which drops any undefined side -- so a cell's own explicit "no border here" became byte-for-byte identical to a side the cell simply never mentioned, and the row/table-level border cascade (cascadeRowBorders) refilled it from sprmTTableBorders regardless. TableCellProperties/RawCell now carry a clearedSides set alongside borders, populated only by an explicit sprmTSetBrc NilBrc and threaded through every fold (sprmTMerge, sprmTVertMerge, shading). cascadeRowBorders consults it before falling back to the row's own cascade, so an explicitly cleared side stays absent no matter which of the table's first/last row or first/last cell it would otherwise inherit from.
…rapStyle alone [MS-DOC] 2.6.3's own text for sprmTCellVertAlignStyle (0x347C) is "A VerticalAlign value that specifies the vertical alignment of content within cells as defined by a Table style" -- no "MUST NOT appear outside" sentence anywhere in it. That restriction belongs to its other neighbour, sprmTCellNoWrapStyle (0x347D) alone: "This Sprm is used by table styles and MUST NOT appear outside of the grpprlTapx array of UpxTapx." The README and tap.ts's own comment both misattributed the sentence to both sprms together.
…ssue #945's own second paragraph flagged style-inherited formatting as a separate gap from the row/table-level border cascade this package now reads; the README's scope table named the gap but pointed nowhere. Link it to #1005, which tracks resolving a paragraph's/run's styleId against the style sheet's own grLPUpxSw grpprl on both the read and write sides.
…rmatting table row This row's last cell carried 136 extra trailing spaces before the closing pipe, growing the line past the table's other rows' 955-character width. eslint's own `--fix` pass silently corrects this at CI runtime, leaving a tracked file that fails a plain local `pnpm lint` run.
sprmTCellVertAlignStyle (0x347C) does not sit between sprmTCellNoWrapStyle (0x347D) and sprmTCellShdStyle (0xD687) in MS-DOC 2.6.3's own table: it precedes sprmTCellNoWrapStyle, which is itself several entries away from sprmTCellShdStyle once the sprmTCellBrc* entries are counted. Restate all three as simply listed in the same 2.6.3 table rather than as adjacent neighbours, in both table/tap.ts's own top-of-file note and the README's mirrored wording.
…jacency claim [MS-DOC] 2.6.3's own table lists sprmTSetShdTable at ispmd 0x60 and sprmTCellShdStyle at 0x87, with over twenty other sprms between them, not side by side as the reader/writer comment and README both claimed. State the real relationship -- both listed in the same 2.6.3 table -- matching the framing already used for the sprmTCellVertAlignStyle/ sprmTCellNoWrapStyle pair a few sentences later.
… paragraph never got The cascade paragraph still described the pre-25860a37 behaviour: an explicit sprmTSetBrc NilBrc clear folded indistinguishably into an unstated side, so the row/table cascade refilled it regardless. Since that commit, read.ts's cascadeRowBorders consults RawCell.clearedSides first and never re-fills a side it names, leaving only TC80's own mandatory Brc80 fields genuinely ambiguous. Restate the paragraph to match what read.ts:209 and tap.ts's applyBrcToCell actually do.
…ar path The SPRM_T_* constants in tap.ts covered every table sprm through sprmTSetBrc (0xD62F) but skipped sprmTSetBrc80 (0xD620) entirely, so this Word 97-era per-cell border override never reached applyBrcToCell. Two consequences: an explicit NilBrc80 clear on a Word-97-era file was silently refilled by the row/table cascade instead of staying absent, and a real per-cell border override stated only through sprmTSetBrc80 was discarded outright, with the cascade's own border used in its place -- worse than losing it, since the cell reads with a border it never asked for. Add a case for sprmTSetBrc80 that reads its operand's Brc80MayBeNil field with the already-imported readBrc80 and folds it through the identical applyBrcToCell/clearedSides path sprmTSetBrc already uses -- the two operands share the same cb/ItcFirstLim/bordersToApply header and differ only in which four-byte border field follows it.
…rmTSetBrc80 too The border-clear ambiguity paragraph named sprmTSetBrc as the only sprm whose explicit NilBrc clear is exempt from TC80's own mandatory-field ambiguity. sprmTSetBrc80's own NilBrc80 clear is exempt for the identical reason -- it is now read through the same applyBrcToCell/clearedSides path -- so restate both border-clear paragraphs, the borders overview, and the cell-decoration bullet to name the Word 97-era sibling alongside the modern one.
…-row sprmTSetShdTable's own [MS-DOC] 2.6.3 text says it specifies shading for "the entire table", but this reader applies it in tap.ts's per-row grpprl fold, to every cell of whichever row's own grpprl carries it -- not resolved across the whole table the way the sprm's name implies. A producer writing it on only one row's grpprl therefore shades, per this implementation, only that row. Restate the README and the SPRM_T_SET_SHD_TABLE constant's own doc comment to say so directly rather than hedging with "row/table" and "whole-table" wording that overstates the resolved scope.
…overclaim sprmTSetBrc80's operand is [MS-DOC] 2.9.304's TableBrc80Operand, not "TableBrcOperand80" -- the suffix sits in the wrong place. Fixed everywhere it was cited (tap.ts's module note and two inline operand comments, the README's cell-decoration paragraph, and a read.test.ts fixture comment); TableBordersOperand80 (2.9.303, sprmTTableBorders80's own operand) already carries the suffix correctly and is untouched. Also corrects an overclaim in the same border-cascade documentation: TC80's own byte-level ambiguity (a stated "no border" is indistinguishable from a side TC80 never touched) is a genuine format-level ambiguity, but it is not the only case the row/table-level cascade can silently get wrong. [MS-DOC] 2.6.3 also defines sprmTCellBrcType (0xD662) and the sprmTBrcTopCv/BrcLeftCv/BrcBottomCv/BrcRightCv family (0xD61A-0xD61D), each capable of making an unambiguous per-side border statement this reader does not act on -- a real reader gap, not a format ambiguity, named honestly in tap.ts and the README rather than folded into the TC80-alone claim.
…om border
applyRowLevelBorderCascade decided a cell's own bottom edge purely from
its physical row index (rowIndex === lastRowIndex), so a vertMerge
anchor spanning down to the table's actual last row -- but itself
sitting in an earlier, non-final row -- always got that row's own
insideHorizontal border on its bottom side instead of the table's real
bcBottom. The continuation cell that physically sits in the last row
would have resolved bcBottom correctly, but buildRows drops a
vertical-merge continuation's own decoration unconditionally (it is
`{blocks: []}` by the shared schema's own convention), so the table's
real bottom border never reached the output at all for that column.
The cascade now resolves the table's own shared column grid before
running (tryAssembleTable computes canonicalColumnBoundariesTwips
first), and cascadeRowBorders walks a cell's own vertMerge continuation
chain by grid position -- the identical matching buildRows' own rowSpan
computation performs -- to find which physical row the chain's bottom
edge actually falls in, mirroring how isRightmostPhysicalCell already
walks a horizontal merge's trailing continuations to find its real
right edge. The chain-walk itself (vertMergeChainLastRow) is shared
between the cascade's own bottom-edge check and buildRows' rowSpan
computation, rather than duplicated.
The row-border cascade paragraph still said brcBottom applies only to the table's own last physical row, the rule before the vertically-merged- anchor fix. A merge anchor sitting in an earlier row can now reach the table's real bottom edge through its own vertMerge continuation chain, resolved on the shared column grid by cellReachesTableBottom and vertMergeChainLastRow rather than the cell's own row index alone -- this paragraph now states that rule instead of the one it replaced.
Mearman
force-pushed
the
fix/doc-codec-row-table-border-read
branch
from
September 5, 2026 16:04
a6bf71a to
920aa4b
Compare
…erywhere it was described applyRowLevelBorderCascade's own doc comment, cascadeRowBorders' own doc comment, and tap.ts's own parenthetical still described brcBottom as landing only on the table's own last physical row -- the rule the round-6 fix (giving a vertically merged anchor the table's real bottom border) actually superseded. Restate all three to say what the code does: brcBottom lands on whichever cell's own bottom edge is the table's real bottom edge, which a vertically-merged anchor can reach through its own continuation chain from an earlier physical row. cascadeRowBorders' own comment additionally led with the superseded rule as its opening sentence and only corrected itself several sentences later; restructured so the opening sentence states the real rule directly, with the chain-walking mechanism explained immediately after rather than as a self-contradicting correction. Also adds the blank comment-separator line missing between applyRowLevelBorderCascade's own sprmTCellBrcType paragraph and its following note on why grid resolution runs before the cascade -- without it the second note read as a continuation of the unrelated paragraph above.
…bottom-border cascade The existing regression test for #945's follow-up fix used identical column boundaries in every row, so a cell's canonical grid index always equalled its raw physical-array index -- reverting cellReachesTableBottom's grid-based matching to plain physical-index matching would still have passed it. Adds a case where the table's last row states a genuinely different boundary array (a horizontal merge shifts the vertMerge target from physical index 2 to physical index 1), so the target's continuation is only found by walking the shared canonical grid, not by indexing rows positionally.
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
1 participant
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.
Summary
A table decorated only through the row/table-level border cascade --
sprmTTableBorders/sprmTTableBorders80, plussprmTSetShdTablefor background -- used to read with no cell borders at all, since neither sprm was read. This reads them and cascades their values down to each physical cell's own borders/background, with a cell's own explicitTC80/sprmTSetBrc/sprmTSetBrc80border always taking precedence, matchingsprmTTableBorders's own stated rule ("specifies the borders for this row unless modified by other Sprms applied to the cells").table/decoration.ts:readTableBordersOperand/readTableBordersOperand80parse the six-fieldTableBordersOperand([MS-DOC] 2.9.302)/TableBordersOperand80(2.9.303) byte layout --brcTop,brcLeft,brcBottom,brcRight,brcHorizontalInside,brcVerticalInside.table/tap.ts: captures a row's own cascade unresolved onTableRowDefinition.rowBorders(resolving it needs a cell's position in the whole table, which a single row's grpprl never carries), readssprmTSetShdTableas an ordinary per-row shading sprm (applied to every cell of whichever row states it -- its own text carries none ofsprmTTableBorders's "unless modified" exception, and it is not resolved across the whole table the way its name implies), and reads an explicitsprmTSetBrc/sprmTSetBrc80NilBrc(80)clear onto each cell's ownclearedSidesrather than folding it indistinguishably intoborders-- an explicit "this cell has no border here" is a real, out-of-band statement the row-level cascade must never re-fill.table/read.ts:applyRowLevelBorderCascade, run once every row of the table (and the table's own shared column grid) is known, fills in whichever cells/sidesTC80/sprmTSetBrc/sprmTSetBrc80left unstated andclearedSidesdoes not name --brcTop/brcBottomonly for the table's own first/last row,brcLeft/brcRightonly for a row's own first/last physical cell,brcHorizontalInside/brcVerticalInsideeverywhere else -- the sametblBorders/tcBordersprecedence [ECMA-376] uses, which [MS-DOC]'s own Overview of Tables defers to. A vertically merged anchor's own bottom edge is resolved through its full vertMerge continuation chain (matched by grid position, the same matchingrowSpanalready uses) rather than the anchor's own physical row alone, so an anchor sitting in a non-final row still gets the table's real bottom border when its merge reaches the table's actual last row -- mirroring how a horizontally merged anchor's right edge is already resolved through its own trailinghorzMergecontinuations.Two genuine, narrow gaps are documented rather than guessed at.
TC80's ownBrc80fields are mandatory for every cell, so a cell whose ownTC80states "no border" on a side is byte-for-byte indistinguishable from a cell that never had that side stated at all -- the cascade fills it in either way, which is correct for the common case this issue is about, and is a genuine ambiguity in the format itself. Separately, and not a format ambiguity at all: [MS-DOC] 2.6.3 also definessprmTCellBrcType(0xD662) and thesprmTBrcTopCv/BrcLeftCv/BrcBottomCv/BrcRightCvfamily (0xD61A-0xD61D), each capable of making an unambiguous per-side border statement this reader does not yet act on, so a producer using either has that statement silently overridden by the cascade.Verification
pnpm exec turbo run _lint _typecheck _build _test _test:workers _test:smoke --filter=doc-codec-- all clean, 263/263 tests passing.TableBordersOperand/TableBordersOperand80parsing (table/decoration.test.ts); end-to-endreadDocContentcascade behaviour (table/read.test.ts) -- all six sides mapped correctly across a 2x2 table, a cell's ownTC80/sprmTSetBrc/sprmTSetBrc80border winning over the cascade regardless of grpprl order, the Word 97-eraTableBordersOperand80spelling,sprmTSetShdTable's whole-row background, a latersprmTDefTableShdoverriding an earliersprmTSetShdTable, an explicitsprmTSetBrc/sprmTSetBrc80clear surviving the cascade, and a vertically merged anchor getting the table's real bottom border when its merge chain -- not its own physical row -- reaches the table's last row.clearedSidesmechanism, and both remaining gaps above.Fixes #945