Last Comment Bug 449396 - MATHML: <mstyle> attribute mathvariant not applied to underlying <mi>
: MATHML: <mstyle> attribute mathvariant not applied to underlying <mi>
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: MathML (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla6
Assigned To: Frédéric Wang (:fredw)
:
: Anthony Jones (:kentuckyfriedtakahe, :k17e)
Mentors:
http://metatype.it/firefox-testcase.xml
: 571529 (view as bug list)
Depends on: mstyle
Blocks: mathvariant mathml-screenshots
  Show dependency treegraph
 
Reported: 2008-08-06 07:30 PDT by Matteo Centonza
Modified: 2011-05-13 00:44 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Another testcase (830 bytes, application/xhtml+xml)
2009-02-02 13:42 PST, distler
no flags Details

Description Matteo Centonza 2008-08-06 07:30:56 PDT
User-Agent:       Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9.0.1) Gecko/2008071616 CentOS/3.0.1-1.el5.centos Firefox/3.0.1
Build Identifier: Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9.0.1) Gecko/2008071616 CentOS/3.0.1-1.el5.centos Firefox/3.0.1

The mathml test at the above url shows how firefox currently doesn't seem to honor certain mathvariant values inherited by <mi> from <mstyle>.



Reproducible: Always

Steps to Reproduce:
1.Open firefox
2.Connect to http://metatype.it/firefox-testcase.xml
3.Read the test explanation
Actual Results:  
<mstyle> attribute mathvariant is not correctly inherited by underlying <mi>

Expected Results:  
<mi> should inherit mathvariant from outer <mstyle>, like partially does in case of mathvariant="sans-serif".
Comment 1 Karl Tomlinson (:karlt) 2008-08-06 20:17:39 PDT
mstyle is interesting.  Thanks for reporting.

http://www.w3.org/TR/MathML2/chapter3.html#presm.mstyle
Comment 2 distler 2009-02-02 13:42:46 PST
Created attachment 360149 [details]
Another testcase

In the above testcase, you see the result of
   <math><mi>B</mi><mstyle mathvariant="..."><mi>B</mi></mstyle><mstyle mathvariant="..."><mi>BB</mi></mstyle></math>

As you can see, the results vary wildly, depending on which mathvariant is used and whether the <mi> element contains a 1- or 2-character token.
Comment 3 Ahmad Syukri 2009-03-27 21:44:55 PDT
Is this duplicate of bug 297461?
Comment 4 distler 2009-08-27 13:15:55 PDT
A workaround seems to be the following CSS:

