[webkit-reviews] review granted: [Bug 102080] Support animation of basic shape 'polygon' : [Attachment 174330] Patch

[bugzilla-daemon at webkit.org]

Daniel Bates <dbates at webkit.org> has granted Dirk Schulze <krit at webkit.org>'s
request for review:
Bug 102080: Support animation of basic shape 'polygon'
https://bugs.webkit.org/show_bug.cgi?id=102080

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

------- Additional Comments from Daniel Bates <dbates at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=174330&action=review


I noticed some minor nits.

> Source/WebCore/ChangeLog:13
> +	   Added polygon animation test to existing test.

Although mentioned in LayoutTests/ChangeLog. You may want to consider listing
the tests in this ChangeLog entry:
fast/exclusions/shape-inside/shape-inside-animation.html and
css3/masking/clip-path-animation.html.

> Source/WebCore/ChangeLog:15
> +	   * page/animation/CSSPropertyAnimation.cpp: Forgot to add copy right
with last commits.

Nit: copy right => copyright

> LayoutTests/css3/masking/clip-path-animation-expected.txt:1
> -  
> +   

We should consider hiding the <div>s when the test is run in DRT so as to avoid
changes in whitespace from appearing in the expected results.

> LayoutTests/css3/masking/clip-path-animation.html:59
> +	 ["polygon-anim",  1, "polygon-box", "webkitClipPath",
'polygon(nonzero, 10% 10%, 90% 10%, 90% 90%, 10% 90%)', 0.05],

Nit: I know that you're following the format of the other array entries above.
It seems weird that we use single quotes around string literals for the fifth
element in these arrays given we use double quotes around string literals
elsewhere.