feat(math): implement m:groupChr group-character converter#2751
feat(math): implement m:groupChr group-character converter#2751Abdeltoto wants to merge 2 commits intosuperdoc-dev:mainfrom
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0be6b2d3d6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 'm:d': null, // Delimiter (parentheses, brackets, braces) | ||
| 'm:eqArr': null, // Equation array (vertical array of equations) | ||
| 'm:groupChr': null, // Group character (overbrace, underbrace) | ||
| 'm:groupChr': convertGroupCharacter, // Group character (overbrace, underbrace) |
There was a problem hiding this comment.
Enabling m:groupChr conversion here changes output from a flat fallback to real <munder>/<mover> layout, but run sizing still comes from estimateMathDimensions() (packages/layout-engine/pm-adapter/src/converters/math-constants.ts), where VERTICAL_ELEMENTS does not include m:groupChr. The DOM measurer then uses run.height for line height budgeting (packages/layout-engine/measuring/dom/src/index.ts, math run branch via maxImageHeight), so under/over-brace formulas can render taller than allocated space and visually collide with neighboring lines. This registration should be paired with a m:groupChr height multiplier update (and a sizing test) to avoid layout regressions.
Useful? React with 👍 / 👎.
Made-with: Cursor
caio-pizzol
left a comment
caio-pizzol
left a comment
There was a problem hiding this comment.
@Abdeltoto solid contribution, tested all the common cases against Word and they match. one small edge case on the character fallback — left an inline comment.
| const chr = groupChrPr?.elements?.find((e) => e.name === 'm:chr'); | ||
| const pos = groupChrPr?.elements?.find((e) => e.name === 'm:pos'); | ||
|
|
||
| const groupChar = chr?.attributes?.['m:val'] ?? DEFAULT_GROUP_CHAR; |
There was a problem hiding this comment.
when <m:chr/> exists but has no value, the spec says the character should be hidden — Word shows nothing. right now this falls back to the default curly bracket instead. doesn't happen with Word-generated files (Word always writes an explicit value), but worth handling correctly.
| const groupChar = chr?.attributes?.['m:val'] ?? DEFAULT_GROUP_CHAR; | |
| const groupChar = chr ? (chr.attributes?.['m:val'] ?? '') : DEFAULT_GROUP_CHAR; |
| * Convert m:groupChr (group character) to MathML <munder> or <mover>. | ||
| * | ||
| * OMML structure: | ||
| * m:groupChr → m:groupChrPr (optional: m:chr@m:val, m:pos@m:val, m:vertJc@m:val), m:e |
There was a problem hiding this comment.
the spec also has a m:vertJc property that controls where the baseline sits — Word renders it differently for each combination. not blocking, just something to keep in mind for later.
2 participants
Closes #2606
Summary
m:groupChrOMML-to-MathML converter (overbrace, underbrace, and other stretchy group characters)<munder>(default, bottom position) or<mover>(top position) with a stretchy<mo>characterm:chris absent, matching Word behaviorMATH_OBJECT_REGISTRYSpec reference
ECMA-376 Section 22.1.2.41
Test plan
vitest runpasses for omml-to-mathml.test.ts