[webkit-reviews] review denied: [Bug 48686] Convert SVGAnimatedNumber/SVGAnimatedNumberList to the new SVGAnimatedPropertyTearOff concept : [Attachment 72430] Patch v2

[bugzilla-daemon at webkit.org]

Rob Buis <rwlbuis at gmail.com> has denied Nikolas Zimmermann
<zimmermann at kde.org>'s request for review:
Bug 48686: Convert SVGAnimatedNumber/SVGAnimatedNumberList to the new
SVGAnimatedPropertyTearOff concept
https://bugs.webkit.org/show_bug.cgi?id=48686

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

------- Additional Comments from Rob Buis <rwlbuis at gmail.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=72430&action=review

R- mainly for the tests that are not relevant to the change.

> WebCore/ChangeLog:10
> +

Typo (wheter -> wether).
The ChangeLog does not seem to state that
SVGAnimatedNumber/SVGAnimatedNumberList are being converted, apart from the Bug
title, may need to be restated/rephrased.

> WebCore/ChangeLog:21
> +

Only SVGAnimatedNumber(List) needs to be tested, others should be done in
another patch.

> WebCore/ChangeLog:44
> +	   * svg/SVGAnimatedRect.idl: Ditto.

Are these idl changes needed (except for SVGAnimatedNumber(List).idl)?

> WebCore/svg/SVGFEConvolveMatrixElement.cpp:152
> +	int kernelMatrixSize = kernelMatrix.size();

Layout

> WebCore/svg/SVGNumberList.h:37
>  

Layout.

> LayoutTests/ChangeLog:20
> +	   * svg/dom/SVGAnimatedBoolean.html: Copied from
LayoutTests/svg/dom/SVGExternalResourcesRequired.html.

Is that a git mesage? Should it just say Added?

> LayoutTests/ChangeLog:39
> +	   * svg/dom/script-tests/SVGAnimatedBoolean.js: Copied from
LayoutTests/svg/dom/script-tests/SVGExternalResourcesRequired.js.

Ditto.