2018403 - Consider wrapping children of math/mtd in an anonymous mrow

Status: ASSIGNED

(Core :: MathML, task)

Description

[Frédéric Wang Nélar]

That should help fix bug 236963 (stretching) and probably bug 2018114 (spacing).

This is what Chromium does, I believe.

On the other hand, this will remove our basic (and probably broken) linebreaking support within math/mtd.

Comment 1

[Frédéric Wang Nélar]

Attachment: WIP: Bug 2018403 - Wrap children of <math>/<mtd> into an anonymous <mrow>. r=emilio,eri

Comment 2

[Frédéric Wang Nélar]

Attachment: Testcase

This is showing some inconsistencies in Firefox when an explicit mrow is used as a direct child of the math or when it is not.

Always creating an anonymous mrow would help solve inconsistencies, but we would lose our rudimentary linebreaking support.

Comment 3

[Frédéric Wang Nélar]

Attachment: anonymous-mrow-assert-001.html

Comment 4

[Frédéric Wang Nélar]

Attachment: anonymous-mrow-assert-002.html

Comment 5

[Frédéric Wang Nélar]

Attachment: anonymous-mrow-assert-003.html

Comment 6

[Frédéric Wang Nélar]

Attachment: anonymous-mrow-assert-001.html

Comment 7

[Frédéric Wang Nélar]

Attachment: anonymous-mrow-assert-002.html

Comment 8

[Frédéric Wang Nélar]

Attachment: anonymous-mrow-assert-003.html

Comment 9

[Frédéric Wang Nélar]

Attachment: anonymous-mrow-assert-004.html

Comment 10

[Frédéric Wang Nélar]

Attachment: anonymous-mrow-assert-005.html

Comment 11

[Frédéric Wang Nélar]

https://treeherder.mozilla.org/jobs?repo=try&revision=175f59740bcc38952546c71182653cda626bf07

Comment 12

[Frédéric Wang Nélar]

Attachment: anonymous-mrow-restyling.html

Comment 13

[Frédéric Wang Nélar]

Attachment: WIP: Bug 2018403 - Make math/mtd elements generate an nsMathMLmrowFrame.

Comment 14

[Frédéric Wang Nélar]

As per matrix discussion: https://phabricator.services.mozilla.com/D290881

Comment 15

[Emilio Cobos Álvarez [:emilio]]

Attachment: WIP: Bug 2018403 - Make math use mrow layout.

Comment 16

[Emilio Cobos Álvarez [:emilio]]

So something like comment 15 gives me mostly a progression:

Error Summary
-------------
/mathml/presentation-markup/operators/mo-lspace-rspace-3.html
  UNEXPECTED-PASS /mathml/presentation-markup/operators/mo-lspace-rspace-3.html - Testing http://web-platform.test:8000/mathml/presentation-markup/operators/mo-lspace-rspace-3.html == http://web-platform.test:8000/mathml/presentation-markup/operators/mo-lspace-rspace-3-ref.html
/mathml/presentation-markup/operators/painting-stretchy-operator-001.html
  UNEXPECTED-PASS /mathml/presentation-markup/operators/painting-stretchy-operator-001.html - Testing http://web-platform.test:8000/mathml/presentation-markup/operators/painting-stretchy-operator-001.html == http://web-platform.test:8000/mathml/presentation-markup/operators/painting-stretchy-operator-001-ref.html
/mathml/presentation-markup/operators/embellished-operator-001.html
  UNEXPECTED-PASS mrow (embellished operator)
  UNEXPECTED-PASS mstyle (embellished operator)
  UNEXPECTED-PASS mphantom (embellished operator)
  UNEXPECTED-PASS merror (embellished operator)
  UNEXPECTED-PASS mprescripts (embellished operator)
  UNEXPECTED-PASS none (embellished operator)
  UNEXPECTED-PASS unknown (embellished operator)
  UNEXPECTED-PASS mfenced (embellished operator)
  UNEXPECTED-PASS a (embellished operator)
/mathml/presentation-markup/operators/embellished-operator-002.html
  UNEXPECTED-PASS msub (embellished operator)
  UNEXPECTED-PASS msup (embellished operator)
  UNEXPECTED-PASS msubsup (embellished operator)
  UNEXPECTED-PASS munder (embellished operator)
  UNEXPECTED-PASS mover (embellished operator)
  UNEXPECTED-PASS munderover (embellished operator)
  UNEXPECTED-PASS mmultiscripts (embellished operator)
  FAIL mfrac (embellished operator, from in-flow children) - assert_approx_equals: expected 50 +/- 1 but got 54.16667175292969
runTests/</<@http://web-platform.test:8000/mathml/presentation-markup/operators/embellished-operator-002.html:68:36
Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:2869:25
test@http://web-platform.test:8000/resources/testharness.js:633:30
runTests/<@http://web-platform.test:8000/mathml/presentation-markup/operators/embellished-operator-002.html:65:16
runTests@http://web-platform.test:8000/mathml/presentation-markup/operators/embellished-operator-002.html:58:34
  FAIL mfrac (not embellished operator) - assert_approx_equals: expected 0 +/- 1 but got 4.1666717529296875
