Bug 36525

Summary: MathML Fraction and Subsup incorrect layout
Product: WebKit Reporter: Stephen Loo <shikil>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: alex, commit-queue, darin, fred.wang, sausset
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.6   
Bug Depends on:    
Bug Blocks: 3251    
Attachments:
Description Flags
An example of MathML of <mfrac> and <msubsup>
none
Correct layout of the xhtml.
none
Test showing the bug when msubsup is embedded in an mrow element.
none
Patch fixing the bug
none
Updated patch none

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.