fix(super-editor): preserve w:tcMar logical/physical key family on round-trip (SD-3152)#3308
Conversation
…und-trip (SD-3152) Word-authored docx with <w:tcMar><w:start/><w:end/> was gaining duplicate <w:left/><w:right/> children on export. translate-table-cell.js merged attrs.cellMargins (physical LTR-default per SD-3134) into tableCellProperties.cellMargins without reconciling the imported pair, so the v3 tcMar decoder emitted both pairs. Per-side detection now writes the user-visible value back into the imported family: logical-only stays logical, physical-only stays physical, mixed-unchanged preserves both, mixed-edited normalizes to physical (mirrors getTableCellMargins import precedence). New cells default physical. Also sorts tcMar and tblCellMar children into ECMA-376 CT_TcMar / CT_TblCellMar sequence order (top, start, left, bottom, end, right) on decode. Existing tests used arrayContaining and didn't catch the order regression.
superdoc-bot
Bot
added
review: thorough
review: careful
and removed
review: thorough
labels
|
I was unable to invoke the ecma-spec MCP tools (permission was repeatedly denied for both Status: PASS The spec claims in this PR check out against ECMA-376 Part 1 §17.4 / §A.1 wml.xsd:
One minor caveat unrelated to spec compliance: the Note: I would recommend re-running this check with the ecma-spec MCP tools available, since I could not access them this session to verify directly. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…-family round-trip (SD-3152) Real Word-authored docx covering every CT_TcMar / CT_TblCellMar sibling the PR touches: tblCellMar with logical children (§17.4.42), cell 1 tcMar logical-only (§17.4.68 + §17.4.35 + §17.4.10), cell 2 tcMar physical-only (Part 4 §14.3.3 + §14.3.8). Fixture round-trips through Word without validation errors. Three test paths: - Unit fixture test: parses the docx, runs encode + generateTableCellProperties + decode, asserts logical-only stays logical-only, physical-only stays physical-only, schema-order on both translators. - Behavior spec: loads the fixture in headless SuperDoc and confirms logical and physical sources resolve to the same physical padding in an LTR table. - Visual: same fixture uploaded to R2 as rendering/sd-3152-tcmar-key-family.docx for pixel-diff baselines (auto-discovered).
…iew importer contract (SD-3152) Two gaps remained after the first test pass: 1. exportDocx wiring. The fixture-backed unit test exercised translators and generateTableCellProperties directly, but skipped the editor → exportDocx() pipeline. A wiring regression there would not surface. New behavior spec under tests/behavior/tests/exporting/ loads the fixture, calls editor.exportDocx, unzips the result, and asserts cell 1 emits only logical w:start/w:end, cell 2 emits only physical w:left/w:right, no cell mixes start+left or end+right, and tblCellMar stays schema-ordered. 2. Importer dual-view contract. The original report claimed source start/end info was "not preserved." The data is preserved on attrs.tableCellProperties.cellMargins by design; attrs.cellMargins is a separate LTR-default physical projection per SD-3134. Two new legacy-handle-table-cell-node tests lock that contract so a future change can't quietly collapse the dual view back into a polymorphic shape.
|
🎉 This PR is included in @superdoc-dev/mcp v0.3.0-next.100 The release is available on GitHub release |
|
🎉 This PR is included in @superdoc-dev/react v1.2.0-next.144 The release is available on GitHub release |
|
🎉 This PR is included in vscode-ext v2.3.0-next.146 |
|
🎉 This PR is included in superdoc-cli v0.8.0-next.115 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-sdk v1.8.0-next.98 |
|
🎉 This PR is included in superdoc v1.30.0-next.95 The release is available on GitHub release |
2 participants
Word-authored docx with
<w:tcMar><w:start/><w:end/></w:tcMar>was gaining duplicate<w:left/><w:right/>children on export. The translate-table-cell merge wroteattrs.cellMargins(physical LTR-default per SD-3134) intotableCellProperties.cellMarginswithout reconciling the imported key family, so the v3 tcMar decoder emitted both pairs. Real-worldrtl-bidivisual-tcmar-asymmetric.docxfixture reproduces the leak.Per-side detection writes the user-visible value back into whichever pair the importer preserved: logical-only stays logical, physical-only stays physical, mixed-unchanged preserves both, mixed-edited normalizes that side to physical (mirrors the
getTableCellMarginsimport precedence where physical wins). New cells default to physical. Per-side independence — editing one side does not retroactively rewrite the other.Also sorts
w:tcMarandw:tblCellMarchildren into the ECMA-376 §A.1CT_TcMar/CT_TblCellMarxsd:sequenceorder (top, start, left, bottom, end, right) at decode time. Previous tests usedarrayContainingand missed the order regression, which surfaces whenever both pairs end up in the property bag.I kept the sort targeted to the two margin translators rather than threading it through
createNestedPropertiesTranslator— that helper backs pPr, rPr, tcPr, borders, levels, and a global ordering change needs a wider audit than this bug.Must stay the same:
attrs.cellMarginsis physical LTR-default; the painter remains the single owner of thew:bidiVisualmirror perpackages/layout-engine/pm-adapter/src/direction/README.md.Review: check the mixed-source policy in
translate-table-cell.js— per-side, physical wins on edit. Ignore thearrayContaining→ position-by-position test churn.Verified:
pnpm --filter super-editor exec vitest runover the touched files → 37 pass; wider super-editor sweep (5899 tests) → pass;@superdoc/pm-adapter(1838 tests) → pass.