[webkit-reviews] review denied: [Bug 69747] FEComponentTransfer element doesn't support dynamic invalidation : [Attachment 110532] Proposed patch

[bugzilla-daemon at webkit.org]

Nikolas Zimmermann <zimmermann at kde.org> has denied Renata Hodovan
<reni at webkit.org>'s request for review:
Bug 69747: FEComponentTransfer element doesn't support dynamic invalidation
https://bugs.webkit.org/show_bug.cgi?id=69747

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

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


Tests look great, I checked each of them. I wish the changes would be more
easily visible (did the filter apply really work?) I have to trust you all
tests work manually fine and produce this result which is different to no
filter applied :-)

r- because of abusing SVGElement to share code. Please rework this part,
everything else is great.

> Source/WebCore/svg/SVGElement.cpp:141
> +	   return;

Unnecessary.

> Source/WebCore/svg/SVGElement.h:120
> +    void invalidateParents();

This does not belong in SVGElement - it has a very generic name and leads to
confusion. SVGElement should be kept as small as possible.
Why don't you introduce a free-function in eg.
SVGFilterPrimitiveStandardAttributes.h or another filter-related class?