diff --git a/packages/super-editor/src/document-api-adapters/plan-engine/executor.ts b/packages/super-editor/src/document-api-adapters/plan-engine/executor.ts index 05af4111eb..bce50e2925 100644 --- a/packages/super-editor/src/document-api-adapters/plan-engine/executor.ts +++ b/packages/super-editor/src/document-api-adapters/plan-engine/executor.ts @@ -507,7 +507,7 @@ function applyRunAttributePatch( if (overlappingRuns.length === 0) return false; if (Object.prototype.hasOwnProperty.call(updates, 'fontFamily')) { - tr.setMeta(PRESERVE_RUN_PROPERTIES_META_KEY, ['fontFamily']); + tr.setMeta(PRESERVE_RUN_PROPERTIES_META_KEY, [{ key: 'fontFamily', preferExisting: true }]); } let changed = false; @@ -630,6 +630,16 @@ function applyInlinePatchToRange( if (inline.caps !== undefined) { textStylePatch.textTransform = capsToTextTransform(inline.caps ?? null); } + // When fontFamily is being set (not cleared) via the textStyle mark path, + // tell the calculateInlineRunPropertiesPlugin to preserve the mark-derived + // fontFamily rather than re-deriving it through the encodeMarksFromRPr + // comparison (which can incorrectly drop it due to theme font normalization). + // Only for non-null values — clearing fontFamily must not trigger preservation, + // otherwise the plugin would copy the old value back from existingRunProperties. + if (textStylePatch.fontFamily != null) { + tr.setMeta(PRESERVE_RUN_PROPERTIES_META_KEY, ['fontFamily']); + } + if (applyTextStylePatch(tr, schema.marks.textStyle, absFrom, absTo, textStylePatch)) { changed = true; } diff --git a/packages/super-editor/src/extensions/run/calculateInlineRunPropertiesPlugin.js b/packages/super-editor/src/extensions/run/calculateInlineRunPropertiesPlugin.js index a3da2e605b..f1db6063f9 100644 --- a/packages/super-editor/src/extensions/run/calculateInlineRunPropertiesPlugin.js +++ b/packages/super-editor/src/extensions/run/calculateInlineRunPropertiesPlugin.js @@ -50,12 +50,16 @@ export const calculateInlineRunPropertiesPlugin = (editor) => if (!runType) return null; const preservedDerivedKeys = new Set(); + const preferExistingKeys = new Set(); transactions.forEach((transaction) => { - const keys = transaction.getMeta(RUN_PROPERTY_PRESERVE_META_KEY); - if (!Array.isArray(keys)) return; - keys.forEach((key) => { - if (typeof key === 'string' && key.length > 0) { - preservedDerivedKeys.add(key); + const entries = transaction.getMeta(RUN_PROPERTY_PRESERVE_META_KEY); + if (!Array.isArray(entries)) return; + entries.forEach((entry) => { + if (typeof entry === 'string' && entry.length > 0) { + preservedDerivedKeys.add(entry); + } else if (entry && typeof entry === 'object' && typeof entry.key === 'string') { + preservedDerivedKeys.add(entry.key); + if (entry.preferExisting) preferExistingKeys.add(entry.key); } }); }); @@ -91,6 +95,7 @@ export const calculateInlineRunPropertiesPlugin = (editor) => $pos, editor, preservedDerivedKeys, + preferExistingKeys, ); const runProperties = firstInlineProps ?? null; @@ -218,7 +223,15 @@ export function extractTableInfo($pos, depth) { * @param {object} editor * @returns {{ segments: Array<{ inlineProps: Record|null, inlineKey: string, content: import('prosemirror-model').Node[] }>, firstInlineProps: Record|null }} */ -function segmentRunByInlineProps(runNode, paragraphNode, tableInfo, $pos, editor, preservedDerivedKeys) { +function segmentRunByInlineProps( + runNode, + paragraphNode, + tableInfo, + $pos, + editor, + preservedDerivedKeys, + preferExistingKeys, +) { const segments = []; let lastKey = null; let boundaryCounter = 0; @@ -233,6 +246,7 @@ function segmentRunByInlineProps(runNode, paragraphNode, tableInfo, $pos, editor $pos, editor, preservedDerivedKeys, + preferExistingKeys, ); const last = segments[segments.length - 1]; if (last && inlineKey === lastKey) { @@ -279,6 +293,7 @@ function computeInlineRunProps( $pos, editor, preservedDerivedKeys, + preferExistingKeys, ) { const runPropertiesFromMarks = decodeRPrFromMarks(marks); const paragraphProperties = @@ -294,12 +309,14 @@ function computeInlineRunProps( false, Boolean(paragraphNode.attrs.paragraphProperties?.numberingProperties), ); + const inlineRunProperties = getInlineRunProperties( runPropertiesFromMarks, runPropertiesFromStyles, existingRunProperties, editor, preservedDerivedKeys, + preferExistingKeys, ); const inlineProps = Object.keys(inlineRunProperties).length ? inlineRunProperties : null; const inlineKey = stableStringifyInlineProps(inlineProps); @@ -321,10 +338,34 @@ function getInlineRunProperties( existingRunProperties, editor, preservedDerivedKeys = new Set(), + preferExistingKeys = new Set(), ) { const inlineRunProperties = {}; for (const key in runPropertiesFromMarks) { - if (preservedDerivedKeys.has(key)) continue; + if (preservedDerivedKeys.has(key)) { + const fromMarks = runPropertiesFromMarks[key]; + const existing = existingRunProperties?.[key]; + if (preferExistingKeys.has(key) && existing != null) { + // rFonts / runAttribute path: the run node was directly updated with + // per-script data — existing is authoritative and already fresh. + inlineRunProperties[key] = existing; + } else if ( + fromMarks != null && + existing != null && + typeof fromMarks === 'object' && + typeof existing === 'object' + ) { + // textStyle mark path: use mark-decoded font names (fresh from the + // current mark), merged with OOXML-only metadata from existing run + // properties that the mark round-trip cannot represent (e.g. theme + // refs, hint). The spread order ensures mark font names win over + // stale existing names while preserving fields the mark cannot encode. + inlineRunProperties[key] = { ...existing, ...fromMarks }; + } else if (fromMarks !== undefined) { + inlineRunProperties[key] = fromMarks; + } + continue; + } const valueFromMarks = runPropertiesFromMarks[key]; const valueFromStyles = runPropertiesFromStyles[key]; if (JSON.stringify(valueFromMarks) !== JSON.stringify(valueFromStyles)) { diff --git a/packages/super-editor/src/extensions/run/calculateInlineRunPropertiesPlugin.test.js b/packages/super-editor/src/extensions/run/calculateInlineRunPropertiesPlugin.test.js index f4d792304b..8ab7e39e7d 100644 --- a/packages/super-editor/src/extensions/run/calculateInlineRunPropertiesPlugin.test.js +++ b/packages/super-editor/src/extensions/run/calculateInlineRunPropertiesPlugin.test.js @@ -497,6 +497,154 @@ describe('calculateInlineRunPropertiesPlugin', () => { expect(runNode?.attrs.runProperties).toEqual({ rsidR: 'r1' }); }); + it('preserves fontFamily from marks when sdPreserveRunPropertiesKeys includes fontFamily', () => { + decodeRPrFromMarksMock.mockImplementation(() => ({ + fontFamily: { ascii: 'Georgia', eastAsia: 'Georgia', hAnsi: 'Georgia', cs: 'Georgia' }, + })); + resolveRunPropertiesMock.mockImplementation(() => ({ + fontFamily: { ascii: 'Arial', hAnsi: 'Arial', eastAsia: 'Arial', cs: 'Arial' }, + })); + // Even if encodeMarksFromRPr would normalize both to the same value (the bug scenario), + // the preserve mechanism should bypass that comparison entirely. + encodeMarksFromRPrMock.mockImplementation(() => [{ attrs: { fontFamily: 'Arial, sans-serif' } }]); + + const schema = makeSchema(); + const doc = paragraphDoc(schema, { runProperties: { rsidR: 'r1' } }); + const state = createState(schema, doc); + const { from, to } = runTextRange(state.doc, 0, 1); + + const tr = state.tr.addMark(from, to, schema.marks.bold.create()); + tr.setMeta('sdPreserveRunPropertiesKeys', ['fontFamily']); + const { state: nextState } = state.applyTransaction(tr); + + const runNode = nextState.doc.nodeAt(runPos(nextState.doc) ?? 0); + expect(runNode?.attrs.runProperties).toEqual({ + rsidR: 'r1', + fontFamily: { ascii: 'Georgia', eastAsia: 'Georgia', hAnsi: 'Georgia', cs: 'Georgia' }, + }); + }); + + it('preserves existing rFonts metadata when fontFamily is preserved', () => { + const markFont = { ascii: 'Georgia', eastAsia: 'Georgia', hAnsi: 'Georgia', cs: 'Georgia' }; + decodeRPrFromMarksMock.mockImplementation(() => ({ fontFamily: markFont })); + resolveRunPropertiesMock.mockImplementation(() => ({ fontFamily: { ascii: 'Arial' } })); + + const schema = makeSchema(); + const existingFont = { + ascii: 'Georgia', + hAnsi: 'Georgia', + eastAsiaTheme: 'minorEastAsia', + csTheme: 'minorBidi', + hint: 'eastAsia', + }; + const doc = paragraphDoc(schema, { runProperties: { fontFamily: existingFont, rsidR: 'r1' } }); + const state = createState(schema, doc); + const { from, to } = runTextRange(state.doc, 0, 1); + + const tr = state.tr.addMark(from, to, schema.marks.bold.create()); + tr.setMeta('sdPreserveRunPropertiesKeys', ['fontFamily']); + const { state: nextState } = state.applyTransaction(tr); + + const runNode = nextState.doc.nodeAt(runPos(nextState.doc) ?? 0); + // The merge preserves OOXML-only metadata from existing (themes, hint) + // while overlaying fresh font names from the mark-decoded value. + expect(runNode?.attrs.runProperties?.fontFamily).toEqual({ + ...existingFont, + eastAsia: 'Georgia', + cs: 'Georgia', + }); + }); + + it('preserves untouched eastAsia and hAnsi fonts during a partial rFonts update', () => { + const markFont = { ascii: 'Georgia', eastAsia: 'Georgia', hAnsi: 'Georgia', cs: 'Georgia' }; + decodeRPrFromMarksMock.mockImplementation(() => ({ fontFamily: markFont })); + resolveRunPropertiesMock.mockImplementation(() => ({ fontFamily: { ascii: 'Arial' } })); + + const schema = makeSchema(); + const existingFont = { + ascii: 'Times New Roman', + hAnsi: 'Cambria', + eastAsia: 'MS Gothic', + cs: 'Georgia', + }; + const updatedFont = { + ascii: 'Georgia', + hAnsi: 'Cambria', + eastAsia: 'MS Gothic', + cs: 'Georgia', + }; + const doc = paragraphDoc(schema, { runProperties: { fontFamily: existingFont, rsidR: 'r1' } }); + const state = createState(schema, doc); + const pos = runPos(state.doc); + const runNode = state.doc.nodeAt(pos ?? 0); + + const tr = state.tr.setNodeMarkup( + pos ?? 0, + schema.nodes.run, + { ...runNode?.attrs, runProperties: { fontFamily: updatedFont, rsidR: 'r1' } }, + runNode?.marks, + ); + tr.setMeta('sdPreserveRunPropertiesKeys', [{ key: 'fontFamily', preferExisting: true }]); + const { state: nextState } = state.applyTransaction(tr); + + const nextRunNode = nextState.doc.nodeAt(runPos(nextState.doc) ?? 0); + expect(nextRunNode?.attrs.runProperties?.fontFamily).toEqual(updatedFont); + }); + + it('preserves untouched cs and latin fonts during a partial rFonts update', () => { + const markFont = { ascii: 'Georgia', eastAsia: 'Georgia', hAnsi: 'Georgia', cs: 'Georgia' }; + decodeRPrFromMarksMock.mockImplementation(() => ({ fontFamily: markFont })); + resolveRunPropertiesMock.mockImplementation(() => ({ fontFamily: { ascii: 'Arial' } })); + + const schema = makeSchema(); + const existingFont = { + ascii: 'Times New Roman', + hAnsi: 'Cambria', + eastAsia: 'MS Gothic', + cs: 'Traditional Arabic', + }; + const updatedFont = { + ascii: 'Times New Roman', + hAnsi: 'Cambria', + eastAsia: 'MS Gothic', + cs: 'Noto Sans Arabic', + }; + const doc = paragraphDoc(schema, { runProperties: { fontFamily: existingFont, rsidR: 'r1' } }); + const state = createState(schema, doc); + const pos = runPos(state.doc); + const runNode = state.doc.nodeAt(pos ?? 0); + + const tr = state.tr.setNodeMarkup( + pos ?? 0, + schema.nodes.run, + { ...runNode?.attrs, runProperties: { fontFamily: updatedFont, rsidR: 'r1' } }, + runNode?.marks, + ); + tr.setMeta('sdPreserveRunPropertiesKeys', [{ key: 'fontFamily', preferExisting: true }]); + const { state: nextState } = state.applyTransaction(tr); + + const nextRunNode = nextState.doc.nodeAt(runPos(nextState.doc) ?? 0); + expect(nextRunNode?.attrs.runProperties?.fontFamily).toEqual(updatedFont); + }); + + it('does not preserve fontFamily when sdPreserveRunPropertiesKeys is not set', () => { + decodeRPrFromMarksMock.mockImplementation(() => ({ fontFamily: { ascii: 'Arial' } })); + resolveRunPropertiesMock.mockImplementation(() => ({ fontFamily: { ascii: 'Arial' } })); + encodeMarksFromRPrMock.mockImplementation(() => [{ attrs: { fontFamily: 'Arial, sans-serif' } }]); + + const schema = makeSchema(); + const doc = paragraphDoc(schema, { runProperties: { fontFamily: { ascii: 'Arial' }, rsidR: 'r1' } }); + const state = createState(schema, doc); + const { from, to } = runTextRange(state.doc, 0, 1); + + // No setMeta — fontFamily should be dropped since marks match styles + 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).toEqual({ rsidR: 'r1' }); + }); + it('maps changed ranges through later transactions', () => { const schema = makeSchema(); const doc = schema.node('doc', null, [ diff --git a/tests/doc-api-stories/tests/formatting/inline-formatting.ts b/tests/doc-api-stories/tests/formatting/inline-formatting.ts index 0e738c9a9b..7d6d5610a4 100644 --- a/tests/doc-api-stories/tests/formatting/inline-formatting.ts +++ b/tests/doc-api-stories/tests/formatting/inline-formatting.ts @@ -185,6 +185,52 @@ describe('document-api story: inline formatting', () => { await saveResult(sid, 'fontFamily.docx'); }); + it('fontFamily: persists in doc.get after applying via format.fontFamily (SD-2249)', async () => { + const sid = `fontFamily-persist-${Date.now()}`; + const target = await setupFormattableText(sid, 'This text should be Georgia'); + + const result = unwrap( + await client.doc.format.apply({ + sessionId: sid, + target, + inline: { fontFamily: 'Georgia' }, + }), + ); + expect(result.receipt?.success).toBe(true); + + // Verify fontFamily survives in the in-memory model via doc.get + const doc = unwrap(await client.doc.get({ sessionId: sid })); + const block = doc.body?.[0]; + const runs = block?.paragraph?.inlines?.filter((i: any) => i.kind === 'run') ?? []; + const propsWithFont = runs.find((r: any) => r.run?.props?.fontFamily); + expect(propsWithFont).toBeDefined(); + expect(propsWithFont.run.props.fontFamily).toBe('Georgia'); + }); + + it('fontFamily: clearing with null removes fontFamily from doc.get (SD-2249)', async () => { + const sid = `fontFamily-clear-${Date.now()}`; + const target = await setupFormattableText(sid, 'This text should reset font'); + + // Set fontFamily first + const setResult = unwrap( + await client.doc.format.apply({ sessionId: sid, target, inline: { fontFamily: 'Georgia' } }), + ); + expect(setResult.receipt?.success).toBe(true); + + // Clear fontFamily + const clearResult = unwrap( + await client.doc.format.apply({ sessionId: sid, target, inline: { fontFamily: null } }), + ); + expect(clearResult.receipt?.success).toBe(true); + + // Verify fontFamily is absent after clearing + const doc = unwrap(await client.doc.get({ sessionId: sid })); + const block = doc.body?.[0]; + const runs = block?.paragraph?.inlines?.filter((i: any) => i.kind === 'run') ?? []; + const propsWithFont = runs.find((r: any) => r.run?.props?.fontFamily); + expect(propsWithFont).toBeUndefined(); + }); + it('color: sets a hex color', async () => { const sid = `color-${Date.now()}`; const target = await setupFormattableText(sid, 'This text should be red');