runTests/</<@http://web-platform.test:8000/mathml/presentation-markup/operators/embellished-operator-002.html:74:36
Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:2869:25
test@http://web-platform.test:8000/resources/testharness.js:633:30
runTests/<@http://web-platform.test:8000/mathml/presentation-markup/operators/embellished-operator-002.html:71:16
runTests@http://web-platform.test:8000/mathml/presentation-markup/operators/embellished-operator-002.html:58:34
/mathml/presentation-markup/operators/embellished-operator-003.html
  UNEXPECTED-PASS maction (embellished operator)
  FAIL maction (embellished operator, from in-flow children) - assert_approx_equals: expected 50 +/- 1 but got 100
runTests/</<@http://web-platform.test:8000/mathml/presentation-markup/operators/embellished-operator-003.html:70:36
Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:2869:25
test@http://web-platform.test:8000/resources/testharness.js:633:30
runTests/<@http://web-platform.test:8000/mathml/presentation-markup/operators/embellished-operator-003.html:67:16
runTests@http://web-platform.test:8000/mathml/presentation-markup/operators/embellished-operator-003.html:60:32
  UNEXPECTED-PASS maction (embellished operator, one child)
  UNEXPECTED-PASS semantics (embellished operator)
  FAIL semantics (embellished operator, from in-flow children) - assert_approx_equals: expected 50 +/- 1 but got 100
runTests/</<@http://web-platform.test:8000/mathml/presentation-markup/operators/embellished-operator-003.html:70:36
Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:2869:25
test@http://web-platform.test:8000/resources/testharness.js:633:30
runTests/<@http://web-platform.test:8000/mathml/presentation-markup/operators/embellished-operator-003.html:67:16
runTests@http://web-platform.test:8000/mathml/presentation-markup/operators/embellished-operator-003.html:60:32
  UNEXPECTED-PASS semantics (embellished operator, one child)
/mathml/presentation-markup/operators/embellished-operator-004.html
  UNEXPECTED-PASS Vertical stretching performed for math > mo.
  UNEXPECTED-PASS Vertical stretching performed for Stretching for math[display=block] > mo.
  FAIL Vertical stretching performed for math > mrow > mo. - assert_greater_than_equal: expected a number greater than or equal to 100 but got 10
runTests/</<@http://web-platform.test:8000/mathml/presentation-markup/operators/embellished-operator-004.html:35:40
Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:2869:25
test@http://web-platform.test:8000/resources/testharness.js:633:30
runTests/<@http://web-platform.test:8000/mathml/presentation-markup/operators/embellished-operator-004.html:33:15
runTests@http://web-platform.test:8000/mathml/presentation-markup/operators/embellished-operator-004.html:32:61
  FAIL Vertical stretching performed for Stretching for math[display=block] > mrow > mo. - assert_greater_than_equal: expected a number greater than or equal to 100 but got 10
runTests/</<@http://web-platform.test:8000/mathml/presentation-markup/operators/embellished-operator-004.html:35:40
Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:2869:25
test@http://web-platform.test:8000/resources/testharness.js:633:30
runTests/<@http://web-platform.test:8000/mathml/presentation-markup/operators/embellished-operator-004.html:33:15
runTests@http://web-platform.test:8000/mathml/presentation-markup/operators/embellished-operator-004.html:32:61
  UNEXPECTED-PASS Vertical stretching performed for math > mrow > mrow > mo.
  UNEXPECTED-PASS Vertical stretching performed for Stretching for math[display=block] > mrow > mrow > mo.
/mathml/presentation-markup/operators/mo-lspace-rspace-5.html
  UNEXPECTED-PASS lspace/rspace spacing correctly set around operator (child of math, not embellished, LTR, normal text).
  UNEXPECTED-PASS lspace/rspace spacing correctly set around operator (child of math, not embellished, LTR, largeop variant).
  UNEXPECTED-PASS lspace/rspace spacing correctly set around operator (child of math, not embellished, LTR, stretchy variant).
  UNEXPECTED-PASS lspace/rspace spacing correctly set around operator (child of math, not embellished, LTR, glyph assembly).
  UNEXPECTED-PASS lspace/rspace spacing correctly set around operator (child of math, embellished, LTR, normal text).
  UNEXPECTED-PASS lspace/rspace spacing correctly set around operator (child of math, embellished, LTR, largeop variant).
  UNEXPECTED-PASS lspace/rspace spacing correctly set around operator (child of math, embellished, LTR, stretchy variant).
  UNEXPECTED-PASS lspace/rspace spacing correctly set around operator (child of math, embellished, LTR, glyph assembly).
  UNEXPECTED-PASS lspace/rspace spacing correctly set around operator (child of math, not embellished, RTL, normal text).
  UNEXPECTED-PASS lspace/rspace spacing correctly set around operator (child of math, not embellished, RTL, largeop variant).
  UNEXPECTED-PASS lspace/rspace spacing correctly set around operator (child of math, not embellished, RTL, stretchy variant).
  UNEXPECTED-PASS lspace/rspace spacing correctly set around operator (child of math, not embellished, RTL, glyph assembly).
  UNEXPECTED-PASS lspace/rspace spacing correctly set around operator (child of math, embellished, RTL, normal text).
  UNEXPECTED-PASS lspace/rspace spacing correctly set around operator (child of math, embellished, RTL, largeop variant).
  UNEXPECTED-PASS lspace/rspace spacing correctly set around operator (child of math, embellished, RTL, stretchy variant).
  UNEXPECTED-PASS lspace/rspace spacing correctly set around operator (child of math, embellished, RTL, glyph assembly).

