[webkit-reviews] review canceled: [Bug 80773] MathML crash in WebCore::Node::previousSibling() : [Attachment 131684] Patch

[bugzilla-daemon at webkit.org]

Julien Chaffraix <jchaffraix at webkit.org> has canceled Jacky Jiang
<zkjiang008 at gmail.com>'s request for review:
Bug 80773: MathML crash in WebCore::Node::previousSibling()
https://bugs.webkit.org/show_bug.cgi?id=80773

Attachment 131684: Patch
https://bugs.webkit.org/attachment.cgi?id=131684&action=review

------- Additional Comments from Julien Chaffraix <jchaffraix at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=131684&action=review


> I agree with Jacky.

Yes, now I see that I just overlook the ASSERT. Thanks Dave & Jacky for
correcting me.

> Source/WebCore/rendering/mathml/RenderMathMLSubSup.cpp:100
> +	       if (!childElement)
> +		   return;

If this ever get reached, we will leak |child| and was what prompted my comment
about leaking. The alternative is crashing so I guess it's OK.

> LayoutTests/mathml/msub-anonymous-child-render-crash.html:7
> +	   <mi>X</mi>
> +	   <mtr>3</mtr>

Adding text unfortunately here makes the render tree dump platform specific due
to font difference. This can be alleviated but IMHO it doesn't add much as it
was caused by me being confused.

Sorry to get you chase wild rabbits here but could you just revert it to
dumpAsText?