[webkit-reviews] review granted: [Bug 28673] Modifying <text rotate=""> doesn't clear the corresponding SVGAnimatedNumberList : [Attachment 38462] Fix and test

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun Aug 23 22:20:52 PDT 2009


Darin Adler <darin at apple.com> has granted Cameron McCormack <cam at mcc.id.au>'s
request for review:
Bug 28673: Modifying <text rotate=""> doesn't clear the corresponding
SVGAnimatedNumberList
https://bugs.webkit.org/show_bug.cgi?id=28673

Attachment 38462: Fix and test
https://bugs.webkit.org/attachment.cgi?id=38462&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +	   * svg/dom/resources/text-rotate-live.js: Added.
> +	   (getRotate):
> +	   (getAndSetRotate):

If you're not commenting on these functions individually and the file is new,
then there's no need to leave this list of functions in the change log. The
idea of prepare-ChangeLog is that it helps you write the ChangeLog entry, not
that you should use that it generates as-is.

The test code doesn't follow our usual brace style. Open braces for functions
go on their own lines. Single line for loop bodies don't get braces around
them. I would have put the "var" for the "i" variable inside the for loop. I
believe the function results could be arrays rather than strings -- the
shouldBe macro knows how to check arrays for equality. We don't use "get" in
the names of functions that simply return results.

None of these are significant issues, r=me on this patch as it is.


More information about the webkit-reviews mailing list