[webkit-reviews] review granted: [Bug 81513] Relative-height block SVG root not layed out on container height change : [Attachment 132593] Patch

[bugzilla-daemon at webkit.org]

Nikolas Zimmermann <zimmermann at kde.org> has granted Florin Malita
<fmalita at google.com>'s request for review:
Bug 81513: Relative-height block SVG root not layed out on container height
change
https://bugs.webkit.org/show_bug.cgi?id=81513

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

------- Additional Comments from Nikolas Zimmermann <zimmermann at kde.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=132593&action=review


r=me, with some small nitpicks:

> Source/WebCore/rendering/RenderBox.cpp:4032
> +    RenderStyle* s = style();

You don't need to introduce style() local vars, its inlined.

> Source/WebCore/rendering/RenderBox.cpp:4040
> +    RenderStyle* s = style();

Ditto.

> Source/WebCore/rendering/RenderView.cpp:120
> +	       // FIXME: can RenderView have non-RenderBox children to justify
keeping the explicit style checks below?

Can you try this, running regression tests with this fixed, and see if
something changes? Ideally before you land this, even if I set r+ :-)

> Source/WebCore/rendering/svg/RenderSVGRoot.cpp:428
> +    return
svg->intrinsicWidth(SVGSVGElement::IgnoreCSSProperties).isPercent() ||
RenderSVGRoot::hasRelativeLogicalHeight();

I'd leave this as-is to save a virtual function call.