1924672 - Support for CSS units in MathMLElement::ParseNumericValue

Status: RESOLVED FIXED

(Core :: MathML, enhancement)

Description

[Frédéric Wang (:fredw)]

Small step toward bug 1574751.

See https://searchfox.org/mozilla-central/rev/754074e05178e017ef6c3d8e30428ffa8f1b794d/dom/mathml/MathMLElement.cpp#309

Comparing https://drafts.csswg.org/css-values/#length-value Vs https://www.w3.org/TR/MathML3/chapter2.html#fund.units the following are missing:

ch => eCSSUnit_Char
cap => eCSSUnit_CapHeight
ic => eCSSUnit_Ideographic
vm => eCSSUnit_VW
vh => eCSSUnit_VH
vi, vb => eCSSUnit_VW or eCSSUnit_VH
vmin => eCSSUnit_VMin
vmax => eCSSUnit_VMax
rem => eCSSUnit_RootEM
lh => eCSSUnit_LineHeight
rlh => eCSSUnit_RootLineHeight

Comment 1

[Robert Longson [:longsonr]]

Maybe the SVGLengthUnits macros could help?

https://searchfox.org/mozilla-central/source/dom/svg/SVGLengthUnits.h

Or roll your own perhaps?

Comment 2

[Frédéric Wang (:fredw)]

Thanks, I don't know... I think ideally we would just like to map the MathML attributes that currently rely on MathMLElement::ParseNumericValue to CSS length properties, like we do for many other MathML attributes. But using internal CSS properties instead. But we are currently stuck on this because of legacy MathML3 syntax. Just tweaking MathMLElement::ParseNumericValue could be a first step to support these units for now.

Comment 3

[Robert Longson [:longsonr]]

The SVG code for that is: https://searchfox.org/mozilla-central/source/dom/svg/SVGElement.cpp#1071

Comment 4

[Frédéric Wang (:fredw)]

Assigning to Harry, he is willing to give a try.

Comment 5

[Harry Chen]

Attachment: Bug 1924672 - Add WPT tests and support for more CSS units in MathML. r?fredw

vi and vb need special handling since they depend on the writing
mode; see https://bugzilla.mozilla.org/show_bug.cgi?id=1077525
and https://drafts.csswg.org/css-values/#relative-lengths.

Comment 6

[Frédéric Wang (:fredw)]

https://treeherder.mozilla.org/jobs?repo=try&revision=17968fb1405e7d11959ca661000c024407d59a43

Comment 7

[Frédéric Wang (:fredw)]

(In reply to Frédéric Wang (:fredw) from comment #6)

https://treeherder.mozilla.org/jobs?repo=try&revision=17968fb1405e7d11959ca661000c024407d59a43

This is green but lando warns that some comments are unresolved. Can you please go over https://phabricator.services.mozilla.com/differential/revision/inlines/230738/ and check whether they have all be addressed / or no longer relevant (and if so mark them as Done). Thanks!

Comment 8

[Harry Chen]

Oops sorry, I managed to miss marking a few as done, but I believe they should all be addressed!

Comment 9

[Frédéric Wang (:fredw)]

https://phabricator.services.mozilla.com/D230738 failed to land when I tried last Friday, it has some conflicts https://lando.moz.tools/D230738/ ; can you please rebase?

Comment 10

[Harry Chen]

Sure, should be done

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4a74b1fc4b79
Add WPT tests and support for more CSS units in MathML. r=fredw,emilio

Comment 12

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/52262 for changes under testing/web-platform/tests

Comment 13

[amarc]

Backed out for causing build bustages @ nsMathMLmfracFrame.cpp

• Backout link
• Push with failures
• Failure Log

Comment 14

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 15

[Harry Chen]

Sorry, my Firefox build isn't working, it should be fixed now. (Lando seems to be down for maintenance)

Comment 16

[Frédéric Wang (:fredw)]

(In reply to Harry Chen from comment #15)

Sorry, my Firefox build isn't working, it should be fixed now. (Lando seems to be down for maintenance)

So have you built the updated patch locally? If not, I guess better to send to try server again before landing.

Comment 17

[Harry Chen]

Not yet -- I'm trying to send it to try now but Lando is down, I'll try again soon

Comment 18

[Frédéric Wang (:fredw)]

OK, I guess you mean uploading to phabricator, not landing via lando.

Comment 19

[Harry Chen]

(In reply to Frédéric Wang (:fredw) from comment #18)

OK, I guess you mean uploading to phabricator, not landing via lando.

Sorry, I meant the Lando try server, but in any case I don't think I have permissions (unless the landing job ID 129244 exists?). Would it be possible to send this to try please?

Comment 20

[Harry Chen]

Nevermind, looks green (failure seems unrelated): https://treeherder.mozilla.org/jobs?repo=try&revision=5b27c0233717c94a622de18f0fe2791f1dbcd77d

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2bd090dba619
Add WPT tests and support for more CSS units in MathML. r=fredw,emilio

Comment 22

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/2bd090dba619

Comment 23

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 24

[Sebastian Zartner [:sebo]]

This addition should probably be mentioned in the related browser compat data.

Sebastian