Bug 36525 - MathML Fraction and Subsup incorrect layout
Summary: MathML Fraction and Subsup incorrect layout
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.6
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 3251
  Show dependency tree / graph
 
Reported: 2010-03-24 05:36 PDT by Stephen Loo
Modified: 2010-06-29 15:58 PDT (History)
5 users (show)

See Also:

Attachments
An example of MathML of <mfrac> and <msubsup> (588 bytes, application/xhtml+xml)
2010-03-24 05:36 PDT, Stephen Loo 		no flags Details
Correct layout of the xhtml. (8.87 KB, image/png)
2010-03-24 05:37 PDT, Stephen Loo 		no flags Details
Test showing the bug when msubsup is embedded in an mrow element. (297 bytes, application/xhtml+xml)
2010-04-03 11:10 PDT, François Sausset 		no flags Details
Patch fixing the bug (54.48 KB, patch)
2010-06-29 14:47 PDT, François Sausset 		no flags Details | Formatted Diff | Diff
Updated patch (54.53 KB, patch)
2010-06-29 15:12 PDT, François Sausset 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Stephen Loo 2010-03-24 05:36:42 PDT 
Created attachment 51489 [details]
An example of MathML of <mfrac> and <msubsup>

In my attachment example, I combine <mfrac> and <msubsup> to use, but the layout is incorrect. Please see attached xhtml for references. And the correct layout should be same as attached png.
Comment 1 Stephen Loo 2010-03-24 05:37:58 PDT 
Created attachment 51490 [details]
Correct layout of the xhtml.
Comment 2 François Sausset 2010-04-03 11:07:32 PDT 
The problem does not come from the mfrac element, but from the mrow in which the msubsup is embedded. See the attachment.
Comment 3 François Sausset 2010-04-03 11:10:28 PDT 
Created attachment 52497 [details]
Test showing the bug when msubsup is embedded in an mrow element.
Comment 4 Alex Milowski 2010-06-22 13:47:55 PDT 
Actually, math and mrow are almost the exact same rendering object (RenderMathMLRow).  The cause of this is a row containing another row and stretching.  The row operator stretching algorithm is doing something strange to the layout of msubsup in this context.

The new layout patch in https://bugs.webkit.org/show_bug.cgi?id=40986 doesn't fix this yet.
Comment 5 François Sausset 2010-06-29 14:47:04 PDT 
Created attachment 60058 [details]
Patch fixing the bug
Comment 6 Darin Adler 2010-06-29 14:50:58 PDT 
Comment on attachment 60058 [details]
Patch fixing the bug

> +    if (base->firstChild()->isRenderMathMLBlock()) {

What guarantees that base->firstChild() is non-null?

Otherwise, this patch looks good to me.
Comment 7 François Sausset 2010-06-29 15:12:08 PDT 
Created attachment 60064 [details]
Updated patch

I added a test to be sure that base->firstChild() exists.

However, lines 100 and 101 of RenderMathMLSubSup.cpp should guarantee it.
Comment 8 Darin Adler 2010-06-29 15:28:41 PDT 
(In reply to comment #7)
> I added a test to be sure that base->firstChild() exists.
> 
> However, lines 100 and 101 of RenderMathMLSubSup.cpp should guarantee it.

We don't need both a test and a guarantee! Only one or the other.
Comment 9 François Sausset 2010-06-29 15:36:35 PDT 
Comment on attachment 60064 [details]
Updated patch

Revert to previous patch to avoid unnecessary test.
Comment 10 WebKit Commit Bot 2010-06-29 15:57:59 PDT 
Comment on attachment 60058 [details]
Patch fixing the bug

Clearing flags on attachment: 60058

Committed r62152: <http://trac.webkit.org/changeset/62152>
Comment 11 WebKit Commit Bot 2010-06-29 15:58:04 PDT 
All reviewed patches have been landed.  Closing bug.