Closed Bug 1984018 Opened 1 year ago Closed 11 months ago

Remove work around for DisplayOperatorMinHeight

Categories

(Core :: MathML, task)

task

Tracking

()

RESOLVED FIXED
144 Branch
Tracking Status
firefox144 --- fixed

People

(Reporter: fwang, Assigned: eri)

References

(Blocks 2 open bugs, )

Details

Attachments

(1 file)

displayOperatorMinHeight is calculated here: https://searchfox.org/firefox-main/rev/60308bc3792ef201b82377682de068a5a1c72575/layout/mathml/nsMathMLChar.cpp#972

This is similar to https://w3c.github.io/mathml-core/#layout-of-operators and https://w3c.github.io/mathml-core/#dfn-displayoperatorminheight but historically Firefox has been performing extra tweaks to work around issues with some fonts.

I believe what happened is that Microsoft made a mistake in Cambria Math / Microsoft Office (they switched delimitedSubFormulaMinHeight and displayOperatorMinHeight parameters): https://github.com/MicrosoftDocs/typography-issues/issues/1136

Several math rendering engines (including Firefox) had to work around that issue by their own mean.

Other math fonts (Asana, GUST e-foundry's ones) had similar issues probably because they were copying Cambria Math or were tested with Microsoft Office or with other engines not revealing the issue.

When Igalia implemented MathML in Chromium, we refrain from introducing this non-standard work around and I reported the issue to all the font authors I know at that time. I believe only Cambria Math remained to be fixed (and Microsoft was not clear whether they would fix it on the MicrosoftDocs issue above). Possibly new fonts had been created in the meantime. I hope they did not propagate the issue further, but I guess these recent fonts have smaller usages anyway.

Finally, Khaled wrote a work around for Cambria Math in HarfBuzz at https://github.com/harfbuzz/harfbuzz/issues/4653 which we imported in bug 1961055: https://searchfox.org/firefox-main/rev/60308bc3792ef201b82377682de068a5a1c72575/gfx/harfbuzz/src/hb-ot-math.cc#90

So to summarize, I believe we can now try and remove Firefox's work around i.e. always just set displayOperatorMinHeight = mathFont->MathTable()->Constant(gfxMathTable::DisplayOperatorMinHeight, oneDevPixel) for fonts with a MATH table.

Probably, we should add a WPT test largeop-default-font.html that verifies whether basic operators like "N-Ary Summation (U+2211)" or "Integral (U+222B)" are drawn bigger with the default font installed on the system similar to existing ones at https://searchfox.org/firefox-main/search?q=default-font.html&path=&case=false&regexp=false ; such a test would pass at least on Windows which has Cambria Math and Mac which has STIX Two.

Blocks: 1891673

I guess we also need a test to check that with a DisplayOperatorMinHeight that is too small, browsers don't try to work around the value in order to pick a larger variant.

A small fix for the Harfbuzz patch has been merged just now, so we have to wait a bit for the release and update our copy again. With that, and removing the workaround, I already confirmed that it works for Cambria Math.

Removes the workaround when DisplayOperatorMinHeight is too small. This
previously fixed an issue with Cambria Math, but harfbuzz was updated
to address this so this is no longer necessary.

Adds two tests. One detects if an engine is using a workaround like the
one we removed to compensate for the constant being too small. The other
checks whether the system font has a reasonable DisplayOperatorMinHeight
value, mainly to see if Cambria Math on Windows is fixed.

Spec: https://w3c.github.io/mathml-core/#layout-of-operators

Assignee: nobody → eri
Status: NEW → ASSIGNED
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/54605 for changes under testing/web-platform/tests
Status: ASSIGNED → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → 144 Branch
Upstream PR merged by moz-wptsync-bot
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: