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 @@ -109,12 +109,24 @@ export function encodeMarksFromRPr(runProperties, docx) {
const fontFamily = resolveDocxFontFamily(value, docx, getToCssFontFamily());
textStyleAttrs[key] = fontFamily;
// value can be a string (from resolveRunPropertiesFromParagraphStyle) or an object
const eastAsiaFamily = typeof value === 'object' && value !== null ? value['eastAsia'] : undefined;

if (eastAsiaFamily) {
const eastAsiaCss = getFontFamilyValue({ 'w:ascii': eastAsiaFamily }, docx);
if (!fontFamily || eastAsiaCss !== textStyleAttrs.fontFamily) {
textStyleAttrs.eastAsiaFontFamily = eastAsiaCss;
// Preserve per-script fonts when they differ from ascii (ECMA-376 §17.3.2.26:
// ascii, hAnsi, eastAsia, cs are independent font slots). Without this, the mark
// round-trip flattens all scripts to the ascii font, causing false-positive inline
// detection and w:rFonts injection on export. (SD-2517)
if (typeof value === 'object' && value !== null) {
const eastAsiaFamily = value['eastAsia'];
if (eastAsiaFamily) {
const eastAsiaCss = getFontFamilyValue({ 'w:ascii': eastAsiaFamily }, docx);
if (!fontFamily || eastAsiaCss !== textStyleAttrs.fontFamily) {
textStyleAttrs.eastAsiaFontFamily = eastAsiaCss;
}
}
const csFamily = value['cs'];
if (csFamily) {
const csCss = getFontFamilyValue({ 'w:ascii': csFamily }, docx);
if (!fontFamily || csCss !== textStyleAttrs.fontFamily) {
textStyleAttrs.csFontFamily = csCss;
}
}
}
break;
Expand Down Expand Up @@ -599,6 +611,20 @@ export function decodeRPrFromMarks(marks) {
runProperties.fontFamily = result;
}
break;
case 'eastAsiaFontFamily':
// Restore per-script East Asian font from the mark attribute preserved
// during encode. Without this, decodeRPrFromMarks flattens all scripts
// to the ascii font, causing false-positive inline detection. (SD-2517)
if (value != null && runProperties.fontFamily) {
runProperties.fontFamily.eastAsia = value.split(',')[0].trim();
}
break;
case 'csFontFamily':
// Restore per-script Complex Script font (same pattern as eastAsia).
if (value != null && runProperties.fontFamily) {
runProperties.fontFamily.cs = value.split(',')[0].trim();
}
break;
case 'vertAlign':
if (value != null) {
runProperties.vertAlign = value;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -538,4 +538,52 @@ describe('marks encoding/decoding round-trip', () => {
// encodeMarksFromRPr doesn't handle 'caps', so it produces no textTransform mark.
expect(marksFromCaps.some((m) => m.type === 'textStyle' && m.attrs.textTransform)).toBe(false);
});

// SD-2517: per-script fonts (eastAsia, cs) must survive the mark round-trip.
// ECMA-376 §17.3.2.26 defines 4 independent font slots: ascii, hAnsi, eastAsia, cs.
it('preserves per-script fonts (eastAsia, cs) through encode → decode round-trip', () => {
const rPr = {
fontFamily: { ascii: 'Arial', hAnsi: 'Arial', eastAsia: 'MS Mincho', cs: 'Times New Roman' },
};
const marks = encodeMarksFromRPr(rPr, {});
const textStyleMark = marks.find((m) => m.type === 'textStyle');
// Encode should preserve per-script fonts as separate mark attributes
expect(textStyleMark.attrs.eastAsiaFontFamily).toMatch(/^MS Mincho/);
expect(textStyleMark.attrs.csFontFamily).toMatch(/^Times New Roman/);

const decoded = decodeRPrFromMarks(marks);
// Decode should restore per-script fonts, not flatten to ascii
expect(decoded.fontFamily.ascii).toMatch(/^Arial/);
expect(decoded.fontFamily.hAnsi).toMatch(/^Arial/);
expect(decoded.fontFamily.eastAsia).toMatch(/^MS Mincho/);
expect(decoded.fontFamily.cs).toMatch(/^Times New Roman/);
});

it('omits per-script mark attrs when all scripts match ascii', () => {
const rPr = {
fontFamily: { ascii: 'Arial', hAnsi: 'Arial', eastAsia: 'Arial', cs: 'Arial' },
};
const marks = encodeMarksFromRPr(rPr, {});
const textStyleMark = marks.find((m) => m.type === 'textStyle');
expect(textStyleMark.attrs.eastAsiaFontFamily).toBeUndefined();
expect(textStyleMark.attrs.csFontFamily).toBeUndefined();

const decoded = decodeRPrFromMarks(marks);
expect(decoded.fontFamily).toEqual({ ascii: 'Arial', eastAsia: 'Arial', hAnsi: 'Arial', cs: 'Arial' });
});

it('preserves cs font when only cs differs from ascii', () => {
const rPr = {
fontFamily: { ascii: 'Arial', hAnsi: 'Arial', eastAsia: 'Arial', cs: 'Times New Roman' },
};
const marks = encodeMarksFromRPr(rPr, {});
const textStyleMark = marks.find((m) => m.type === 'textStyle');
expect(textStyleMark.attrs.eastAsiaFontFamily).toBeUndefined();
expect(textStyleMark.attrs.csFontFamily).toMatch(/^Times New Roman/);

const decoded = decodeRPrFromMarks(marks);
expect(decoded.fontFamily.ascii).toMatch(/^Arial/);
expect(decoded.fontFamily.cs).toMatch(/^Times New Roman/);
expect(decoded.fontFamily.eastAsia).toMatch(/^Arial/);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,13 @@ const containsEastAsianCharacters = (text) => EAST_ASIAN_CHARACTER_REGEX.test(te
export const resolveFontFamily = (textStyleAttrs, text) => {
if (!text) return textStyleAttrs;
const eastAsiaFont = textStyleAttrs?.eastAsiaFontFamily;
if (!eastAsiaFont) return textStyleAttrs;
const hasPerScriptAttrs = eastAsiaFont || textStyleAttrs?.csFontFamily;
if (!hasPerScriptAttrs) return textStyleAttrs;
// Strip per-script font attrs from the visual mark — they're round-trip metadata
// preserved on the ProseMirror mark attrs, not CSS properties. (SD-2517)
const normalized = { ...textStyleAttrs };
delete normalized.eastAsiaFontFamily;
delete normalized.csFontFamily;
const shouldUseEastAsia = typeof text === 'string' && containsEastAsianCharacters(text);
if (!shouldUseEastAsia) return normalized;
return { ...normalized, fontFamily: eastAsiaFont };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,27 @@ describe('w:r helper utilities', () => {
const attrs = { eastAsiaFontFamily: 'Meiryo, sans-serif' };
expect(resolveFontFamily(attrs, '')).toEqual(attrs);
});

// SD-2517: csFontFamily must be stripped from visual attrs (not a CSS property)
it('strips csFontFamily from visual attrs for non-EA text', () => {
const attrs = { fontFamily: 'Arial', csFontFamily: 'Times New Roman' };
const result = resolveFontFamily(attrs, 'Hello');
expect(result.csFontFamily).toBeUndefined();
expect(result.fontFamily).toBe('Arial');
});

it('strips both eastAsiaFontFamily and csFontFamily for non-EA text', () => {
const attrs = { fontFamily: 'Arial', eastAsiaFontFamily: 'MS Mincho', csFontFamily: 'Times New Roman' };
const result = resolveFontFamily(attrs, 'Hello');
expect(result.eastAsiaFontFamily).toBeUndefined();
expect(result.csFontFamily).toBeUndefined();
expect(result.fontFamily).toBe('Arial');
});

it('returns attrs unchanged when no per-script fonts are present', () => {
const attrs = { fontFamily: 'Arial', fontSize: '12pt' };
expect(resolveFontFamily(attrs, 'Hello')).toBe(attrs);
});
});

describe('merge helpers', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,8 +243,10 @@ describe('Diff', () => {
'runProperties.boldCs': true,
});
expect(formattingRunAttrsDiff.deleted).toEqual({});
// SD-2517: importer now preserves [] for runs with no inline w:rPr.
// When user adds bold (diff_after7), hasNewInlineProps triggers mark key addition.
expect(formattingRunAttrsDiff.modified?.runPropertiesInlineKeys).toEqual({
from: ['fontFamily', 'fontSize'],
from: [],
to: ['bold', 'boldCs', 'fontFamily', 'fontSize'],
});
expect(formattingRunAttrsDiff.modified?.rsidRPr).toMatchObject({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,18 @@ export const FontFamily = Extension.create({
return { style: `font-family: ${attrs.fontFamily}` };
},
},
// Per-script font overrides (ECMA-376 §17.3.2.26). These are round-trip
// metadata — not rendered to DOM. They preserve per-script font data through
// the mark round-trip so the inline/style comparison doesn't produce false
// positives that inject w:rFonts on export. (SD-2517)
eastAsiaFontFamily: {
default: null,
rendered: false,
},
csFontFamily: {
default: null,
rendered: false,
},
},
},
];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,46 +99,98 @@ export const calculateInlineRunPropertiesPlugin = (editor) =>
preservedDerivedKeys,
preferExistingKeys,
);
const runProperties = firstInlineProps ?? null;
let runProperties = firstInlineProps ?? null;

const existingInlineKeys = runNode.attrs?.runPropertiesInlineKeys || [];
// [] means "importer explicitly found nothing inline"; null means "no metadata" (legacy).
// The exporter treats null as "export all keys" for backward compat, so [] must be preserved.
const hadInlineKeysMetadata = Array.isArray(runNode.attrs?.runPropertiesInlineKeys);
const styleKeys = runNode.attrs?.runPropertiesStyleKeys || [];
const keysFromMarks = (segment) => {
const textNode = segment.content?.find((n) => n.isText);
return Object.keys(decodeRPrFromMarks(textNode?.marks || []));
};
const overrideKeysFromInlineProps = (inlineProps) => styleKeys.filter((k) => inlineProps && k in inlineProps);

// When the importer set an empty inline keys list ([]), it means the original run
// had no inline w:rPr — all properties are style-inherited. Preserve that decision
// unless the user has genuinely added new formatting (detected by new keys appearing
// in computed inline props that weren't in the previous run properties).
//
// Without this guard, mark-derived keys (e.g. fontFamily from paragraph style) get
// added to the allow-list. Marks lose per-script fidelity through the round-trip
// (eastAsia/cs get flattened to the ascii font), causing the exporter to emit inline
// w:rPr that breaks style inheritance in Word. (SD-2517 / IT-907)
const existingRunPropsKeys = new Set(
runNode.attrs?.runProperties ? Object.keys(runNode.attrs.runProperties) : [],
);

/**
* Compute inline keys for a segment, respecting the [] vs null distinction.
* @param {Record<string, any>|null} segmentInlineProps - Computed inline props for this segment
* @param {{ content: import('prosemirror-model').Node[] }} segment - The segment to extract mark keys from
* @returns {{ inlineKeys: string[]|null, overrideKeys: string[]|null }}
*/
const computeSegmentKeys = (segmentInlineProps, segment) => {
// Detect genuinely new inline properties (user-applied formatting, not just
// recomputation artifacts from mark round-trip fidelity loss).
const hasNewInlineProps =
segmentInlineProps != null && Object.keys(segmentInlineProps).some((k) => !existingRunPropsKeys.has(k));
const shouldAddMarkKeys = !hadInlineKeysMetadata || existingInlineKeys.length > 0 || hasNewInlineProps;
const markKeysToAdd = shouldAddMarkKeys ? keysFromMarks(segment) : [];
const keys = [...new Set([...existingInlineKeys, ...markKeysToAdd])];
const ok = overrideKeysFromInlineProps(segmentInlineProps);
return {
inlineKeys: keys.length ? keys : hadInlineKeysMetadata ? [] : null,
overrideKeys: ok?.length ? ok : null,
};
};

if (segments.length === 1) {
const hadInlineKeys =
Array.isArray(runNode.attrs?.runPropertiesInlineKeys) && runNode.attrs.runPropertiesInlineKeys.length > 0;
if (JSON.stringify(runProperties) === JSON.stringify(runNode.attrs.runProperties) && hadInlineKeys) return;
// Allow-list = prior inline keys ∪ mark keys only. Do not union Object.keys(runProperties): runs often
// carry resolved paragraph-style noise in runProperties; listing every key would re-export it on w:rPr
// and bloat document.xml. Importer / plan-engine must seed runPropertiesInlineKeys for true OOXML keys.
const newInlineKeys = [...new Set([...existingInlineKeys, ...keysFromMarks(segments[0])])];
const newOverrideKeys = overrideKeysFromInlineProps(runProperties);
// When the importer set non-empty inline keys and the computed inline props
// dropped some of those keys (e.g. fontFamily "matches" the style due to
// mark round-trip comparison), preserve the original keys. The importer saw
// explicit w:rPr in the XML and that decision is authoritative. (SD-2517)
if (hadInlineKeys) {
const computedKeys = new Set(runProperties ? Object.keys(runProperties) : []);
const lostKeys = existingInlineKeys.filter((k) => !computedKeys.has(k));
if (lostKeys.length > 0) {
if (!runProperties) runProperties = {};
lostKeys.forEach((k) => {
if (runNode.attrs?.runProperties?.[k] !== undefined) {
runProperties[k] = runNode.attrs.runProperties[k];
}
});
}
}
const { inlineKeys: newInlineKeys, overrideKeys: newOverrideKeys } = computeSegmentKeys(
runProperties,
segments[0],
);
tr.setNodeMarkup(
mappedPos,
runNode.type,
{
...runNode.attrs,
runProperties,
runPropertiesInlineKeys: newInlineKeys.length ? newInlineKeys : null,
runPropertiesOverrideKeys: newOverrideKeys.length ? newOverrideKeys : null,
runPropertiesInlineKeys: newInlineKeys,
runPropertiesOverrideKeys: newOverrideKeys,
},
runNode.marks,
);
} else {
const newRuns = segments.map((segment) => {
const props = segment.inlineProps ?? null;
const segmentInlineKeys = [...new Set([...existingInlineKeys, ...keysFromMarks(segment)])];
const segmentOverrideKeys = overrideKeysFromInlineProps(props);
const { inlineKeys: segInlineKeys, overrideKeys: segOverrideKeys } = computeSegmentKeys(props, segment);
return runType.create(
{
...(runNode.attrs ?? {}),
runProperties: props,
runPropertiesInlineKeys: segmentInlineKeys.length ? segmentInlineKeys : null,
runPropertiesOverrideKeys: segmentOverrideKeys.length ? segmentOverrideKeys : null,
runPropertiesInlineKeys: segInlineKeys,
runPropertiesOverrideKeys: segOverrideKeys,
},
Fragment.fromArray(segment.content),
runNode.marks,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -759,4 +759,102 @@ describe('calculateInlineRunPropertiesPlugin', () => {
const boldRun = finalState.doc.nodeAt(runs[1]);
expect(boldRun?.attrs.runProperties).toEqual({ bold: true });
});

// SD-2517: empty inline keys metadata preservation
describe('SD-2517: empty inline keys [] vs null semantics', () => {
it('preserves runPropertiesInlineKeys: [] when no user edits occur (zero-edit round-trip)', () => {
// Simulate a run imported with no inline w:rPr ([] from importer).
// The plugin should not pollute the allow-list with mark-derived keys.
decodeRPrFromMarksMock.mockImplementation(() => ({ fontFamily: { ascii: 'Arial' } }));
resolveRunPropertiesMock.mockImplementation(() => ({ fontFamily: { ascii: 'Arial' } }));

const schema = makeSchema();
const doc = paragraphDoc(
schema,
{
runProperties: { fontFamily: { ascii: 'Arial' }, lang: { val: 'pt-BR' } },
runPropertiesInlineKeys: [],
runPropertiesStyleKeys: null,
runPropertiesOverrideKeys: null,
},
[],
'Heading text',
);
const state = createState(schema, doc);

// Trigger a doc change so the plugin fires (simulates another plugin changing the doc)
const textPos = runPos(state.doc) + 1;
const tr = state.tr.insertText('X', textPos).delete(textPos, textPos + 1);
const { state: nextState } = state.applyTransaction(tr);

const runNode = nextState.doc.nodeAt(runPos(nextState.doc) ?? 0);
// The inline keys should remain [] (or at most not contain fontFamily)
const inlineKeys = runNode?.attrs.runPropertiesInlineKeys;
expect(inlineKeys === null || (Array.isArray(inlineKeys) && !inlineKeys.includes('fontFamily'))).toBe(true);
});

it('adds mark keys when user applies new formatting to a run with empty inline keys', () => {
// A run imported with [] gets user-applied bold → bold should appear in inline keys.
decodeRPrFromMarksMock.mockImplementation((marks) => ({
bold: marks.some((mark) => mark.type.name === 'bold'),
fontFamily: { ascii: 'Arial' },
}));
// resolveRunProperties returns style-resolved props WITHOUT bold (style doesn't define bold)
resolveRunPropertiesMock.mockImplementation(() => ({ fontFamily: { ascii: 'Arial' } }));

const schema = makeSchema();
const doc = paragraphDoc(
schema,
{
runProperties: { lang: { val: 'pt-BR' } },
runPropertiesInlineKeys: [],
runPropertiesStyleKeys: null,
runPropertiesOverrideKeys: null,
},
[],
'Hello',
);
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);
// bold should be in inline keys (it's a genuine user edit)
expect(runNode?.attrs.runPropertiesInlineKeys).toContain('bold');
});

it('preserves null inline keys for legacy runs (backward compatibility)', () => {
// Legacy collab payloads have runPropertiesInlineKeys: null.
// The plugin should keep null (not convert to []).
decodeRPrFromMarksMock.mockImplementation(() => ({ bold: false }));
resolveRunPropertiesMock.mockImplementation(() => ({ bold: false }));

const schema = makeSchema();
const doc = paragraphDoc(
schema,
{
runProperties: { lang: { val: 'en-US' } },
runPropertiesInlineKeys: null,
runPropertiesStyleKeys: null,
runPropertiesOverrideKeys: null,
},
[],
'Hello',
);
const state = createState(schema, doc);
const textPos = runPos(state.doc) + 1;
const tr = state.tr.insertText('X', textPos).delete(textPos, textPos + 1);
const { state: nextState } = state.applyTransaction(tr);

const runNode = nextState.doc.nodeAt(runPos(nextState.doc) ?? 0);
// With null metadata (legacy), the plugin adds mark-derived keys (backward compat).
// The key invariant: it must NOT become [] (that would mean "importer said nothing inline").
const inlineKeys = runNode?.attrs.runPropertiesInlineKeys;
if (inlineKeys !== null) {
expect(Array.isArray(inlineKeys) && inlineKeys.length > 0).toBe(true);
}
});
});
});
Loading
Loading