Closed Bug 569124 Opened 10 years ago Closed 9 years ago

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

Categories

(Core :: MathML, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla7

People

(Reporter: fredw, Assigned: fredw)

References

Details

(Keywords: dev-doc-complete)

Attachments

(1 file, 2 obsolete files)

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...
Depends on: mstyle
Attached patch Patch V1 (obsolete) — Splinter Review
Assignee: nobody → fred.wang
Status: NEW → ASSIGNED
Keywords: dev-doc-needed
Attachment #500675 - Flags: review?(karlt)
Attachment #500675 - Flags: review?(karlt) → review+
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.
layout/tools/reftest/reftest-analyzer.xhtml is useful but don't include the "NEXT ERROR" in the pasted selection.
Attached patch Patch V2 (obsolete) — Splinter Review
> 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.
> 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
(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.
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?
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.
scriptsizemultiplier would only affect changes in scriptlevel so shouldn't affect the toplevel element.
(In reply to comment #10)
> scriptsizemultiplier would only affect changes in scriptlevel so shouldn't
> affect the toplevel element.

OK, I'll try this.
(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.
Attached patch Patch V3Splinter Review
Attachment #500675 - Attachment is obsolete: true
Attachment #522592 - Attachment is obsolete: true
Attachment #524802 - Flags: review?(karlt)
Attachment #524802 - Flags: review?(karlt) → review+
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/52e0b9902f48
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.2
Keywords: dev-doc-needed
Depends on: 655451
Backed out of aurora for 5 due to bug 655451:
http://hg.mozilla.org/releases/mozilla-aurora/rev/f18cccf550ba
Target Milestone: mozilla5 → mozilla6
Looks like the patch in bug 655451 won't be reviewed in time for 6.
Target Milestone: mozilla6 → Future
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.
(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)
Target Milestone: Future → mozilla7
(In reply to comment #17)
> https://developer.mozilla.org/en/Firefox_6_for_developers
> needs reverting if comment 16 is true.

done
(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
Depends on: 669719
You need to log in before you can comment on or make changes to this bug.