Last Comment Bug 669719 - the <math/> element does not support the displaystyle attribute
: the <math/> element does not support the displaystyle attribute
Status: RESOLVED FIXED
[good first bug]
: dev-doc-complete
Product: Core
Classification: Components
Component: MathML (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla8
Assigned To: Jonathan Hage
:
Mentors:
Depends on:
Blocks: 569124
  Show dependency tree / graph
 
Reported: 2011-07-06 12:38 PDT by Frédéric Wang (:fredw)
Modified: 2011-07-23 07:45 PDT (History)
6 users (show)
dao+bmo: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Attachments
Patch (990 bytes, patch)
2011-07-08 05:59 PDT, Jonathan Hage 		roc: review+
Details | Diff | Splinter Review
Reftests (1.51 KB, patch)
2011-07-08 06:00 PDT, Jonathan Hage 		no flags Details | Diff | Splinter Review
Reftests (1.51 KB, patch)
2011-07-08 06:43 PDT, Jonathan Hage 		roc: review+
Details | Diff | Splinter Review
Patch (2.38 KB, patch)
2011-07-11 00:30 PDT, Jonathan Hage 		no flags Details | Diff | Splinter Review
Code (2.38 KB, patch)
2011-07-11 00:34 PDT, Jonathan Hage 		no flags Details | Diff | Splinter Review
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Description Frédéric Wang (:fredw) 2011-07-06 12:38:46 PDT 
In bug 569124, we allowed the <math/> element to behave as a <mstyle/>. The displaystyle attribute, handled in the nsMathMLmstyleFrame, was however not taken into account in this change.

A possible fix: in nsMathMLFrame::GetPresentationDataFrom, after setting aPresentationData.flags according to display->mDisplay, we can add a call to nsMathMLFrame::FindAttrDisplaystyle, so that we will look to a possible displaystyle attribute on the <math/> element.

See the testcase, which can easily be converted into a == reftest.
Comment 1 Frédéric Wang (:fredw) 2011-07-08 00:13:39 PDT 
I forgot to attach a testcase. The idea was simply to compare

<math displaystyle="true">
  ...
</math>

and

<math>
  <mstyle displaystyle="true">
    ...
  </mstyle>
</math>

where ... are elements rendered differently in displaystyle = true/false
Comment 2 Jonathan Hage 2011-07-08 05:59:56 PDT 
Created attachment 544790 [details] [diff] [review]
Patch
Comment 3 Jonathan Hage 2011-07-08 06:00:26 PDT 
Created attachment 544791 [details] [diff] [review]
Reftests
Comment 4 Jonathan Hage 2011-07-08 06:43:09 PDT 
Created attachment 544797 [details] [diff] [review]
Reftests
Comment 5 Robert O'Callahan (:roc) (email my personal email if necessary) 2011-07-08 15:37:17 PDT 
Comment on attachment 544790 [details] [diff] [review]
Patch

Review of attachment 544790 [details] [diff] [review]:
-----------------------------------------------------------------
Comment 6 Robert O'Callahan (:roc) (email my personal email if necessary) 2011-07-08 15:37:28 PDT 
Comment on attachment 544797 [details] [diff] [review]
Reftests

Review of attachment 544797 [details] [diff] [review]:
-----------------------------------------------------------------
Comment 7 Dão Gottwald [:dao] 2011-07-09 06:56:36 PDT 
http://hg.mozilla.org/mozilla-central/rev/5d92749612cc
Comment 8 Dão Gottwald [:dao] 2011-07-09 07:51:20 PDT 
backed out for crashtest failures: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1310221738.1310222230.5384.gz&fulltext=1
Comment 9 Frédéric Wang (:fredw) 2011-07-09 09:46:47 PDT 
We need to update nsMathMLFrame::FindAttrDisplaystyle, now that <math/> is allowed to have the displaystyle attribute:

http://mxr.mozilla.org/mozilla-central/source/layout/mathml/nsMathMLFrame.cpp#65
Comment 10 Jonathan Hage 2011-07-11 00:30:12 PDT 
Created attachment 545130 [details] [diff] [review]
Patch
Comment 11 Jonathan Hage 2011-07-11 00:34:37 PDT 
Created attachment 545131 [details] [diff] [review]
Code
Comment 12 Frédéric Wang (:fredw) 2011-07-11 04:19:52 PDT 
(In reply to comment #11)
> Created attachment 545131 [details] [diff] [review] [review]
> Code

I ran the crashtests with a debug build. The only failing test I have is

parser/htmlparser/tests/crashtests/445171-1.html

but that does not seem related to this patch.
Comment 13 Boris Zbarsky [:bz] (TPAC) 2011-07-12 09:08:45 PDT 
http://hg.mozilla.org/integration/mozilla-inbound/rev/91740322b888
Note You need to log in before you can comment on or make changes to this bug.