[webkit-reviews] review denied: [Bug 61556] REGRESSION(87152): Crash on page with svg fonts : [Attachment 96692] Style-updated patch which actually applies (sorry!)

[bugzilla-daemon at webkit.org]

Dirk Schulze <krit at webkit.org> has denied Tim Horton
<timothy_horton at apple.com>'s request for review:
Bug 61556: REGRESSION(87152): Crash on page with svg fonts
https://bugs.webkit.org/show_bug.cgi?id=61556

Attachment 96692: Style-updated patch which actually applies (sorry!)
https://bugs.webkit.org/attachment.cgi?id=96692&action=review

------- Additional Comments from Dirk Schulze <krit at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=96692&action=review

This definitely needs a crash and regression test (both can be one test in this
case).

> Source/WebCore/ChangeLog:9
> +	   We can't assume that the parent of a SVG-font-styled
> +	   text node won't be an anonymous block.
> +	   http://bugs.webkit.org/show_bug.cgi?id=61556
> +
> +	   No new tests. (OOPS!)

The style is wrong:

bugtitle
bugURL

text

> Source/WebCore/rendering/svg/SVGTextRunRenderingContext.cpp:-80
> -    ASSERT(newRenderer->node());
> -    ASSERT(newRenderer->node()->isElementNode());

Why did you remove these asserts?

> Source/WebCore/rendering/svg/SVGTextRunRenderingContext.cpp:270
> +	   String language;

What happens if language is empty? Seems that we assumed that we have lang set.