1923569 - Implement math-shift: compact (TeX's "cramped")

Status: RESOLVED FIXED

(Core :: MathML, enhancement)

Description

[Frédéric Wang Nélar]

• Spec:
  • https://w3c.github.io/mathml-core/#the-math-shift
  • https://w3c.github.io/mathml-core/#user-agent-stylesheet
  • https://learn.microsoft.com/en-us/typography/opentype/spec/math
• WPT:
  • https://wpt.fyi/results/mathml/presentation-markup/scripts/cramped-001.html
  • https://wpt.fyi/results/mathml/presentation-markup/scripts/subsup-parameters-1.html

This should replace what we call "compressed" or "cramped" in our code:
https://searchfox.org/mozilla-central/search?q=symbol:M_fbf1ee6a28c1ad46&redirect=false
https://searchfox.org/mozilla-central/search?q=compressed%7Ccramped&path=layout%2Fmathml&case=false&regexp=true

Comment 1

[eri]

Attachment: Bug 1923569 - Implement math-shift: compact. r?fredw,emilio

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/ed9282cc218f
https://hg.mozilla.org/integration/autoland/rev/57e38576c7ac
Implement math-shift: compact. r=fredw,emilio,firefox-style-system-reviewers

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/4ba9672c619f
https://hg.mozilla.org/integration/autoland/rev/d23575ccff6d
Revert "Bug 1923569 - Implement math-shift: compact. r=fredw,emilio,firefox-style-system-reviewers" for causing non-unified build bustages @ nsMathMLmrootFrame.cpp

Comment 4

[amarc]

Backed out for causing non-unified build bustages @ nsMathMLmrootFrame.cpp

• Backout link
• Push with failures
• Failure Log

Comment 5

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

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

Comment 6

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

Upstream PR merged by moz-wptsync-bot

Comment 7

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

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

Comment 8

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

Upstream PR merged by moz-wptsync-bot

Comment 9

[eri]

(In reply to amarc from comment #4)

I just commented on Phabricator, but I'll send it here too for the needinfo request:

I am very sorry about the busted built. It was indeed missing an import in layout/mathml/nsMathMLmrootFrame.cpp. I'm really puzzled about how that could slip by, since it was already missing in the first revision, but it didn't fail to build locally or in any of the try runs (1, 2, 3, 4). In any case, I apologize for the oversight. I have pushed a new revision with the import and started another try run.

Comment 10

[Frédéric Wang Nélar]

(In reply to eri from comment #9)

(In reply to amarc from comment #4)

I just commented on Phabricator, but I'll send it here too for the needinfo request:

I am very sorry about the busted built. It was indeed missing an import in layout/mathml/nsMathMLmrootFrame.cpp. I'm really puzzled about how that could slip by, since it was already missing in the first revision, but it didn't fail to build locally or in any of the try runs (1, 2, 3, 4). In any case, I apologize for the oversight. I have pushed a new revision with the import and started another try run.

likely you need to try a non-unified build to be sure

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/49a741920968
https://hg.mozilla.org/integration/autoland/rev/881c4d906e08
Implement math-shift: compact. r=fredw,emilio,firefox-style-system-reviewers

Comment 12

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/881c4d906e08

Comment 13

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

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

Comment 14

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

Upstream PR merged by moz-wptsync-bot

Comment 15

[Donal Meehan [:dmeehan]]

:eri could you consider nominating this for a release note? (Process info)
We could include it in the nightly only release notes.

Comment 16

[eri]

Release Note Request (optional, but appreciated)
[Why is this notable]: It improves MathML rendering in certain situations, bringing it closer to the MathML Core spec and TeX. For example, a superscript inside of a root is slightly shifted downwards to fit correctly. Before Firefox had many rules which didn't cover all of the possible cases, now they are more clearly defined in the mathml.css style sheet.
[Affects Firefox for Android]: Yes.
[Suggested wording]: Implement math-shift compact.
[Links (documentation, blog post, etc)]: https://w3c.github.io/mathml-core/#the-math-shift

Comment 17

[Donal Meehan [:dmeehan]]

Thanks, added to the Fx144 nightly release notes, please allow 30 minutes for the site to update.
Keeping the relnote-firefox flag as ? to keep it on the radar for inclusion in the final Fx144 release notes.

Comment 18

[Dan Minor [:dminor]]

Hi Eri, it looks this patch added new telemetry for css_math_shift to the code generated file dom/base/use_counter_metrics.yaml. If I run ./mach gen-use-counter-metrics after making changes to dom/base/UseCounters.conf, the new telemetry will be removed. The build still works with it removed so I don't think this telemetry is actually being used anywhere.

I'm planning to remove some telemetry in Bug 1970931 anyway, so I can remove the css_math_shift telemetry there if it's unnecessary, I just wanted to check what the intention was. If there is a plan to use this telemetry, you should probably file a follow up bug to add it properly.

Comment 19

[eri]

Hi Dan, thank you for checking that, it's great that you caught it. The telemetry was left over from a previous version of the patch where I was looking at what math-style did. You are right, it is not being used anywhere at the moment, and I don't think there is any plan to do so. If you remove it in you patch that would be great, thank you!

Comment 20

[Dan Minor [:dminor]]

Oops, it looks like rerunning ./mach gen-use-counter-metrics changes the order in the generated file, but leaves the new telemetry in place, so I'll do that.

Comment 21

[Donal Meehan [:dmeehan]]

Added to the Fx144 release notes

Comment 22

[Sebastian Zartner [:sebo]]

This needs to be documented, so adding the dev-doc-needed flag.

Sebastian