odf.js: read fo:border-* into ContentParagraph.borders - #1092
Merged
Conversation
Mearman
marked this pull request as ready for review
September 7, 2026 18:37
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Extracts the XSL-FO border-shorthand parser (parseBorderEdge, formatBorderEdge, and the shared BorderEdgeKey/BORDER_EDGE_* constants) out of typed/shared/table.ts into its own typed/shared/border.ts, since it was already shared between table-cell reading and ods sheet-cell writing before this change, and paragraph borders make it a third consumer. Adds fo:border-left/right/top/bottom (and the fo:border shorthand that seeds all four before a more specific per-edge attribute overrides one) as four flat borderLeft/Right/Top/Bottom fields on StyleProperties, deliberately not grouped into one nested object: the style cascade in cascade.ts folds a chain of StyleProperties results with a plain shallow spread, which is correct per-field for every other property here but would let a child style's borders replace a parent's wholesale rather than merge per-edge. A style-style token of "none"/"hidden" sets the field to an explicit undefined -- a real own-property, distinct from the field never having been touched -- so the cascade can override an inherited border away to nothing, not just add one. readOdfParagraph assembles the four flat fields into document-schema.js's nested ContentParagraphBordersSchema shape once, after cascade resolution has finished, closing the odt half of the border-only horizontal-rule detection #1082 built for docx.
…borders too odf.js now populates ContentParagraph.borders from fo:border-* (#1086), closing the odt-side gap isBorderOnlyHorizontalRule's own top-of-file note and WordProcessingPreview.tsx's comment both described as still open. No behavioural change here: the router-side shape match was already generic across formats, so it needed no code change once the reader started producing the shape -- only the comments describing that gap as open needed correcting.
Mearman
force-pushed
the
fix/1086-odf-paragraph-borders
branch
from
September 7, 2026 18:55
1dbd854 to
764e9d0
Compare
Contributor
|
🎉 This PR is included in version 2.7.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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.
Closes the odt half of #1082's own border-only horizontal-rule detection: odf.js's paragraph reader never read ODF's fo:border-* shorthand into ContentParagraph.borders, so a border-only paragraph built by LibreOffice without going through the named "Horizontal Line" style rendered as a plain paragraph in the web preview.
Extracted the XSL-FO border-shorthand parser (parseBorderEdge/formatBorderEdge and the BorderEdgeKey/BORDER_EDGE_* constants) out of typed/shared/table.ts into its own typed/shared/border.ts, since it was already shared between table-cell reading and ods sheet-cell writing before this change and paragraph borders make it a third consumer -- reusing it rather than writing a second parser for the identical grammar.
The four fo:border-left/right/top/bottom attributes (plus the fo:border shorthand that seeds all four before a more specific per-edge attribute overrides one) land as four flat borderLeft/Right/Top/Bottom fields on StyleProperties rather than one nested object. That's deliberate: cascade.ts's own style resolution folds a chain of StyleProperties results with a plain shallow spread, correct per-field for every other property here, but a single nested borders object would let a child style's borders wholesale-replace a parent's instead of merging per-edge -- a child overriding only its own bottom edge would silently wipe out an inherited left/right/top. Flat fields let the existing cascade already do the right thing for free. A style token of "none"/"hidden" sets the field to an explicit
undefined(a real own-property, not just an absent one), so the cascade can override an inherited border away to nothing, not just add one -- verified with a dedicated cascade test.readOdfParagraph assembles the four fields into the real nested ContentParagraphBordersSchema shape once, after cascade resolution finishes.
Also updated two web-side comments (router.ts, WordProcessingPreview.tsx) that described this as a still-open gap, now that it isn't, and renamed the router test that was already proving the shape-match logic is generic across formats.
Fixes #1086