Bug 36525 - MathML Fraction and Subsup incorrect layout
: MathML Fraction and Subsup incorrect layout
Status: RESOLVED FIXED
: WebKit
Layout and Rendering
: 528+ (Nightly build)
: PC Mac OS X 10.6
: P2 Normal
Assigned To:
:
:
:
: 3251
  Show dependency treegraph
 
Reported: 2010-03-24 05:36 PST by
Modified: 2010-06-29 15:58 PST (History)


Attachments
An example of MathML of <mfrac> and <msubsup> (588 bytes, application/xhtml+xml)
2010-03-24 05:36 PST, Stephen Loo
no flags Details
Correct layout of the xhtml. (8.87 KB, image/png)
2010-03-24 05:37 PST, 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 PST, François Sausset
no flags Details
Patch fixing the bug (54.48 KB, patch)
2010-06-29 14:47 PST, François Sausset
no flags Review Patch | Details | Formatted Diff | Diff
Updated patch (54.53 KB, patch)
2010-06-29 15:12 PST, François Sausset
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2010-03-24 05:36:42 PST
Created an attachment (id=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 From 2010-03-24 05:37:58 PST -------
Created an attachment (id=51490) [details]
Correct layout of the xhtml.
------- Comment #2 From 2010-04-03 11:07:32 PST -------
The problem does not come from the mfrac element, but from the mrow in which the msubsup is embedded. See the attachment.
------- Comment #3 From 2010-04-03 11:10:28 PST -------
Created an attachment (id=52497) [details]
Test showing the bug when msubsup is embedded in an mrow element.
------- Comment #4 From 2010-06-22 13:47:55 PST -------
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 From 2010-06-29 14:47:04 PST -------
Created an attachment (id=60058) [details]
Patch fixing the bug
------- Comment #6 From 2010-06-29 14:50:58 PST -------
(From update of attachment 60058 [details])
> +    if (base->firstChild()->isRenderMathMLBlock()) {

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

Otherwise, this patch looks good to me.
------- Comment #7 From 2010-06-29 15:12:08 PST -------
Created an attachment (id=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 From 2010-06-29 15:28:41 PST -------
(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 From 2010-06-29 15:36:35 PST -------
(From update of attachment 60064 [details])
Revert to previous patch to avoid unnecessary test.
------- Comment #10 From 2010-06-29 15:57:59 PST -------
(From update of attachment 60058 [details])
Clearing flags on attachment: 60058

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