Descriptions from the MathML spec: http://www.w3.org/TR/MathML/chapter3.html#presm.scriptlevel http://www.w3.org/TR/MathML/chapter2.html#interf.toplevel The work here is essentially the same as in bug 764996, except that one has to add a -moz-display-style CSS property. * In layout/style/*, add all the stuff to implement the -moz-display-style property. attachment 295888 [details] [diff] [review] might help, but here we just want a boolean property so that can be simpler. Ask to CSS folks on IRC for more details. * In layout/mathml/* replace all the references to displaystyle (or DISPLAYSTYLE etc) to use -moz-display-style instead. (attachment 692250 [details] [diff] [review] might be helpful) * In content/mathml/content/src/nsMathMLElement.cpp map math@displaystyle & mstyle@displaystyle to -moz-display-style (attachment 691943 [details] [diff] [review] might be helpful) * In layout/mathml/mathml.css add a rule math[display="block"] { -moz-display-style: true } Rationale: - http://lists.w3.org/Archives/Public/www-math/2012Sep/0002.html - https://wiki.mozilla.org/MathML:mstyle - and in particular, this will hopefully fix 832800.

In layout/mathml/mathml.css, I guess -moz-display-style should be reset to false on the math element, unless it is math[display="block"].

I want to work on this bug.I am well acquainted with the knowledge of c/c++.I think it this first bug would be a great opportunity for me.

Looking at Bablu Kumar's Bugzilla activity, he commented on several bugs during one week of March, offering to work on them but never got time to work on them. So I guess I can take this bug.

The patches fix bug 713606 and bug 832800, issues with displaystyle on math & mtable and also allows to compute script-level on mfrac with pure CSS rules. However, an assertion shows up in the new test displaystyle-3.html, so I'll have to analyze why.

So the new assertion mentioned in comment 9 is in nsStyleChangeList::AppendChange with "Shouldn't be trying to restyle non-elements directly". It seems that this ends up being called with the text frames of token MathML elements. There is an XXX comment from Boris: // XXXbz we should make this take Element instead of nsIContent I wonder if fixing this would help here. However, I'm not sure at all why this restyle is called on the text frames. Since script level might be involved here, I wonder if that's related to this other comment from Robert: // XXXroc this does a ContentStatesChanged, is it safe to call here? If // not we should do it in a post-reflow callback. Can you give me more hints about how to try to do these changes so that I see if that solves my problem here.

> However, I'm not sure at all why this restyle is called on the text frames. That's the real question, yes. I don't even see any changes to nsStyleFont::CalcDifference to take the new member into account, so why would we think a restyle is needed on those text frames?

