The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in mozilla7

Status

()

Core
MathML
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: fredw, Assigned: fredw)

Tracking

({dev-doc-complete})

Trunk
mozilla7
dev-doc-complete
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

7 years ago
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...
(Assignee)

Updated

7 years ago
Depends on: 569125
(Assignee)

Comment 1

6 years ago
Created attachment 500675 [details] [diff] [review]
Patch V1
Assignee: nobody → fred.wang
Status: NEW → ASSIGNED
(Assignee)

Updated

6 years ago
Keywords: dev-doc-needed
(Assignee)

Updated

6 years ago
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.
(Assignee)

Comment 4

6 years ago
Created attachment 522592 [details] [diff] [review]
Patch V2
(Assignee)

Comment 5

6 years ago
> 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.
(Assignee)

Comment 6

6 years ago
> 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
(Assignee)

Comment 7

6 years ago
(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?
(Assignee)

Comment 9

6 years ago
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.
(Assignee)

Comment 11

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

OK, I'll try this.
(Assignee)

Comment 12

6 years ago
(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.
(Assignee)

Comment 13

6 years ago
Created attachment 524802 [details] [diff] [review]
Patch V3
Attachment #500675 - Attachment is obsolete: true
Attachment #522592 - Attachment is obsolete: true
Attachment #524802 - Flags: review?(karlt)
Attachment #524802 - Flags: review?(karlt) → review+
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/52e0b9902f48
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite+
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.2
(Assignee)

Updated

6 years ago
Keywords: dev-doc-needed
(Assignee)

Updated

6 years ago
Keywords: dev-doc-complete
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

Comment 17

6 years ago
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.
Keywords: dev-doc-complete → dev-doc-needed
(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

Comment 19

6 years ago
(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
Keywords: dev-doc-needed → dev-doc-complete
(Assignee)

Updated

6 years ago
Depends on: 669719
You need to log in before you can comment on or make changes to this bug.