Last Comment Bug 569124 - The top-Level math element should support the attributes of <mstyle>
: The top-Level math element should support the attributes of <mstyle>
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: MathML (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla7
Assigned To: Frédéric Wang (:fredw)
:
Mentors:
Depends on: mstyle 655451 669719
Blocks:
  Show dependency treegraph
 
Reported: 2010-05-30 05:48 PDT by Frédéric Wang (:fredw)
Modified: 2011-07-06 12:38 PDT (History)
6 users (show)
ehsan: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch V1 (4.20 KB, patch)
2011-01-02 07:46 PST, Frédéric Wang (:fredw)
karlt: review+
Details | Diff | Splinter Review
Patch V2 (4.24 KB, patch)
2011-03-28 22:40 PDT, Frédéric Wang (:fredw)
no flags Details | Diff | Splinter Review
Patch V3 (4.27 KB, patch)
2011-04-08 23:20 PDT, Frédéric Wang (:fredw)
karlt: review+
Details | Diff | Splinter Review

Description Frédéric Wang (:fredw) 2010-05-30 05:48:28 PDT
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) 2011-01-02 07:46:42 PST
Created attachment 500675 [details] [diff] [review]
Patch V1
Comment 2 Karl Tomlinson (:karlt) 2011-03-28 15:33:04 PDT
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) 2011-03-28 15:35:46 PDT
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) 2011-03-28 22:40:07 PDT
Created attachment 522592 [details] [diff] [review]
Patch V2
Comment 5 Frédéric Wang (:fredw) 2011-04-04 01:35:17 PDT
> 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) 2011-04-05 03:47:17 PDT
> 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) 2011-04-08 11:49:45 PDT
(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) 2011-04-08 12:49:09 PDT
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) 2011-04-08 13:32:44 PDT
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) 2011-04-08 13:58:31 PDT
scriptsizemultiplier would only affect changes in scriptlevel so shouldn't affect the toplevel element.
Comment 11 Frédéric Wang (:fredw) 2011-04-08 14:21:38 PDT
(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) 2011-04-08 23:18:55 PDT
(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) 2011-04-08 23:20:08 PDT
Created attachment 524802 [details] [diff] [review]
Patch V3
Comment 15 Karl Tomlinson (:karlt) 2011-05-12 18:29:16 PDT
Backed out of aurora for 5 due to bug 655451:
http://hg.mozilla.org/releases/mozilla-aurora/rev/f18cccf550ba
Comment 16 Karl Tomlinson (:karlt) 2011-05-22 20:46:02 PDT
Looks like the patch in bug 655451 won't be reviewed in time for 6.
Comment 17 j.j. 2011-05-24 13:51:43 PDT
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) (Away until Aug 29) 2011-05-24 15:28:48 PDT
(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. 2011-05-28 10:42:04 PDT
(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 [:fscholz] (MDN) 2011-06-25 10:17:38 PDT
(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

Note You need to log in before you can comment on or make changes to this bug.