(In reply to Boris Zbarsky [:bz] from comment #11) > > However, I'm not sure at all why this restyle is called on the text frames. > > That's the real question, yes. > > I don't even see any changes to nsStyleFont::CalcDifference to take the new > member into account, so why would we think a restyle is needed on those text > frames? OK, I just tried with a fresh build and for some reason the assertion no longer shows up. There was still an issue with the rendering not updated in displaystyle-3.html, but updating nsStyleFont::CalcDifference solved that. So all these new tests pass now. I've sent the patches to try server.

unbitrot this patch

Just doing the change for nsStyleFont::CalcDifference...

Comment on attachment 8356527 [details] [diff] [review] Part 1 - CSS Review of attachment 8356527 [details] [diff] [review]: ----------------------------------------------------------------- r=me with comments addressed. ::: layout/style/nsCSSPropList.h @@ +3356,5 @@ > // property when mUnsafeRulesEnabled is set. > CSS_PROPERTY_PARSE_VALUE, > "", > + // script-level can take Auto, Integer and Number values, but only Auto > + // ("increment if parent is not in displaystyle") and Integer I'll assume this logic ("increment if parent is not in displaystyle") makes sense. ::: layout/style/nsCSSProps.cpp @@ +1359,5 @@ > +const int32_t nsCSSProps::kDisplayStyleKTable[] = { > + eCSSKeyword_none, NS_MATHML_DISPLAYSTYLE_NONE, > + eCSSKeyword_true, NS_MATHML_DISPLAYSTYLE_TRUE, > + eCSSKeyword_UNKNOWN,-1 > +}; "none" and "true" aren't good values for this property; we should avoid "true/false" values, and "none" and "true" don't seem to be complementary. How about "display" and "text" make sense (to match \displaystyle and \textstyle mentioned in the spec)? ::: layout/style/nsRuleNode.cpp @@ +1916,1 @@ > // MathML is not in use. Therefore if all but four s/four/five/ @@ +3462,5 @@ > // "absolute" > aFont->mScriptLevel = ClampTo8Bit(int32_t(scriptLevelValue->GetFloatValue())); > } > + else if (eCSSUnit_Auto == scriptLevelValue->GetUnit()) { > + // auto I think this -- and the eCSSUnit_Integer case above -- should be setting aCanStoreInRuleTree to false since it relies on the inherited value. ::: layout/style/nsStyleConsts.h @@ +553,5 @@ > #define NS_MATHML_MATHVARIANT_TAILED 16 > #define NS_MATHML_MATHVARIANT_LOOPED 17 > #define NS_MATHML_MATHVARIANT_STRETCHED 18 > > +// See nsStyleFont nsStyleFont::mDisplayStyle

(In reply to Cameron McCormack (:heycam) from comment #16) > ::: layout/style/nsCSSProps.cpp > @@ +1359,5 @@ > > +const int32_t nsCSSProps::kDisplayStyleKTable[] = { > > + eCSSKeyword_none, NS_MATHML_DISPLAYSTYLE_NONE, > > + eCSSKeyword_true, NS_MATHML_DISPLAYSTYLE_TRUE, > > + eCSSKeyword_UNKNOWN,-1 > > +}; > > "none" and "true" aren't good values for this property; we should avoid > "true/false" values, and "none" and "true" don't seem to be complementary. > How about "display" and "text" make sense (to match \displaystyle and > \textstyle mentioned in the spec)? The MathML concept is really displaystyle="true" or displaystyle="false". These correspond to display=block and display=inline for the <math> root, so perhaps these can be used to avoid introducing new keywords. Otherwise, I'm fine with introducing keywords "displaystyle" and "textstyle" although they are more TeX concepts (e.g. \scriptstyle and \scriptscriptstyle are also displaystyle=false). What do you prefer?

Yeah, -moz-display-style: {block,inline} sounds OK to me. I worry slightly that -moz-display-style might be confusing -- it's not really obvious that it's MathML specific -- but not enough to suggest a name that doesn't match the attribute.

changing none/true => inline/block for -moz-display-style

(In reply to Cameron McCormack (:heycam) from comment #18) > Yeah, -moz-display-style: {block,inline} sounds OK to me. I worry slightly > that -moz-display-style might be confusing -- it's not really obvious that > it's MathML specific -- but not enough to suggest a name that doesn't match > the attribute. I am. How about '-moz-math-display'? 'style' is redundant. It doesn't really matter that it doesn't match the attribute, it's close enough.

WFM

Comment on attachment 8359144 [details] [diff] [review] Part 3 - MathML frames Review of attachment 8359144 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/mathml/nsMathMLmencloseFrame.cpp @@ +350,5 @@ > if (IsToDraw(NOTATION_LONGDIV) || IsToDraw(NOTATION_RADICAL)) { > nscoord phi; > // Rule 11, App. G, TeXbook > // psi = clearance between rule and content > + if (StyleFont()->mMathDisplay) You should check == NS_MATHML_DISPLAYSTYLE_BLOCK, much clearer than using a boolean. ::: layout/mathml/nsMathMLmfracFrame.cpp @@ +241,5 @@ > nscoord denShift1, denShift2; > > GetNumeratorShifts(fm, numShift1, numShift2, numShift3); > GetDenominatorShifts(fm, denShift1, denShift2); > + if (StyleFont()->mMathDisplay) { Same here and everywhere else in this patch ::: layout/mathml/nsMathMLmoFrame.cpp @@ +561,5 @@ > } > > static uint32_t > GetStretchHint(nsOperatorFlags aFlags, nsPresentationData aPresentationData, > + bool aIsVertical, bool aIsDisplayStyle) Should be a uint8_t. Or to be even clearer, pass an nsStyleFont so we can get mMathDisplay from it.

https://hg.mozilla.org/integration/mozilla-inbound/rev/72f18544d8ec https://hg.mozilla.org/integration/mozilla-inbound/rev/3703497837ee https://hg.mozilla.org/integration/mozilla-inbound/rev/0e9323ed1e70 https://hg.mozilla.org/integration/mozilla-inbound/rev/eebf26497204 https://hg.mozilla.org/integration/mozilla-inbound/rev/eb77025a3196

https://hg.mozilla.org/mozilla-central/rev/72f18544d8ec https://hg.mozilla.org/mozilla-central/rev/3703497837ee https://hg.mozilla.org/mozilla-central/rev/0e9323ed1e70 https://hg.mozilla.org/mozilla-central/rev/eebf26497204 https://hg.mozilla.org/mozilla-central/rev/eb77025a3196