[webkit-reviews] review denied: [Bug 56906] CSS related SVG*Element changes doesn't require relayout : [Attachment 86589] Proposed patch

[bugzilla-daemon at webkit.org]

Dirk Schulze <krit at webkit.org> has denied Renata Hodovan <reni at webkit.org>'s
request for review:
Bug 56906: CSS related SVG*Element changes doesn't require relayout
https://bugs.webkit.org/show_bug.cgi?id=56906

Attachment 86589: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=86589&action=review

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

I think you miss the case, where a CSS property gets changed on a filter
element

<filter id="f1" style="flood-color:red">
<feFlood flood-color="inherit"/>
</filter>

I think we should invalidate in this case, since we don't know which effect is
affected by this change and therefor we can't update the FE*.cpp. Just remove
renderer->isSVGResourceFilter() in SVGResourceCache. I'd still	like to see a
test case with this scenario.

> Source/WebCore/ChangeLog:10
> +	   we need here an early return. So
RenderSVGResourceFilterPrimitive::styleDidChange() can handle these properties

s/here//

> Source/WebCore/rendering/svg/RenderSVGResourceFilterPrimitive.cpp:42
> +	   return;

What should happen for this case? At the moment we would neither repaint, nor
relayout, right?

> Source/WebCore/rendering/svg/SVGResourcesCache.cpp:129
> +    // In this case the proper SVGFE*Element will deceide whether the
modifided CSS properties implies a relayout or repaint.

s/deceide/decide/