[Webkit-unassigned] [Bug 57897] Crash in WebCore::RenderMathMLSubSup::baselinePosition()

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=57897





--- Comment #19 from Alex Milowski <alex at milowski.com>  2011-06-03 07:37:39 PST ---
(In reply to comment #17)
> (From update of attachment 92668 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=92668&action=review
> 
> I think the bug fix here is compelling, but I think we should just fix the crash with this bug. Since Jeffrey posted a patch just to fix the crash, I think I will r+ that, and we should file a new bug for the rendering issue you are working on here.

That will end up with the wrong rendering after the script runs.  The additional tests show that the correct result will happen for msup/msub/msubsup when a child is removed and added again.

I would really prefer this is fixed properly and not just patched to remove the crash.

> 
> > Source/WebCore/rendering/mathml/RenderMathMLSubSup.cpp:69
> > +            position++;
> 
> Our style-guide dictates that there should be braces around this for-loop since it contains more than one line of code. Also, is it necessary to compare the nodeType() to Node::ELEMENT_NODE? Can you just ask current->isElementNode()?

Yet, it passed all the style checks.  I can fix that.

> 
> > Source/WebCore/rendering/mathml/RenderMathMLSubSup.cpp:73
> > +        RenderMathMLBlock* wrapper = new (renderArena()) RenderMathMLBlock(node());
> 
> How do you know this is always position 1? Is there a way to break this?

Because position 1 is the first element child and, for these MathML elements, the first element child is always the base of the msup/msub/msubsup.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.