I haven't investigated the regression but that seems mostly straight-forward?

Comment 17

[Emilio Cobos Álvarez [:emilio]]

https://treeherder.mozilla.org/jobs?repo=try&landoInstance=lando-prod&landoCommitID=189780

Comment 18

[Frédéric Wang Nélar]

Thank you Emilio for trying this. I think we are getting similar issues as my two previous patches, but this is some progress:

• The regression in /mathml/presentation-markup/operators/embellished-operator-004.html I suspect this is bug 2020658... except that now math is like an mrow, so the failure happens with one mrow instead of two nested mrows. I think we could probably work around that but I'll have to think about it. One thing I'm considering is rewriting stretching to follow the algorithm of the MathML Core spec (adding inline stretch size constraint and block stretch size constraint during Place()) but that will likely be a bigger refactoring.

• Stretching fails in attachment https://bug2018403.bmoattachments.org/attachment.cgi?id=9547515 ; I debugged it and fortunately it's easily fixable by https://phabricator.services.mozilla.com/D291568#inline-1585571 ; D290881 had the same issue (and likely the same fix works).

• Horizontal centering (for <math display="block">) is also broken in https://bug2018403.bmoattachments.org/attachment.cgi?id=9547515 ; my D290881 had the same issue. As I said on matrix, this is supposed to be handled -moz rule: https://searchfox.org/firefox-main/rev/6193f8fd1172486e3ce941b8de251b9d1a4dfa0b/layout/mathml/mathml.css#44 ; this is is covered by https://wpt.fyi/results/mathml/relations/html5-tree/display-1.html but this test regresses with your patch (D290881 had the same issue). Likely a fix for that is to have mrow do the centering itself as described in MathML Core (https://w3c.github.io/mathml-core/#the-top-level-math-element) and as WebKit/Chrome do I believe. the block math should still take the full available width, and center its content within it.

Comment 19

[Frédéric Wang Nélar]

Attachment: Testcase for <math display="block"> and CSS style

Comment 20

[Frédéric Wang Nélar]

Attachment: WIP: Bug 2018403 - Attempt to get the baseline for the inner frame of mtd.

Comment 21

[Frédéric Wang Nélar]

Attachment: mtable-default-vertical-align.html

Comment 22

[Frédéric Wang Nélar]

(In reply to Frédéric Wang Nélar from comment #18)

Thank you Emilio for trying this. I think we are getting similar issues as my two previous patches, but this is some progress:

• Stretching fails in attachment https://bug2018403.bmoattachments.org/attachment.cgi?id=9547515 ; I debugged it and fortunately it's easily fixable by https://phabricator.services.mozilla.com/D291568#inline-1585571 ; D290881 had the same issue (and likely the same fix works).

• Horizontal centering (for <math display="block">) is also broken in https://bug2018403.bmoattachments.org/attachment.cgi?id=9547515 ; my D290881 had the same issue. As I said on matrix, this is supposed to be handled -moz rule: https://searchfox.org/firefox-main/rev/6193f8fd1172486e3ce941b8de251b9d1a4dfa0b/layout/mathml/mathml.css#44 ; this is is covered by https://wpt.fyi/results/mathml/relations/html5-tree/display-1.html but this test regresses with your patch (D290881 had the same issue). Likely a fix for that is to have mrow do the centering itself as described in MathML Core (https://w3c.github.io/mathml-core/#the-top-level-math-element) and as WebKit/Chrome do I believe. the block math should still take the full available width, and center its content within it.

So these two bugs should be fixed in the latest version of https://phabricator.services.mozilla.com/D290881

Now I'm trying to fix the regression with /mathml/presentation-markup/tables/table-002.html and /mathml/presentation-markup/tables/table-003.html / attachment 9567602 [details] for tables.

I tried https://phabricator.services.mozilla.com/D292767 but the MathML code does some fancy thing to save reflow output and I don't see a straightforward way to determine the baseline of the inner frame for mtd.

Comment 23

[Emilio Cobos Álvarez [:emilio]]

Sorry for the lag getting to this... It seems nsMathMLmtdInnerFrame::Reflow or so could save the BlockStartAscent rather than trying to recompute it after the fact, can't it?