Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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']);
Comment thread
caio-pizzol marked this conversation as resolved.
}

if (applyTextStylePatch(tr, schema.marks.textStyle, absFrom, absTo, textStylePatch)) {
changed = true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
});
});
Expand Down Expand Up @@ -91,6 +95,7 @@ export const calculateInlineRunPropertiesPlugin = (editor) =>
$pos,
editor,
preservedDerivedKeys,
preferExistingKeys,
);
const runProperties = firstInlineProps ?? null;

Expand Down Expand Up @@ -218,7 +223,15 @@ export function extractTableInfo($pos, depth) {
* @param {object} editor
* @returns {{ segments: Array<{ inlineProps: Record<string, any>|null, inlineKey: string, content: import('prosemirror-model').Node[] }>, firstInlineProps: Record<string, any>|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;
Expand All @@ -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) {
Expand Down Expand Up @@ -279,6 +293,7 @@ function computeInlineRunProps(
$pos,
editor,
preservedDerivedKeys,
preferExistingKeys,
) {
const runPropertiesFromMarks = decodeRPrFromMarks(marks);
const paragraphProperties =
Expand All @@ -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);
Expand All @@ -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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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, [
Expand Down
46 changes: 46 additions & 0 deletions tests/doc-api-stories/tests/formatting/inline-formatting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<any>(
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<any>(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<any>(
await client.doc.format.apply({ sessionId: sid, target, inline: { fontFamily: 'Georgia' } }),
);
expect(setResult.receipt?.success).toBe(true);

// Clear fontFamily
const clearResult = unwrap<any>(
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<any>(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');
Expand Down
Loading