[mathvariant="bold"] *  {
  font-style: normal;
  font-variant: normal;
  font-weight: bold;
}
[mathvariant="italic"] *  {
  font-style: italic;
  font-variant: normal;
  font-weight: normal;
}
[mathvariant="bold-italic"] *  {
  font-style: italic;
  font-variant: normal;
  font-weight: bold;
}
[mathvariant="sans-serif"] *  {
  font-style: normal;
  font-variant: normal;
  font-weight: normal;
}
[mathvariant="bold-sans-serif"] *  {
  font-style: normal;
  font-variant: normal;
  font-weight: bold;
}
[mathvariant="sans-serif-italic"] *  {
  font-style: italic;
  font-variant: normal;
  font-weight: normal;
}
[mathvariant="sans-serif-bold-italic"] *  {
  font-style: italic;
  font-variant: normal;
  font-weight: bold;
}
Comment 5 Frédéric Wang (:fredw) 2009-11-14 02:09:01 PST
(In reply to comment #3)
> Is this duplicate of bug 297461?

The reporter of bug 297461 says that the problem he indicated was about extra "+" signs. Anyway, the test http://www.w3.org/Math/testsuite/build/main/Presentation/TokenElements/mi/miSfontsize9-full.xhtml does not use the mathvariant attribute.
Comment 6 Frédéric Wang (:fredw) 2010-04-04 05:12:42 PDT
Proposed fix: add info in nsPresentationData to encode explicit values of mathvariant on a mstyle an transmit this info to the descendants. Use this flag in nsMathMLTokenFrame::SetTextStyle() when setting the _moz_math_fontstyle_ attribute.
Comment 7 Karl Tomlinson (:karlt) 2010-04-06 00:02:27 PDT
I wonder whether there may be other attributes with non-inherited default values that are also not being picked up from mstyle.

(In reply to comment #6)
> Proposed fix: add info in nsPresentationData to encode explicit values of
> mathvariant on a mstyle an transmit this info to the descendants.

It looks like nsPresentationData::mstyle and nsMathMLFrame::GetAttribute() were intended for getting this information:

http://hg.mozilla.org/mozilla-central/annotate/7447cdd1f110/layout/mathml/nsIMathMLFrame.h#l283
http://hg.mozilla.org/mozilla-central/annotate/7447cdd1f110/layout/mathml/nsMathMLFrame.cpp#l242

This comment says "Don't use nsMathMLFrame::GetAttribute for mathvariant or fontstyle as default values are not inherited" 
http://hg.mozilla.org/mozilla-central/annotate/7447cdd1f110/layout/mathml/nsMathMLTokenFrame.cpp#l72
but I can't see that nsMathMLFrame::GetAttribute() makes any assumptions about inheritance.
Comment 8 Frédéric Wang (:fredw) 2010-04-06 03:07:00 PDT
I've just read the MathML3 spec again and apparently the default value is always normal (or italic for a mi with a single character). In that case, our implementation is correct but I don't see what is the point of allowing mathvariant on <mstyle/> if the value is never applied to the descendants.
Comment 9 Karl Tomlinson (:karlt) 2010-04-06 17:44:20 PDT
I think the point is that attributes on an mstyle element change the default values for all descendants.  This is not inheritance (so I've changed the bug summary).

http://www.w3.org/TR/MathML3/chapter3.html#presm.mstyle

"Loosely speaking, the effect of the mstyle element is to change the default value of an attribute for the elements it contains."

"Other attributes, such as linethickness on mfrac, have default values that are not normally inherited. That is, if the linethickness  attribute is not set on the start tag of an mfrac  element, it will normally use the default value of "1", even if it was contained in a larger mfrac element that set this attribute to a different value. For attributes like this, specifying a value with an mstyle element has the effect of changing the default value for all elements within its scope. The net effect is that setting the attribute value with mstyle propagates the change to all the elements it contains directly or indirectly, except for the individual elements on which the value is overridden. Unlike in the case of inherited attributes, elements that explicitly override this attribute have no effect on this attribute's value in their children."
Comment 10 Frédéric Wang (:fredw) 2010-04-07 02:43:11 PDT
OK, thanks for the update. In my opinion, we should just use GetAttribute here:

http://hg.mozilla.org/mozilla-central/annotate/7447cdd1f110/layout/mathml/nsMathMLTokenFrame.cpp#l79

and here:

http://hg.mozilla.org/mozilla-central/annotate/7447cdd1f110/layout/mathml/nsMathMLTokenFrame.cpp#l336

It seems that the code comment you give in comment 7 was due to the same bad interpretation as mine.
Comment 11 Karl Tomlinson (:karlt) 2010-04-07 15:47:48 PDT
I'm guessing that _moz_math_fontstyle_ will still need to be set in SetTextStyle() if GetAttribute finds an attribute but HasAttr doesn't.
If that is the case, then GetMathMLFrameType() will find the _moz_math_fontstyle_ and GetAttribute will be unnecessary there.
(But I'm open to any suggestions that don't need _moz_math_fontstyle_.)
Comment 12 Frédéric Wang (:fredw) 2010-04-08 02:32:08 PDT
(In reply to comment #11)
> I'm guessing that _moz_math_fontstyle_ will still need to be set in
> SetTextStyle() if GetAttribute finds an attribute but HasAttr doesn't.
> If that is the case, then GetMathMLFrameType() will find the
> _moz_math_fontstyle_ and GetAttribute will be unnecessary there.

Yes, I think you're right.

> (But I'm open to any suggestions that don't need _moz_math_fontstyle_.)

It would be a good idea to get rid of a private attribute. I'm wondering whether, as in nsMathMLElement::MapMathMLAttributesInto, there is a way to have access to the nsRuleData or similar from the nsMathMLTokenFrame, so we could directly apply the expected style to the token node.
Comment 13 Frédéric Wang (:fredw) 2010-06-11 14:45:56 PDT
*** Bug 571529 has been marked as a duplicate of this bug. ***
Comment 14 Frédéric Wang (:fredw) 2011-05-13 00:44:26 PDT
Fixed by one patch of bug 569125

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