[webkit-reviews] review denied: [Bug 34328] Manipulating SVG element attributes in Javascript does not work as expected : [Attachment 57423] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun May 30 23:31:35 PDT 2010


Dirk Schulze <krit at webkit.org> has denied Rob Buis <rwlbuis at gmail.com>'s
request for review:
Bug 34328: Manipulating SVG element attributes in Javascript does not work as
expected
https://bugs.webkit.org/show_bug.cgi?id=34328

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

------- Additional Comments from Dirk Schulze <krit at webkit.org>
Patch looks great. Just some style issues:

WebCore/svg/SVGFitToViewBox.cpp:58
 +	float x = 0.0f, y = 0.0f, w = 0.0f, h = 0.0f;
WebKit style wants newlines for every declaration. Can you rename w and h to
width and height? This would match the style in other code parts.

WebCore/svg/SVGFitToViewBox.cpp:99
 +	    FloatRect r;
Can you please name the rect viewBox, like everywhere else?

WebCore/svg/SVGFitToViewBox.h:47
 +	bool parseViewBox(Document*, const String& s, FloatRect& viewBox);
The names s and viewBox are not needed here.

WebCore/svg/SVGViewSpec.cpp:53
 +	FloatRect r;
Rename r to viewBox and the current viewBox to maybe viewBoxString.

WebCore/svg/SVGViewSpec.cpp:104
 +		    FloatRect r;
dito

r- because of the style issues.


More information about the webkit-reviews mailing list