569124 - The top-Level math element should support the attributes of <mstyle>

Status: RESOLVED FIXED

(Core :: MathML, defect)

Description

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

MathML3 says:

"The math element accepts any of the attributes that can be set on Section 3.3.4 Style Change <mstyle>"
http://www.w3.org/TR/MathML3/chapter2.html#interf.toplevel.atts

I think there are two modifications to make:

* For attributes mapped to style:
  Make nsGkAtoms::math use mstyleMap instead of commonPresMap
  http://mxr.mozilla.org/mozilla-central/source/content/mathml/content/src/nsMathMLElement.cpp#168

* For attributes taken from nsMathMLFrame::GetAttribute, make it possible to access the attributes of <math/>
  http://mxr.mozilla.org/mozilla-central/source/layout/mathml/nsMathMLFrame.cpp#242
  It seems that we only need to allow the mPresentationData.mstyle's to point to the parent <math/>, instead of being null.
  I've tried to add mPresentationData.flags = frame; here
  http://mxr.mozilla.org/mozilla-central/source/layout/mathml/nsMathMLFrame.cpp#229
  but it does not seem to work, I need to study it more...

Comment 1

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

Attachment: Patch V1

Comment 2

[Karl Tomlinson (:karlt)]

Try server is seeing a difference in the vertical position of the a/b mfrac in the reftest.
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1301287648.1301289198.4395.gz#err0

Maybe try adding an mrow (or, if necessary, an empty mstyle) around the mfrac in math-as-mstyle-1.xhtml.

Comment 3

[Karl Tomlinson (:karlt)]

layout/tools/reftest/reftest-analyzer.xhtml is useful but don't include the "NEXT ERROR" in the pasted selection.

Comment 4

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

Attachment: Patch V2

Comment 5

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

> Maybe try adding an mrow (or, if necessary, an empty mstyle) around the mfrac
> in math-as-mstyle-1.xhtml.

Adding an empty mstyle does not seem to help:

http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1301900330.1301901837.27363.gz#err0

I'll try with an mrow next time, but I doubt it is going to work.

Comment 6

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

> I'll try with an mrow next time, but I doubt it is going to work.

Same issue with the mrow:

http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1301997879.1301999385.32014.gz#err0

Comment 7

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

(In reply to comment #2)
> Try server is seeing a difference in the vertical position of the a/b mfrac in
> the reftest.
> http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1301287648.1301289198.4395.gz#err0
> 
> Maybe try adding an mrow (or, if necessary, an empty mstyle) around the mfrac
> in math-as-mstyle-1.xhtml.

Any idea on why this difference happens? Maybe using another scriptlevel for the math element changes its computed padding/margin.

Comment 8

[Karl Tomlinson (:karlt)]

I wonder whether scriptlevel might be affecting the font size (I don't remember clearly what is does), which would affect the vertical layout.
Is there a different attribute that can be used in the test?

Comment 9

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

The other candidates were scriptminsize or scriptsizemultiplier, which are also probably involved in font size computation. mathcolor/mathbackground are now usable for all elements, so testing them won't be relevant.

We can maybe modify the test to set some CSS properties of the math element in order to prevent the vertical shift.

Comment 10

[Karl Tomlinson (:karlt)]

scriptsizemultiplier would only affect changes in scriptlevel so shouldn't affect the toplevel element.

Comment 11

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

(In reply to comment #10)
> scriptsizemultiplier would only affect changes in scriptlevel so shouldn't
> affect the toplevel element.

OK, I'll try this.

Comment 12

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

(In reply to comment #11)
> (In reply to comment #10)
> > scriptsizemultiplier would only affect changes in scriptlevel so shouldn't
> > affect the toplevel element.
> 
> OK, I'll try this.

Done. The TryServer did not detect any reftest failure this time. Thanks.

Comment 13

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

Attachment: Patch V3

Comment 14

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/52e0b9902f48

Comment 15

[Karl Tomlinson (:karlt)]

Backed out of aurora for 5 due to bug 655451:
http://hg.mozilla.org/releases/mozilla-aurora/rev/f18cccf550ba

Comment 16

[Karl Tomlinson (:karlt)]

Looks like the patch in bug 655451 won't be reviewed in time for 6.

Comment 17

[j.j.]

dev-doc-needed again.
Documentation was not complete, 
https://developer.mozilla.org/en/MathML/Element/mstyle
needs update.

https://developer.mozilla.org/en/Firefox_6_for_developers
needs reverting if comment 16 is tue.

Comment 18

[Justin Wood (:Callek)]

(In reply to comment #15)
> Backed out of aurora for 5 due to bug 655451:
> http://hg.mozilla.org/releases/mozilla-aurora/rev/f18cccf550ba

Also backed out of aurora for 6
http://hg.mozilla.org/releases/mozilla-aurora/rev/26d6981b3d6a

(marking t-m back down to mozilla7, since we still have the patch in Nightly)

Comment 19

[j.j.]

(In reply to comment #17)
> https://developer.mozilla.org/en/Firefox_6_for_developers
> needs reverting if comment 16 is true.

done

Comment 20

[Florian Scholz (Open Web Docs)]

(In reply to comment #17)
> dev-doc-needed again.
> Documentation was not complete, 
> https://developer.mozilla.org/en/MathML/Element/mstyle
> needs update.
> 
> https://developer.mozilla.org/en/Firefox_6_for_developers
> needs reverting if comment 16 is tue.

Those are different things introduced in different versions of Gecko. Documentation for mstyle will be addressed in bug 569125.

Docs are complete here:
https://developer.mozilla.org/en/Firefox_7_for_developers#MathML
https://developer.mozilla.org/en/MathML/Element/math