Skip to content

fix(converter): preserve theme rFonts on DOCX round-trip (SD-2894)#3225

Open
tupizz wants to merge 1 commit into
mainfrom
tadeu/sd-2894-preserve-theme-rfonts-on-round-trip
Open

fix(converter): preserve theme rFonts on DOCX round-trip (SD-2894)#3225
tupizz wants to merge 1 commit into
mainfrom
tadeu/sd-2894-preserve-theme-rfonts-on-round-trip

Conversation

@tupizz
Copy link
Copy Markdown
Contributor

@tupizz tupizz commented May 11, 2026

Summary

  • Extends the combineRunProperties font-slot dedup to cover all four <w:rFonts> script slots (was only ascii/asciiTheme), so a higher-priority style with hAnsiTheme / eastAsiaTheme / cstheme no longer leaves the lower-priority concrete name in place.
  • Adds the same theme-aware merge to calculateInlineRunPropertiesPlugin so theme references survive the mark-driven recompute that runs after import.

Why

Customer fixture in SD-2894 had every body run mapped to <w:rFonts w:asciiTheme="majorBidi" w:hAnsiTheme="majorBidi" w:cstheme="majorBidi"/>. The cascade only knew how to drop target.ascii when source.asciiTheme was present — the other three slots fell through, so the merged run carried { ascii: "Arial", hAnsi: "Arial", cs: "Arial", asciiTheme: "majorBidi", hAnsiTheme: "majorBidi", cstheme: "majorBidi" }. Word treats the concrete value as an override that defeats per-script theme resolution: Latin body text mapped to majorBidi rendered as Calibri Light instead of the per-script font (Times New Roman for Hebrew/Arab).

A second loss happened in calculateInlineRunPropertiesPlugin on initial load — the plugin computes fontFamily from textStyle marks (which only carry concrete CSS names) and overrode runProperties.fontFamily with concrete-only values, dropping the theme references that the import had stored.

Test plan

  • New integration test (sd-2894-theme-rfonts-roundtrip.test.js) that round-trips the customer fixture and asserts w:asciiTheme / w:hAnsiTheme / w:cstheme counts don't drop and that no source theme reference becomes a concrete font on export.
  • Existing ooxml-rFonts-rstyle-linked-combos-roundtrip test still passes.
  • Full pnpm test for super-editor (12 708 / 12 721 pass).
  • @superdoc/style-engine cascade unit tests pass.
  • Manual verification: re-exported customer fixture from the dev app, opened in Word. asciiTheme count: 65 → 65 (was 65 → ~25). No Calibri Light substitutions on body text. Per-script resolution intact.

Verified diffs on the customer fixture

File input before fix after fix
w:asciiTheme in document.xml 65 25 65
w:hAnsiTheme in document.xml 65 25 65
w:cstheme in document.xml 65 25 65
w:ascii="Calibri Light" runs in document.xml 0 1 (concrete substitution) 0

The cascade's font-slot dedup only handled the ascii/asciiTheme pair, so a
higher-priority style with hAnsiTheme/eastAsiaTheme/cstheme would still leave
the lower-priority concrete name in place. Word reads the concrete value as
an override that defeats per-script theme resolution — Latin body text mapped
to majorBidi rendered as Calibri Light instead of the per-script font
(Times New Roman for Hebrew/Arab).

Extend the dedup to all four <w:rFonts> slots, and add the same logic to
calculateInlineRunPropertiesPlugin so theme refs from the imported run survive
the mark-driven recompute that runs on initial load.

Verified with a round-trip integration test on the customer fixture: asciiTheme
count 65 → 65 (was dropping to ~25 before the fix), no concrete font
substitutions on runs that originally carried theme references.
@tupizz tupizz requested a review from a team as a code owner May 11, 2026 14:26
@linear
Copy link
Copy Markdown

linear Bot commented May 11, 2026

SD-2894

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants