fix(super-editor): preserve per-script fonts and inline keys metadata (SD-2517)#2768
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a044c9d1cd
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
decodeRPrFromMarks was flattening all font scripts (ascii, eastAsia, hAnsi, cs) to the ascii font value, ignoring the eastAsiaFontFamily attribute already preserved on the mark during encode. This caused getInlineRunProperties to falsely classify fontFamily as an inline override (mark says eastAsia=Arial, style says eastAsia=Times New Roman), which polluted runPropertiesInlineKeys and made the exporter emit w:rFonts on heading runs that should inherit from the paragraph style. The fix adds an eastAsiaFontFamily case to decodeRPrFromMarks so the per-script font data survives the mark round-trip. With correct data, the existing inline/style comparison works without heuristics.
7dcf4ae to
6562286
Compare
… (SD-2517) Zero-edit round-trip was injecting 197 w:rFonts into heading runs. Root cause: decodeRPrFromMarks flattened all font scripts (eastAsia, cs) to the ascii font value. This made getInlineRunProperties falsely classify fontFamily as inline (mark says cs=Arial, style says cs=Times New Roman), polluting runPropertiesInlineKeys and causing the exporter to emit w:rFonts on runs that should inherit from the paragraph style. Two fixes applied together: 1. Mark round-trip fidelity (styles.js): Encode and decode eastAsiaFontFamily and csFontFamily as separate mark attributes so per-script fonts survive the mark round-trip per ECMA-376 §17.3.2.26. 2. Plugin inline keys guard (calculateInlineRunPropertiesPlugin.js): When the importer set runPropertiesInlineKeys to [] (nothing inline), don't add mark-derived keys unless the user genuinely applied new formatting (detected via hasNewInlineProps). This handles remaining edge cases where theme font resolution (hAnsiTheme) still causes false positives.
…SD-2517) Address review findings: - Register eastAsiaFontFamily and csFontFamily as global attributes on the textStyle mark via FontFamily extension. Without schema registration, ProseMirror dropped these attrs during Mark.fromJSON(), making the per-script font decode paths unreachable (Codex review finding). - Strip csFontFamily from visual mark attrs in resolveFontFamily alongside eastAsiaFontFamily. Both are round-trip metadata, not CSS properties (Opus review finding: csFontFamily leaked to DOM). - Replace IIFE with plain const for overrideKeys in computeSegmentKeys (DX finding from both reviewers). Result: 468 → 153 rFonts (vs 183 without schema registration).
…pping (SD-2517) Cover the new csFontFamily handling in resolveFontFamily: - csFontFamily stripped from visual attrs for non-EA text - both eastAsiaFontFamily and csFontFamily stripped together - attrs returned unchanged when no per-script fonts present
…s them (SD-2517) The calculateInlineRunPropertiesPlugin was dropping inline keys that the importer explicitly set (e.g. fontFamily from w:rPr) when the plugin's getInlineRunProperties comparison falsely classified them as matching the style. This caused 118 runs to lose their explicit w:rFonts on export. When the importer set non-empty runPropertiesInlineKeys, restore any keys the plugin dropped back into runProperties from the original node attrs. The importer saw explicit w:rPr in the XML — that decision is authoritative over the plugin's mark-based recomputation. Result: INPUT 271 rFonts → BEFORE 468 → AFTER 275 (near-perfect, +4 from per-script encode edge cases, 0 injection).
End-to-end behavior test that loads a DOCX with localized Portuguese heading styles, exports without edits, and verifies the rFonts count didn't increase (the 197-injection regression).
|
🎉 This PR is included in vscode-ext v1.1.0-next.80 |
|
🎉 This PR is included in @superdoc-dev/react v1.0.0-next.34 The release is available on GitHub release |
|
🎉 This PR is included in template-builder v1.3.0-next.40 The release is available on GitHub release |
|
🎉 This PR is included in esign v2.2.0-next.38 The release is available on GitHub release |
|
🎉 This PR is included in superdoc v1.24.0-next.77 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-cli v0.5.0-next.78 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-sdk v1.3.0-next.79 |
|
🎉 This PR is included in superdoc-sdk v1.4.0 |
|
🎉 This PR is included in superdoc v1.25.0 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-cli v0.6.0 The release is available on GitHub release |
|
🎉 This PR is included in vscode-ext v2.3.0-next.1 |
|
🎉 This PR is included in template-builder v1.5.0-next.1 The release is available on GitHub release |
|
🎉 This PR is included in esign v2.3.0-next.1 The release is available on GitHub release |
|
🎉 This PR is included in vscode-ext v2.3.0 |
Zero-edit round-trip was injecting 197
w:rFontsinto heading runs that should inherit from the paragraph style. Most visible with localized Portuguese heading styles (Ttulo1,Ttulo2).Root cause:
decodeRPrFromMarksflattened per-script fonts (eastAsia, cs) to the ascii value, makinggetInlineRunPropertiesfalsely classifyfontFamilyas inline.Fix (3 layers):
Mark round-trip fidelity (
styles.js,font-family.js): Encode/decodeeastAsiaFontFamilyandcsFontFamilyas registered mark attributes so per-script fonts survive per ECMA-376 §17.3.2.26.Plugin guard (
calculateInlineRunPropertiesPlugin.js): When the importer setrunPropertiesInlineKeys: [], skip mark-derived key addition unless the user applied new formatting. Handles remaining edge cases (theme font resolution).Inline key preservation (
calculateInlineRunPropertiesPlugin.js): When the plugin drops keys the importer explicitly set (e.g. fontFamily on runs withw:rPr), restore them from the original node attrs.Result: INPUT 271 rFonts → BEFORE 468 (+197 injected) → AFTER 275 (+4 from per-script encoding, 0 injected)
112 unit tests passing, regression document uploaded to test corpus.
Closes SD-2517 / IT-907 / #2767