Skip to content

Commit df44933

Browse files
authored
fix(math): preserve italic on variables inside function-wrapped limits (SD-2538) (#2871)
* fix(math): preserve italic on variables inside function-wrapped limits (SD-2538) Word wraps lim_(n→∞) as m:func > m:fName > m:limLow. convertFunction was calling forceNormalMathVariant with querySelectorAll('mi'), which recursed into the <munder> produced by m:limLow and stripped italic from every variable inside m:lim. Scope forceNormalMathVariant so it walks only non-structural children and never overwrites an existing mathvariant — preserves m:sty/m:scr authored styling on the function name and leaves nested limit, subscript, superscript, matrix cells, etc. untouched. * docs(llms): mention native math equation rendering * test(math): add behavior-level regression for SD-2538 Pins the end-to-end shape of the fix by loading math-limit-tests.docx and asserting that every <mi> inside a lim-based <munder>'s limit expression has no mathvariant attribute — i.e. variables in m:lim stay italic when m:limLow is wrapped in m:func > m:fName. Matches the canonical shape produced by Word's own OMML2MML.XSL. * refactor(math): apply review feedback on SD-2538 fix - Rename STRUCTURAL_MATHML → MATH_VARIANT_BOUNDARY_ELEMENTS; the intent is "where the walk stops", not "which elements are structural". - Drop the inner walk closure and the Array.from snapshot — the outer function already recurses cleanly; neither earned its keep. Tests: - Fold the SD-2538 unit test into the adjacent "m:limLow in m:func" test; the fixture shape was identical, only the assertions differed. - Extend "m:limUpp in m:func" with the symmetric mathvariant assertions so the 'mover' entry of the boundary set is pinned, not just 'munder'. - Add a preserve-branch test (m:sty=i on function name) to pin the !hasAttribute guard — previously no test would have failed if we'd removed it. - Behavior test: assert exact count (5 munder + 1 mover) instead of >0, and cover both limLow and limUpp in the same sweep. Review context: Word's own OMML2MML.XSL confirmed as parity target. The subtree-opaque concern from Codex was checked against the XSL output for m:func > m:fName > m:sSub (f_i(x)) and matches our post-fix behavior. * fix(math): restore Array.from on HTMLCollection iteration The review-driven removal of Array.from broke type-check in CI: HTMLCollection is not iterable under the default lib.dom.d.ts (needs `dom.iterable`), so `for…of root.children` fails TS2488. Bun's looser runtime type-check let it slide locally. Restoring Array.from with a comment explaining why.
1 parent db64892 commit df44933

4 files changed

Lines changed: 134 additions & 7 deletions

File tree

apps/docs/llms.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ SuperDoc renders, edits, and automates .docx files — in the browser, headless
99
- Reads and writes OOXML natively — documents stay real .docx files at every step
1010
- 180+ MCP tools covering reading, editing, formatting, comments, tracked changes, and more
1111
- Tracked changes and comments as first-class operations
12+
- Math equations render natively as MathML — no external library needed
1213
- Same document model across browser editor, headless Node, and APIs
1314
- Stateless API for convert, annotate, sign, and verify
1415
- Real-time collaboration with Yjs CRDT

packages/layout-engine/painters/dom/src/features/math/converters/function.ts

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,38 @@ import type { MathObjectConverter } from '../types.js';
33
const MATHML_NS = 'http://www.w3.org/1998/Math/MathML';
44
const FUNCTION_APPLY_OPERATOR = '\u2061';
55

6+
// Boundary elements for the function-name mathvariant walk: every MathML
7+
// element whose children occupy their own semantic slot (base, subscript,
8+
// limit, matrix cell, etc.). When m:fName wraps one of these, the slot
9+
// content carries authored styling per ECMA-376 §22.1.2.111 and must not be
10+
// overwritten. Anything inside these is skipped.
11+
const MATH_VARIANT_BOUNDARY_ELEMENTS = new Set([
12+
'munder',
13+
'mover',
14+
'munderover',
15+
'msub',
16+
'msup',
17+
'msubsup',
18+
'mmultiscripts',
19+
'mfrac',
20+
'msqrt',
21+
'mroot',
22+
'mtable',
23+
'mtr',
24+
'mtd',
25+
]);
26+
627
function forceNormalMathVariant(root: ParentNode): void {
7-
root.querySelectorAll('mi').forEach((identifier) => {
8-
identifier.setAttribute('mathvariant', 'normal');
9-
});
28+
// Array.from is required here: HTMLCollection is not iterable under the
29+
// default DOM lib (needs `dom.iterable`), so `for…of root.children` fails
30+
// type-check.
31+
for (const child of Array.from(root.children)) {
32+
if (MATH_VARIANT_BOUNDARY_ELEMENTS.has(child.localName)) continue;
33+
if (child.localName === 'mi' && !child.hasAttribute('mathvariant')) {
34+
child.setAttribute('mathvariant', 'normal');
35+
}
36+
forceNormalMathVariant(child);
37+
}
1038
}
1139

1240
/**

packages/layout-engine/painters/dom/src/features/math/omml-to-mathml.test.ts

Lines changed: 60 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -861,6 +861,43 @@ describe('m:func converter', () => {
861861
expect(mis[0]!.textContent).toBe('sin');
862862
expect(mis[1]!.textContent).toBe('cos');
863863
});
864+
865+
it('preserves explicit m:sty=i on function-name runs', () => {
866+
// SD-2538 preserve branch: forceNormalMathVariant must NOT overwrite
867+
// an existing mathvariant. When Word marks a function-name run with
868+
// m:sty="i", convertMathRun already set mathvariant="italic" — the
869+
// function-apply pass must leave it alone.
870+
const omml = {
871+
name: 'm:oMath',
872+
elements: [
873+
{
874+
name: 'm:func',
875+
elements: [
876+
{
877+
name: 'm:fName',
878+
elements: [
879+
{
880+
name: 'm:r',
881+
elements: [
882+
{ name: 'm:rPr', elements: [{ name: 'm:sty', attributes: { 'm:val': 'i' } }] },
883+
{ name: 'm:t', elements: [{ type: 'text', text: 'L' }] },
884+
],
885+
},
886+
],
887+
},
888+
{
889+
name: 'm:e',
890+
elements: [{ name: 'm:r', elements: [{ name: 'm:t', elements: [{ type: 'text', text: 'x' }] }] }],
891+
},
892+
],
893+
},
894+
],
895+
};
896+
const result = convertOmmlToMathml(omml, doc);
897+
const nameMi = result!.querySelectorAll('mi')[0];
898+
expect(nameMi!.textContent).toBe('L');
899+
expect(nameMi!.getAttribute('mathvariant')).toBe('italic');
900+
});
864901
});
865902

866903
describe('m:rad converter', () => {
@@ -2296,8 +2333,18 @@ describe('m:limLow converter', () => {
22962333
const munder = result!.querySelector('munder');
22972334
expect(munder).not.toBeNull();
22982335
expect(munder!.children.length).toBe(2);
2299-
expect(munder!.children[0]!.textContent).toBe('lim');
2300-
expect(munder!.children[1]!.textContent).toBe('n');
2336+
2337+
// Base: "lim" — upright via m:sty=p on the run.
2338+
const limMi = munder!.children[0]!.querySelector('mi');
2339+
expect(limMi!.textContent).toBe('lim');
2340+
expect(limMi!.getAttribute('mathvariant')).toBe('normal');
2341+
2342+
// Limit expression: "n" — must stay italic (no mathvariant attribute set).
2343+
// SD-2538 regression: convertFunction used to recurse into the <munder>
2344+
// and force mathvariant="normal" on every <mi>, including this one.
2345+
const nMi = munder!.children[1]!.querySelector('mi');
2346+
expect(nMi!.textContent).toBe('n');
2347+
expect(nMi!.getAttribute('mathvariant')).toBeNull();
23012348
});
23022349
});
23032350

@@ -2565,8 +2612,17 @@ describe('m:limUpp converter', () => {
25652612
const mover = result!.querySelector('mover');
25662613
expect(mover).not.toBeNull();
25672614
expect(mover!.children.length).toBe(2);
2568-
expect(mover!.children[0]!.textContent).toBe('lim');
2569-
expect(mover!.children[1]!.textContent).toBe('x');
2615+
2616+
// Base: "lim" — upright via m:sty=p. Limit variable "x" — italic default.
2617+
// Symmetric to the m:limLow-in-m:func case; pins the 'mover' entry of
2618+
// MATH_VARIANT_BOUNDARY_ELEMENTS.
2619+
const limMi = mover!.children[0]!.querySelector('mi');
2620+
expect(limMi!.textContent).toBe('lim');
2621+
expect(limMi!.getAttribute('mathvariant')).toBe('normal');
2622+
2623+
const xMi = mover!.children[1]!.querySelector('mi');
2624+
expect(xMi!.textContent).toBe('x');
2625+
expect(xMi!.getAttribute('mathvariant')).toBeNull();
25702626
});
25712627
});
25722628

tests/behavior/tests/importing/math-equations.spec.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -724,6 +724,48 @@ test.describe('m:limLow / m:limUpp (limit object) rendering', () => {
724724
expect(counts.sup).toBe(1);
725725
});
726726

727+
test('keeps limit variables italic when m:limLow/m:limUpp is wrapped in m:func (SD-2538)', async ({ superdoc }) => {
728+
await superdoc.loadDocument(LIMIT_DOC);
729+
await superdoc.waitForStable();
730+
731+
// ECMA-376 §22.1.2.111: m:r without m:sty defaults to italic. Word's own
732+
// OMML2MML.xsl emits <mi>n</mi> (no mathvariant) for limit variables.
733+
//
734+
// Fixture math-limit-tests.docx has 6 m:func>m:fName wrappers: 5 around
735+
// m:limLow (→ <munder>) and 1 around m:limUpp (→ <mover>). The function
736+
// bases are lim×4, max×1, sup×1 — all carry m:sty=p and must render
737+
// upright. The limit expression runs have no m:sty and must stay italic.
738+
const FUNCTION_BASES = ['lim', 'max', 'sup'];
739+
const variantCheck = await superdoc.page.evaluate((bases) => {
740+
const collect = (tag: string) =>
741+
Array.from(document.querySelectorAll(tag))
742+
.map((el) => {
743+
const baseMi = el.children[0]?.querySelector('mi');
744+
const limitEl = el.children[1];
745+
return {
746+
base: baseMi?.textContent ?? '',
747+
baseVariant: baseMi?.getAttribute('mathvariant') ?? null,
748+
limitVariants: Array.from(limitEl?.querySelectorAll('mi') ?? []).map((mi) =>
749+
mi.getAttribute('mathvariant'),
750+
),
751+
};
752+
})
753+
.filter((entry) => bases.includes(entry.base));
754+
return { munder: collect('munder'), mover: collect('mover') };
755+
}, FUNCTION_BASES);
756+
757+
// Exact counts pin against a regression that drops a case silently.
758+
expect(variantCheck.munder).toHaveLength(5); // 3×lim + 1×max + 1×sup
759+
expect(variantCheck.mover).toHaveLength(1); // 1×lim (case: m:limUpp in func)
760+
761+
for (const entry of [...variantCheck.munder, ...variantCheck.mover]) {
762+
expect(entry.baseVariant).toBe('normal');
763+
for (const limVariant of entry.limitVariants) {
764+
expect(limVariant).toBeNull();
765+
}
766+
}
767+
});
768+
727769
test('preserves nested <msub> inside <munder> (case 8: lim of x_i → 0)', async ({ superdoc }) => {
728770
await superdoc.loadDocument(LIMIT_DOC);
729771
await superdoc.waitForStable();

0 commit comments

Comments
 (0)