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

[bugzilla-daemon at webkit.org]

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





--- Comment #20 from Beth Dakin <bdakin at apple.com>  2011-06-03 14:34:08 PST ---
(In reply to comment #19)
> (In reply to comment #17)
> > (From update of attachment 92668 [details] [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.
> 

I really think it's better to have one bug-fix per patch. And I consider these to be two bugs.

> > 
> > > 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.
> 

Yeah, the style bot should be fixed here. Check out item 3 in the "Braces" section: http://www.webkit.org/coding/coding-style.html

> > 
> > > 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.

That makes sense.

I really think you should file a new bug for the layout problem and post your layout patch there. Ping me when you do, and I will be happy to review.

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