Skip to content

feat(math): implement m:phant phantom converter#2749

Open
Abdeltoto wants to merge 3 commits intosuperdoc-dev:mainfrom
Abdeltoto:feat/math-phantom-converter
Open

feat(math): implement m:phant phantom converter#2749
Abdeltoto wants to merge 3 commits intosuperdoc-dev:mainfrom
Abdeltoto:feat/math-phantom-converter

Conversation

@Abdeltoto
Copy link
Copy Markdown
Contributor

Closes #2608

Summary

  • Implements the m:phant OMML-to-MathML converter (invisible spacing placeholder)
  • Full phantom maps to <mphantom>; partial dimension control maps to <mpadded> with zeroed width/height/depth
  • Supports m:show, m:zeroWid, m:zeroAsc, and m:zeroDesc property flags
  • Registers the converter in MATH_OBJECT_REGISTRY
  • Adds 3 unit tests covering full phantom, visible with zeroed width, and invisible with zeroed height

Spec reference

ECMA-376 Section 22.1.2.81

Test plan

  • vitest run passes for omml-to-mathml.test.ts
  • Linter and formatter pass (lefthook pre-commit)

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 566fc42ea0

ℹ️ 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".

@Abdeltoto Abdeltoto force-pushed the feat/math-phantom-converter branch from 3e7e2c0 to c6b8cc6 Compare April 8, 2026 03:03
Copy link
Copy Markdown
Contributor

@caio-pizzol caio-pizzol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Abdeltoto thanks for contributing this! the phantom converter is a nice addition and the code is clean — matches Word perfectly on 7 out of 10 test cases.

one thing to fix: when <m:show> is missing, content renders invisible but should be visible. this affects the most common phantom pattern (zeroing a dimension without explicitly setting show). left an inline comment with a one-line fix, comparison screenshots, and 10 Word-native test files.

/** OOXML ST_OnOff true values. */
const isOnOffTrue = (val?: string) => val === '1' || val === 'on' || val === 'true';

const isVisible = isOnOffTrue(show?.attributes?.['m:val']) || (show != null && !show.attributes);
Copy link
Copy Markdown
Contributor

@caio-pizzol caio-pizzol Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when <m:show> is missing from the phantom properties, the content should be visible — but right now it renders invisible. this is because both sides of the OR are false when show is undefined.

this breaks the most common phantom use case: zeroing descent without hiding content (the example in the spec). I tested 10 Word-native .docx files covering every property combination — these three don't match Word:

file what happens should happen
phantom-01 (no properties) content hidden content shown
phantom-03 (zeroed descent) content hidden inside radical content shown inside radical
phantom-09 (fraction + zeroed descent) content hidden content shown

phantom-01
SuperDoc:
image
Word:
image

phantom-03
SuperDoc:
image

Word:
image

Suggested change
const isVisible = isOnOffTrue(show?.attributes?.['m:val']) || (show != null && !show.attributes);
const isVisible = show == null || isOnOffTrue(show?.attributes?.['m:val']) || (show != null && !show.attributes);

tests 1 and 4 will need updating too — they expect the current (wrong) behavior.

test files attached below if you want to verify locally.
phantom-test-files.zip

@caio-pizzol caio-pizzol self-assigned this Apr 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Math: implement m:phant phantom converter (community)

2 participants