diff --git a/packages/layout-engine/style-engine/src/cascade.test.ts b/packages/layout-engine/style-engine/src/cascade.test.ts index c8f5021f73..9da8ecaa51 100644 --- a/packages/layout-engine/style-engine/src/cascade.test.ts +++ b/packages/layout-engine/style-engine/src/cascade.test.ts @@ -152,6 +152,109 @@ describe('cascade - combineRunProperties', () => { bold: true, }); }); + + // SD-2894: each `` script slot has both a concrete form (`w:ascii`, + // `w:hAnsi`, `w:eastAsia`, `w:cs`) and a theme form (`w:asciiTheme`, + // `w:hAnsiTheme`, `w:eastAsiaTheme`, `w:cstheme` — note `cstheme` lowercase). + // When a higher-priority source supplies one form for a slot, the cascade must + // drop the other form from lower-priority sources, or Word resolves the concrete + // name as an override and defeats per-script theme resolution. + // + // The original SD-2894 bug was that only the (ascii, asciiTheme) pair was + // dropping correctly; hAnsi/eastAsia/cs leaked through. These tests pin the + // full 4-slot dedup so the bug cannot regress for any single slot. + describe('SD-2894 four-slot theme/concrete dedup', () => { + it('drops concrete `ascii` from lower when higher supplies `asciiTheme`', () => { + const result = combineRunProperties([ + { fontFamily: { ascii: 'Calibri' } }, + { fontFamily: { asciiTheme: 'majorBidi' } }, + ]); + expect(result.fontFamily).toEqual({ asciiTheme: 'majorBidi' }); + }); + + it('drops concrete `hAnsi` from lower when higher supplies `hAnsiTheme`', () => { + const result = combineRunProperties([ + { fontFamily: { hAnsi: 'Calibri' } }, + { fontFamily: { hAnsiTheme: 'majorHAnsi' } }, + ]); + expect(result.fontFamily).toEqual({ hAnsiTheme: 'majorHAnsi' }); + }); + + it('drops concrete `eastAsia` from lower when higher supplies `eastAsiaTheme`', () => { + const result = combineRunProperties([ + { fontFamily: { eastAsia: 'SimSun' } }, + { fontFamily: { eastAsiaTheme: 'majorEastAsia' } }, + ]); + expect(result.fontFamily).toEqual({ eastAsiaTheme: 'majorEastAsia' }); + }); + + it('drops concrete `cs` from lower when higher supplies `cstheme` (lowercase)', () => { + const result = combineRunProperties([ + { fontFamily: { cs: 'Arial' } }, + { fontFamily: { cstheme: 'majorBidi' } }, + ]); + expect(result.fontFamily).toEqual({ cstheme: 'majorBidi' }); + }); + + it('drops theme `asciiTheme` from lower when higher supplies concrete `ascii`', () => { + const result = combineRunProperties([ + { fontFamily: { asciiTheme: 'majorBidi' } }, + { fontFamily: { ascii: 'Arial' } }, + ]); + expect(result.fontFamily).toEqual({ ascii: 'Arial' }); + }); + + it('drops theme `hAnsiTheme` from lower when higher supplies concrete `hAnsi`', () => { + const result = combineRunProperties([ + { fontFamily: { hAnsiTheme: 'majorHAnsi' } }, + { fontFamily: { hAnsi: 'Arial' } }, + ]); + expect(result.fontFamily).toEqual({ hAnsi: 'Arial' }); + }); + + it('drops theme `eastAsiaTheme` from lower when higher supplies concrete `eastAsia`', () => { + const result = combineRunProperties([ + { fontFamily: { eastAsiaTheme: 'majorEastAsia' } }, + { fontFamily: { eastAsia: 'Arial' } }, + ]); + expect(result.fontFamily).toEqual({ eastAsia: 'Arial' }); + }); + + it('drops theme `cstheme` from lower when higher supplies concrete `cs`', () => { + const result = combineRunProperties([ + { fontFamily: { cstheme: 'majorBidi' } }, + { fontFamily: { cs: 'Arial' } }, + ]); + expect(result.fontFamily).toEqual({ cs: 'Arial' }); + }); + + it('Athenaintelligence customer shape: all 4 concretes from defaults dropped by inline themes', () => { + // Mirrors the customer fixture: docDefaults supply concrete fonts (Arial), + // inline rPr supplies theme refs on ascii/hAnsi/cs (no eastAsiaTheme). The + // cascade must keep only the theme refs on those three slots; eastAsia + // concrete from defaults is independent. + const result = combineRunProperties([ + { fontFamily: { ascii: 'Arial', hAnsi: 'Arial', cs: 'Arial', eastAsia: 'Arial' } }, + { fontFamily: { asciiTheme: 'majorBidi', hAnsiTheme: 'majorBidi', cstheme: 'majorBidi' } }, + ]); + expect(result.fontFamily).toEqual({ + asciiTheme: 'majorBidi', + hAnsiTheme: 'majorBidi', + cstheme: 'majorBidi', + eastAsia: 'Arial', + }); + }); + + it('exports FONT_SLOT_THEME_PAIRS so callers (super-editor plugin) can stay in sync', async () => { + const { FONT_SLOT_THEME_PAIRS } = await import('./cascade.js'); + expect(FONT_SLOT_THEME_PAIRS).toEqual([ + ['ascii', 'asciiTheme'], + ['hAnsi', 'hAnsiTheme'], + ['eastAsia', 'eastAsiaTheme'], + ['cs', 'cstheme'], + ]); + }); + }); }); describe('cascade - combineIndentProperties', () => { diff --git a/packages/layout-engine/style-engine/src/cascade.ts b/packages/layout-engine/style-engine/src/cascade.ts index 505db8d310..dce9cb82d0 100644 --- a/packages/layout-engine/style-engine/src/cascade.ts +++ b/packages/layout-engine/style-engine/src/cascade.ts @@ -104,20 +104,51 @@ function isObject(item: unknown): item is PropertyObject { * @param propertiesArray - Ordered list of run property objects. * @returns Combined run property object. */ +// SD-2894: `` exposes four independent script slots (ascii / hAnsi / eastAsia / cs). +// Each slot can be filled either by a concrete font name (`w:ascii="Calibri"`) or a theme +// reference (`w:asciiTheme="majorBidi"`). When a higher-priority style supplies one form for a +// slot, the other form from lower-priority sources is no longer applicable and must be +// dropped — otherwise the merged run carries both, and Word resolves the concrete name as an +// override that defeats per-script theme resolution (Latin body text mapped to `majorBidi` +// then renders as Calibri Light instead of Times New Roman for Hebrew/Arab). +// +// Pair `` with `Theme`, except for the cs slot whose theme attribute is the +// lowercase `cstheme` (OOXML inconsistency, not a typo). +// Exported so super-editor's calculateInlineRunPropertiesPlugin can consume the same +// list instead of duplicating it (SD-2894 follow-up). +export const FONT_SLOT_THEME_PAIRS: Array<[keyof RunFontFamilyProperties, keyof RunFontFamilyProperties]> = [ + ['ascii', 'asciiTheme'], + ['hAnsi', 'hAnsiTheme'], + ['eastAsia', 'eastAsiaTheme'], + ['cs', 'cstheme'], +]; + +function dropConflictingFontSlots( + target: RunFontFamilyProperties, + source: RunFontFamilyProperties, +): RunFontFamilyProperties { + const result = { ...target }; + for (const [concreteKey, themeKey] of FONT_SLOT_THEME_PAIRS) { + if (source[themeKey] != null) { + delete result[concreteKey]; + delete result[themeKey]; + } else if (source[concreteKey] != null) { + delete result[themeKey]; + } + } + return result; +} + export function combineRunProperties(propertiesArray: RunProperties[]): RunProperties { return combineProperties(propertiesArray, { fullOverrideProps: ['color'], specialHandling: { fontFamily: (target: Record, source: Record): unknown => { const fontFamilySource = { ...(source.fontFamily as object) } as RunFontFamilyProperties; - const fontFamilyTarget = { ...(target.fontFamily as object) } as RunFontFamilyProperties; - if (fontFamilySource.asciiTheme != null) { - delete fontFamilyTarget.ascii; - delete fontFamilyTarget.asciiTheme; - } - if (fontFamilySource.ascii != null) { - delete fontFamilyTarget.asciiTheme; - } + const fontFamilyTarget = dropConflictingFontSlots( + (target.fontFamily as RunFontFamilyProperties) ?? {}, + fontFamilySource, + ); return { ...(fontFamilyTarget as object), ...(fontFamilySource as object) }; }, }, diff --git a/packages/layout-engine/style-engine/src/ooxml/index.ts b/packages/layout-engine/style-engine/src/ooxml/index.ts index b486daed07..e894b5a955 100644 --- a/packages/layout-engine/style-engine/src/ooxml/index.ts +++ b/packages/layout-engine/style-engine/src/ooxml/index.ts @@ -5,7 +5,12 @@ * This module is format-aware (docx), but translator-agnostic. */ -import { combineIndentProperties, combineProperties, combineRunProperties } from '../cascade.js'; +import { + combineIndentProperties, + combineProperties, + combineRunProperties, + FONT_SLOT_THEME_PAIRS, +} from '../cascade.js'; import type { PropertyObject } from '../cascade.js'; import type { ParagraphConditionalFormatting, ParagraphProperties, ParagraphTabStop, RunProperties } from './types.ts'; import type { NumberingProperties } from './numbering-types.ts'; @@ -18,7 +23,7 @@ import type { TableCellProperties, } from './styles-types.ts'; -export { combineIndentProperties, combineProperties, combineRunProperties }; +export { combineIndentProperties, combineProperties, combineRunProperties, FONT_SLOT_THEME_PAIRS }; export type { PropertyObject }; export type * from './types.ts'; export type * from './numbering-types.ts'; diff --git a/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.js b/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.js index f33a09ac31..157f242fe6 100644 --- a/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.js +++ b/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.js @@ -2,6 +2,7 @@ import { Plugin, TextSelection } from 'prosemirror-state'; import { Fragment } from 'prosemirror-model'; import { TableMap } from 'prosemirror-tables'; import { decodeRPrFromMarks, encodeMarksFromRPr, resolveRunProperties } from '@converter/styles.js'; +import { FONT_SLOT_THEME_PAIRS } from '@superdoc/style-engine/ooxml'; import { calculateResolvedParagraphProperties, getResolvedParagraphProperties, @@ -28,6 +29,85 @@ const RUN_PROPERTIES_DERIVED_FROM_MARKS = new Set([ export const TRANSIENT_HYPERLINK_STYLE_IDS = new Set(['Hyperlink', 'FollowedHyperlink']); +/** + * Merge mark-derived concrete fontFamily slots with theme references preserved on the + * existing run. For each slot where the existing fontFamily has a theme reference, keep + * the theme and drop the concrete value from marks; otherwise take the mark value. + * + * Only safe to call when the caller has already verified that marks are a re-derivation + * of `existing` rather than a user override — otherwise this silently reverts user font + * changes. See `marksMatchExistingFontFamily` for the gate. + * + * SD-2894: when the plugin overrides a run's fontFamily from mark-derived values, the + * mark only carries the resolved CSS font name (`ascii: "Calibri Light"`) and forgets the + * theme reference (`asciiTheme: "majorBidi"`) that came from the imported ``. + * Without this merge we'd export concrete font names that defeat Word's per-script theme + * resolution. + * + * AIDEV-NOTE: The gate + merge pair operate at the fontFamily-key level (whole rPr + * fontFamily object), not at the per-slot level. A user action that mutates only a + * per-script mark attr (`eastAsiaFontFamily` or `csFontFamily`) without changing the + * primary `fontFamily` will pass the gate, and this merge will silently restore the + * stale theme on that slot. We accept this trade-off because (a) the toolbar's + * setFontFamily writes all four slots to the same family, so a real user override + * always changes the primary; (b) no current API or paste path produces per-script-only + * mark changes against an imported theme run; (c) a per-slot model adds intricate + * encoder probes that risk regressing the SD-2894 customer fixture again (see commit + * bec7e63ed for the prior per-script regression we just fixed). If a real customer + * reproduction surfaces — e.g. a paste-from-Word flow with per-script fonts being + * silently reverted — file a follow-up ticket with that fixture and rework this helper + * into a per-slot decision then. Codex flagged this on the PR-3225 review. + * + * @param {Record|null|undefined} fromMarks + * @param {Record|null|undefined} existing + * @returns {Record} + */ +function mergeFontFamilyPreservingThemeRefs(fromMarks, existing) { + const merged = { ...(fromMarks || {}) }; + if (!existing || typeof existing !== 'object') return merged; + for (const [concreteKey, themeKey] of FONT_SLOT_THEME_PAIRS) { + if (existing[themeKey] != null) { + merged[themeKey] = existing[themeKey]; + delete merged[concreteKey]; + } + } + return merged; +} + +/** + * True when the mark-derived fontFamily value encodes to the same primary ASCII font as + * the run's existing (imported) fontFamily — i.e., marks are a faithful re-derivation, + * not a user override. + * + * The encoder resolves theme references to their CSS font name. So: + * - import case: existing = { asciiTheme: 'majorBidi' }, marks = { ascii: 'Calibri Light' } + * → both encode to fontFamily: 'Calibri Light' → match → preserve theme. + * - user-edit case: existing = { asciiTheme: 'majorBidi' }, marks = { ascii: 'Arial' } + * → existing encodes to 'Calibri Light', marks encode to 'Arial' → mismatch → user + * has overridden, drop the theme and respect the new value. + * + * Compare ONLY the primary `fontFamily` attr, not the full mark attrs object. Per-script + * extras (`csFontFamily`, `eastAsiaFontFamily`) on the mark are derived only when the + * input has a *concrete* per-script slot. Existing run-props can carry mixed shapes + * (e.g. `{ asciiTheme, hAnsiTheme, cstheme, eastAsia: 'Arial' }`) that encode without + * `csFontFamily` while the mark re-decode adds a concrete `cs` and therefore does + * include it. A full-attrs JSON compare would falsely flag this as user override and + * drop the theme — the regression caught on the SD-2894 customer fixture round-trip. + * + * @param {{ attrs?: Record } | null | undefined} markFromMarks + * @param {Record|null|undefined} existingFontFamily + * @param {(props: Record, docx: Record) => Array<{ attrs?: Record }>} encode + * @param {Record} docx + * @returns {boolean} + */ +function marksMatchExistingFontFamily(markFromMarks, existingFontFamily, encode, docx) { + if (!existingFontFamily || typeof existingFontFamily !== 'object') return false; + if (!markFromMarks?.attrs) return false; + const markFromExisting = encode({ fontFamily: existingFontFamily }, docx)?.[0]; + if (!markFromExisting?.attrs) return false; + return markFromMarks.attrs.fontFamily === markFromExisting.attrs.fontFamily; +} + const RUN_PROPERTY_PRESERVE_META_KEY = 'sdPreserveRunPropertiesKeys'; const COMPANION_INLINE_KEYS = { fontSizeCs: 'fontSize', @@ -536,10 +616,23 @@ function getInlineRunProperties( const valueFromStyles = runPropertiesFromStyles[key]; if (JSON.stringify(valueFromMarks) !== JSON.stringify(valueFromStyles)) { if (key === 'fontFamily') { - const markFromStyles = encodeMarksFromRPr({ [key]: valueFromStyles }, editor.converter?.convertedXml ?? {})[0]; - const markFromMarks = encodeMarksFromRPr({ [key]: valueFromMarks }, editor.converter?.convertedXml ?? {})[0]; + const docx = editor.converter?.convertedXml ?? {}; + const markFromStyles = encodeMarksFromRPr({ [key]: valueFromStyles }, docx)[0]; + const markFromMarks = encodeMarksFromRPr({ [key]: valueFromMarks }, docx)[0]; if (JSON.stringify(markFromMarks?.attrs) !== JSON.stringify(markFromStyles?.attrs)) { - inlineRunProperties[key] = valueFromMarks; + // SD-2894 follow-up (PR #3225 review): only preserve theme refs from `existing` + // when the marks are a faithful re-derivation of the imported value. If marks + // diverge from existing (user picked a new font), respect the user's choice + // and drop the stale theme reference. + const existingFontFamily = existingRunProperties?.[key]; + inlineRunProperties[key] = marksMatchExistingFontFamily( + markFromMarks, + existingFontFamily, + encodeMarksFromRPr, + docx, + ) + ? mergeFontFamilyPreservingThemeRefs(valueFromMarks, existingFontFamily) + : valueFromMarks; } } else { inlineRunProperties[key] = valueFromMarks; diff --git a/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.test.js b/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.test.js index 5b510c94e4..ec7beea5f3 100644 --- a/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.test.js +++ b/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.test.js @@ -857,4 +857,407 @@ describe('calculateInlineRunPropertiesPlugin', () => { } }); }); + + // SD-2894 follow-up: the theme-rFonts preservation must NOT silently swallow user font + // changes. The two branches below pin both directions of the contract: + // (A) marks just re-encode the imported theme → preserve the theme reference; + // (B) marks diverge from the imported theme → user has overridden, drop the theme. + // This is exactly the case Luccas reproduced in the PR review. + describe('SD-2894: theme rFonts vs explicit user font change', () => { + const fontFamilyEncoder = (runProperties) => { + const ff = runProperties?.fontFamily; + if (!ff || typeof ff !== 'object') return []; + // Theme reference encodes to the CSS resolution of that theme. The customer fixture + // had majorBidi → Calibri Light; we use the same here to mirror the bug. + if (ff.asciiTheme === 'majorBidi') { + return [{ type: 'textStyle', attrs: { fontFamily: 'Calibri Light' } }]; + } + if (typeof ff.ascii === 'string') { + return [{ type: 'textStyle', attrs: { fontFamily: ff.ascii } }]; + } + return []; + }; + + beforeEach(() => { + // Style cascade has no fontFamily for this run — the only sources are the run's + // existing runProperties and the mark-derived value. + resolveRunPropertiesMock.mockImplementation(() => ({})); + encodeMarksFromRPrMock.mockImplementation(fontFamilyEncoder); + }); + + it('preserves theme reference when marks re-encode the imported theme value', () => { + // Marks decode to the concrete CSS name that the theme resolves to — the import path. + decodeRPrFromMarksMock.mockImplementation((marks) => ({ + bold: marks.some((mark) => mark.type.name === 'bold'), + fontFamily: { ascii: 'Calibri Light', hAnsi: 'Calibri Light', cs: 'Calibri Light' }, + })); + + const schema = makeSchema(); + const doc = paragraphDoc( + schema, + { + runProperties: { + fontFamily: { asciiTheme: 'majorBidi', hAnsiTheme: 'majorBidi', cstheme: 'majorBidi' }, + }, + runPropertiesInlineKeys: ['fontFamily'], + }, + [], + 'Latin', + ); + const state = createState(schema, doc); + const { from, to } = runTextRange(state.doc, 0, 5); + + // Trigger plugin re-evaluation with any benign mark change. + const tr = state.tr.addMark(from, to, schema.marks.bold.create()); + const { state: nextState } = state.applyTransaction(tr); + + const runNode = nextState.doc.nodeAt(runPos(nextState.doc) ?? 0); + expect(runNode?.attrs.runProperties?.fontFamily).toEqual({ + asciiTheme: 'majorBidi', + hAnsiTheme: 'majorBidi', + cstheme: 'majorBidi', + }); + }); + + // Customer fixture had 3 runs with existing = { asciiTheme, hAnsiTheme, cstheme, eastAsia: 'Arial' }. + // The marks re-decode to a full concrete shape including a `cs:` slot, so the encoder adds + // `csFontFamily` to markFromMarks.attrs but not to markFromExisting.attrs. A full-attrs JSON + // compare flagged these as user overrides and dropped the theme — manifesting as 65 → 62 + // theme attrs on round-trip. The gate must compare only the primary `fontFamily` slot. + it('preserves theme refs when existing mixes themes with a concrete per-script slot', () => { + decodeRPrFromMarksMock.mockImplementation((marks) => ({ + bold: marks.some((mark) => mark.type.name === 'bold'), + fontFamily: { + ascii: 'Calibri Light', + hAnsi: 'Calibri Light', + eastAsia: 'Arial', + cs: 'Calibri Light', + }, + })); + // Encoder mirrors the real encoder: theme resolves to primary 'Calibri Light'; + // a concrete `cs` slot adds `csFontFamily` to mark attrs. + encodeMarksFromRPrMock.mockImplementation((runProperties) => { + const ff = runProperties?.fontFamily; + if (!ff || typeof ff !== 'object') return []; + const attrs = {}; + if (ff.asciiTheme === 'majorBidi' || ff.ascii === 'Calibri Light') { + attrs.fontFamily = 'Calibri Light'; + } else if (typeof ff.ascii === 'string') { + attrs.fontFamily = ff.ascii; + } + if (ff.eastAsia) attrs.eastAsiaFontFamily = ff.eastAsia; + if (ff.cs && ff.cs !== ff.asciiTheme) attrs.csFontFamily = ff.cs; + return [{ type: 'textStyle', attrs }]; + }); + + const schema = makeSchema(); + const doc = paragraphDoc( + schema, + { + runProperties: { + fontFamily: { + asciiTheme: 'majorBidi', + hAnsiTheme: 'majorBidi', + cstheme: 'majorBidi', + eastAsia: 'Arial', + }, + }, + runPropertiesInlineKeys: ['fontFamily'], + }, + [], + 'Latin', + ); + const state = createState(schema, doc); + const { from, to } = runTextRange(state.doc, 0, 5); + + const tr = state.tr.addMark(from, to, schema.marks.bold.create()); + const { state: nextState } = state.applyTransaction(tr); + + const runNode = nextState.doc.nodeAt(runPos(nextState.doc) ?? 0); + const ff = runNode?.attrs.runProperties?.fontFamily; + expect(ff?.asciiTheme).toBe('majorBidi'); + expect(ff?.hAnsiTheme).toBe('majorBidi'); + expect(ff?.cstheme).toBe('majorBidi'); + expect(ff?.ascii).toBeUndefined(); + expect(ff?.hAnsi).toBeUndefined(); + expect(ff?.cs).toBeUndefined(); + }); + + it('drops theme references when the user explicitly sets a different font', () => { + // Marks decode to a CSS name the theme would NOT resolve to — the user-override path. + decodeRPrFromMarksMock.mockImplementation((marks) => ({ + bold: marks.some((mark) => mark.type.name === 'bold'), + fontFamily: { ascii: 'Arial', hAnsi: 'Arial', cs: 'Arial' }, + })); + + const schema = makeSchema(); + const doc = paragraphDoc( + schema, + { + runProperties: { + fontFamily: { asciiTheme: 'majorBidi', hAnsiTheme: 'majorBidi', cstheme: 'majorBidi' }, + }, + runPropertiesInlineKeys: ['fontFamily'], + }, + [], + 'Latin', + ); + const state = createState(schema, doc); + const { from, to } = runTextRange(state.doc, 0, 5); + + const tr = state.tr.addMark(from, to, schema.marks.bold.create()); + const { state: nextState } = state.applyTransaction(tr); + + const runNode = nextState.doc.nodeAt(runPos(nextState.doc) ?? 0); + const ff = runNode?.attrs.runProperties?.fontFamily; + expect(ff?.ascii).toBe('Arial'); + expect(ff?.asciiTheme).toBeUndefined(); + expect(ff?.hAnsiTheme).toBeUndefined(); + expect(ff?.cstheme).toBeUndefined(); + }); + }); + + // Mutation matrix — exhaustively walks the (existing.fontFamily shape, mark-decoded + // shape) cross-product to catch any regression on round-trip integrity. Each row pins + // the expected runProperties.fontFamily shape after the plugin's appendTransaction. + describe('SD-2894 mutation matrix — fontFamily round-trip integrity', () => { + // Theme → CSS map mirrors what `resolveDocxFontFamily` would produce against a + // real theme1.xml. Per-script extras (`eastAsiaFontFamily`, `csFontFamily`) are + // emitted only when the concrete slot value differs from the primary fontFamily — + // matches the real encoder in super-converter/styles.js. + const THEME_TO_CSS = { + majorBidi: 'Calibri Light', + minorBidi: 'Calibri', + majorHAnsi: 'Cambria', + minorHAnsi: 'Calibri', + majorEastAsia: 'SimSun', + minorEastAsia: 'SimSun', + }; + const universalEncoder = (runProperties) => { + const ff = runProperties?.fontFamily; + if (!ff || typeof ff !== 'object') return []; + const attrs = {}; + // Mirror resolveDocxFontFamily: any theme ref wins for the primary slot, then fall + // back to concrete ascii/hAnsi/eastAsia/cs. The real resolver also defaults to the + // 'major' theme when nothing else is present, but tests pin explicit shapes. + const themeRef = ff.asciiTheme ?? ff.hAnsiTheme ?? ff.eastAsiaTheme ?? ff.cstheme; + if (themeRef && THEME_TO_CSS[themeRef]) { + attrs.fontFamily = THEME_TO_CSS[themeRef]; + } else if (typeof ff.ascii === 'string') { + attrs.fontFamily = ff.ascii; + } else if (typeof ff.hAnsi === 'string') { + attrs.fontFamily = ff.hAnsi; + } else if (typeof ff.eastAsia === 'string') { + attrs.fontFamily = ff.eastAsia; + } else if (typeof ff.cs === 'string') { + attrs.fontFamily = ff.cs; + } + if (typeof ff.eastAsia === 'string' && ff.eastAsia !== attrs.fontFamily) { + attrs.eastAsiaFontFamily = ff.eastAsia; + } + if (typeof ff.cs === 'string' && ff.cs !== attrs.fontFamily) { + attrs.csFontFamily = ff.cs; + } + return Object.keys(attrs).length ? [{ type: 'textStyle', attrs }] : []; + }; + + const scenarios = [ + // ── Group A: theme preservation (marks re-encode the imported value) ── + { + name: 'A1: all-4-slot theme refs preserved when marks match', + existing: { + asciiTheme: 'majorBidi', + hAnsiTheme: 'majorHAnsi', + eastAsiaTheme: 'majorEastAsia', + cstheme: 'majorBidi', + }, + marksFontFamily: { + ascii: 'Calibri Light', + hAnsi: 'Cambria', + eastAsia: 'SimSun', + cs: 'Calibri Light', + }, + expected: { + asciiTheme: 'majorBidi', + hAnsiTheme: 'majorHAnsi', + eastAsiaTheme: 'majorEastAsia', + cstheme: 'majorBidi', + }, + }, + { + name: 'A2: SD-2894 customer shape (3 theme slots, no eastAsia) preserved', + existing: { asciiTheme: 'majorBidi', hAnsiTheme: 'majorBidi', cstheme: 'majorBidi' }, + marksFontFamily: { ascii: 'Calibri Light', hAnsi: 'Calibri Light', cs: 'Calibri Light' }, + expected: { asciiTheme: 'majorBidi', hAnsiTheme: 'majorBidi', cstheme: 'majorBidi' }, + }, + { + name: 'A3: themes on 3 slots + concrete eastAsia preserved (regression case)', + existing: { + asciiTheme: 'majorBidi', + hAnsiTheme: 'majorBidi', + cstheme: 'majorBidi', + eastAsia: 'Arial', + }, + marksFontFamily: { + ascii: 'Calibri Light', + hAnsi: 'Calibri Light', + eastAsia: 'Arial', + cs: 'Calibri Light', + }, + expected: { + asciiTheme: 'majorBidi', + hAnsiTheme: 'majorBidi', + cstheme: 'majorBidi', + eastAsia: 'Arial', + }, + }, + { + name: 'A4: ascii-only theme preserved', + existing: { asciiTheme: 'majorBidi' }, + marksFontFamily: { ascii: 'Calibri Light', hAnsi: 'Calibri Light', eastAsia: 'Calibri Light', cs: 'Calibri Light' }, + // hAnsi/eastAsia/cs are mark-derived concrete; only asciiTheme had a theme to preserve. + expected: { + asciiTheme: 'majorBidi', + hAnsi: 'Calibri Light', + eastAsia: 'Calibri Light', + cs: 'Calibri Light', + }, + }, + { + name: 'A5: hAnsi-only theme preserved', + existing: { hAnsiTheme: 'majorHAnsi' }, + marksFontFamily: { ascii: 'Cambria', hAnsi: 'Cambria', eastAsia: 'Cambria', cs: 'Cambria' }, + expected: { + ascii: 'Cambria', + hAnsiTheme: 'majorHAnsi', + eastAsia: 'Cambria', + cs: 'Cambria', + }, + }, + { + name: 'A6: cs-only theme preserved (lowercase `cstheme` OOXML quirk)', + existing: { cstheme: 'majorBidi' }, + marksFontFamily: { ascii: 'Calibri Light', hAnsi: 'Calibri Light', eastAsia: 'Calibri Light', cs: 'Calibri Light' }, + expected: { + ascii: 'Calibri Light', + hAnsi: 'Calibri Light', + eastAsia: 'Calibri Light', + cstheme: 'majorBidi', + }, + }, + { + name: 'A7: eastAsia-only theme preserved', + existing: { eastAsiaTheme: 'majorEastAsia' }, + marksFontFamily: { ascii: 'SimSun', hAnsi: 'SimSun', eastAsia: 'SimSun', cs: 'SimSun' }, + expected: { + ascii: 'SimSun', + hAnsi: 'SimSun', + eastAsiaTheme: 'majorEastAsia', + cs: 'SimSun', + }, + }, + // ── Group B: user override (marks diverge from existing) ── + { + name: 'B1: all-4-slot theme + user picks Arial → drop all themes', + existing: { + asciiTheme: 'majorBidi', + hAnsiTheme: 'majorHAnsi', + eastAsiaTheme: 'majorEastAsia', + cstheme: 'majorBidi', + }, + marksFontFamily: { ascii: 'Arial', hAnsi: 'Arial', eastAsia: 'Arial', cs: 'Arial' }, + expected: { ascii: 'Arial', hAnsi: 'Arial', eastAsia: 'Arial', cs: 'Arial' }, + }, + { + name: 'B2: customer shape + user picks Arial → drop themes (Luccas case)', + existing: { asciiTheme: 'majorBidi', hAnsiTheme: 'majorBidi', cstheme: 'majorBidi' }, + marksFontFamily: { ascii: 'Arial', hAnsi: 'Arial', eastAsia: 'Arial', cs: 'Arial' }, + expected: { ascii: 'Arial', hAnsi: 'Arial', eastAsia: 'Arial', cs: 'Arial' }, + }, + { + name: 'B3: mixed theme + concrete eastAsia, user picks Arial → drop everything stale', + existing: { + asciiTheme: 'majorBidi', + hAnsiTheme: 'majorBidi', + cstheme: 'majorBidi', + eastAsia: 'Arial', + }, + marksFontFamily: { ascii: 'Arial', hAnsi: 'Arial', eastAsia: 'Arial', cs: 'Arial' }, + expected: { ascii: 'Arial', hAnsi: 'Arial', eastAsia: 'Arial', cs: 'Arial' }, + }, + // ── Group C: no-theme baselines (regression sanity) ── + { + name: 'C1: pure concrete unchanged → output stays concrete', + existing: { ascii: 'Arial', hAnsi: 'Arial', eastAsia: 'Arial', cs: 'Arial' }, + marksFontFamily: { ascii: 'Arial', hAnsi: 'Arial', eastAsia: 'Arial', cs: 'Arial' }, + expected: { ascii: 'Arial', hAnsi: 'Arial', eastAsia: 'Arial', cs: 'Arial' }, + }, + { + name: 'C2: pure concrete + user picks Helvetica → output is new concrete', + existing: { ascii: 'Arial', hAnsi: 'Arial', eastAsia: 'Arial', cs: 'Arial' }, + marksFontFamily: { ascii: 'Helvetica', hAnsi: 'Helvetica', eastAsia: 'Helvetica', cs: 'Helvetica' }, + expected: { ascii: 'Helvetica', hAnsi: 'Helvetica', eastAsia: 'Helvetica', cs: 'Helvetica' }, + }, + ]; + + it.each(scenarios)('$name', ({ existing, marksFontFamily, expected }) => { + decodeRPrFromMarksMock.mockImplementation((marks) => ({ + bold: marks.some((mark) => mark.type.name === 'bold'), + fontFamily: marksFontFamily, + })); + encodeMarksFromRPrMock.mockImplementation(universalEncoder); + resolveRunPropertiesMock.mockImplementation(() => ({})); + + const schema = makeSchema(); + const doc = paragraphDoc( + schema, + { + runProperties: { fontFamily: existing }, + runPropertiesInlineKeys: ['fontFamily'], + }, + [], + 'Latin', + ); + const state = createState(schema, doc); + const { from, to } = runTextRange(state.doc, 0, 5); + + const tr = state.tr.addMark(from, to, schema.marks.bold.create()); + const { state: nextState } = state.applyTransaction(tr); + + const runNode = nextState.doc.nodeAt(runPos(nextState.doc) ?? 0); + expect(runNode?.attrs.runProperties?.fontFamily).toEqual(expected); + }); + + // Special baseline: empty existing (run has no prior fontFamily metadata). marks + // become authoritative; nothing to preserve from existing. + it('C3: empty existing + marks have concrete → marks become the source of truth', () => { + decodeRPrFromMarksMock.mockImplementation((marks) => ({ + bold: marks.some((mark) => mark.type.name === 'bold'), + fontFamily: { ascii: 'Arial', hAnsi: 'Arial', eastAsia: 'Arial', cs: 'Arial' }, + })); + encodeMarksFromRPrMock.mockImplementation(universalEncoder); + resolveRunPropertiesMock.mockImplementation(() => ({})); + + const schema = makeSchema(); + const doc = paragraphDoc( + schema, + { runProperties: null, runPropertiesInlineKeys: null }, + [], + 'Latin', + ); + const state = createState(schema, doc); + const { from, to } = runTextRange(state.doc, 0, 5); + + const tr = state.tr.addMark(from, to, schema.marks.bold.create()); + const { state: nextState } = state.applyTransaction(tr); + + const runNode = nextState.doc.nodeAt(runPos(nextState.doc) ?? 0); + expect(runNode?.attrs.runProperties?.fontFamily).toEqual({ + ascii: 'Arial', + hAnsi: 'Arial', + eastAsia: 'Arial', + cs: 'Arial', + }); + }); + }); }); diff --git a/packages/super-editor/src/editors/v1/tests/data/sd-2894-theme-rfonts.docx b/packages/super-editor/src/editors/v1/tests/data/sd-2894-theme-rfonts.docx new file mode 100644 index 0000000000..49ce9f63a1 Binary files /dev/null and b/packages/super-editor/src/editors/v1/tests/data/sd-2894-theme-rfonts.docx differ diff --git a/packages/super-editor/src/editors/v1/tests/import-export/sd-2894-theme-rfonts-roundtrip.test.js b/packages/super-editor/src/editors/v1/tests/import-export/sd-2894-theme-rfonts-roundtrip.test.js new file mode 100644 index 0000000000..f49f9f85ad --- /dev/null +++ b/packages/super-editor/src/editors/v1/tests/import-export/sd-2894-theme-rfonts-roundtrip.test.js @@ -0,0 +1,92 @@ +import { describe, it, expect, beforeAll } from 'vitest'; +import { getTestDataByFileName } from '../helpers/helpers.js'; +import { getExportedResult } from '../export/export-helpers/index.js'; + +const THEME_KEYS = ['w:asciiTheme', 'w:hAnsiTheme', 'w:eastAsiaTheme', 'w:cstheme']; + +const collectInlineRFonts = (doc) => { + const found = []; + + const walk = (node) => { + (node?.elements || []).forEach((child) => { + if (child.name === 'w:r') { + const rPr = (child.elements || []).find((e) => e.name === 'w:rPr'); + const rFonts = (rPr?.elements || []).find((e) => e.name === 'w:rFonts'); + if (rFonts) found.push(rFonts.attributes || {}); + } else if (child.elements) { + walk(child); + } + }); + }; + + walk(doc); + return found; +}; + +const countAttr = (rFontsList, key) => rFontsList.reduce((n, attrs) => (attrs[key] ? n + 1 : n), 0); + +describe('SD-2894 — theme rFonts preserved on DOCX round-trip', () => { + const fileName = 'sd-2894-theme-rfonts.docx'; + let sourceRFonts = []; + let exportedRFonts = []; + + beforeAll(async () => { + const sourceXmlMap = await getTestDataByFileName(fileName); + sourceRFonts = collectInlineRFonts(sourceXmlMap['word/document.xml']); + + const exported = await getExportedResult(fileName); + exportedRFonts = collectInlineRFonts(exported); + }); + + it('preserves the inline rFonts count on document.xml', () => { + expect(exportedRFonts.length).toBe(sourceRFonts.length); + }); + + // The ticket measured the bug by counts dropping (66 → 31). Asserting "≥ source" catches the + // regression direction without over-fitting to leaks from the cascade that don't affect Word + // rendering (e.g. a docDefault eastAsiaTheme appearing on an inline rFonts that didn't have one). + it('does not drop theme attributes that were on the source', () => { + for (const key of THEME_KEYS) { + const sourceCount = countAttr(sourceRFonts, key); + const exportedCount = countAttr(exportedRFonts, key); + expect(exportedCount, `expected ${key} count to be >= source (${sourceCount})`).toBeGreaterThanOrEqual( + sourceCount, + ); + } + }); + + it('preserves the theme value verbatim on each run that had one', () => { + expect(exportedRFonts.length).toBe(sourceRFonts.length); + for (let i = 0; i < sourceRFonts.length; i++) { + for (const key of THEME_KEYS) { + if (sourceRFonts[i][key] !== undefined) { + expect(exportedRFonts[i][key], `run #${i} ${key} should equal source`).toBe(sourceRFonts[i][key]); + } + } + } + }); + + // This is the SD-2894 customer symptom: theme refs in the source were being replaced with concrete + // font names on export, defeating Word's per-script font resolution and causing Times New Roman + // body text to render as Calibri. + it('does not substitute theme references with concrete font names on any run that had a theme reference in the source', () => { + expect(exportedRFonts.length).toBe(sourceRFonts.length); + const themeToConcrete = { + 'w:asciiTheme': 'w:ascii', + 'w:hAnsiTheme': 'w:hAnsi', + 'w:cstheme': 'w:cs', + 'w:eastAsiaTheme': 'w:eastAsia', + }; + + for (let i = 0; i < sourceRFonts.length; i++) { + for (const [themeKey, concreteKey] of Object.entries(themeToConcrete)) { + if (sourceRFonts[i][themeKey] !== undefined) { + expect( + exportedRFonts[i][concreteKey], + `run #${i}: source had ${themeKey}, export must not have a concrete ${concreteKey}`, + ).toBeUndefined(); + } + } + } + }); +});