Bug 36525 - MathML Fraction and Subsup incorrect layout
: MathML Fraction and Subsup incorrect layout
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering
: 528+ (Nightly build)
: PC Mac OS X 10.6
: P2 Normal
Assigned To: Nobody
:
Depends on:
Blocks: 3251
  Show dependency treegraph
 
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

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.