[webkit-reviews] review denied: [Bug 123716] SVG rectangle with filter will not render : [Attachment 224089] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Adenilson Cavalcanti
Silva <savagobr at yahoo.com>'s request for review:
Bug 123716: SVG rectangle with filter will not render
https://bugs.webkit.org/show_bug.cgi?id=123716

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

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=224089&action=review


> Source/WebCore/ChangeLog:8
> +	   A filtered SVG element with width or length bigger than 5.000

You should write this as 5000 or 5,000. 5.000 sounds like decimal 5.

> Source/WebCore/ChangeLog:10
> +	(counting the margin/border) will fail to render. This patch will
> +	instead test against the total element area.

Weird indentation

> Source/WebCore/platform/graphics/filters/FilterEffect.cpp:42
> +static const float kMaxFilterLength = 8192;

Length is ambiguous here. Length of what? Dimension would be better.

> Source/WebCore/platform/graphics/filters/FilterEffect.cpp:84
> +

Extra blank line.

> Source/WebCore/platform/graphics/filters/FilterEffect.h:62
> +    static bool isFilterSizeValid(const IntRect&);
> +    static bool isFilterSizeValid(const FloatRect&);

Are both really necessary?