[Webkit-unassigned] [Bug 34328] Manipulating SVG element attributes in Javascript does not work as expected

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon May 31 00:35:41 PDT 2010


https://bugs.webkit.org/show_bug.cgi?id=34328





--- Comment #11 from Rob Buis <rwlbuis at gmail.com>  2010-05-31 00:35:41 PST ---
Hello Dirk,

Thanks for the review!

(In reply to comment #10)
> (From update of attachment 57423 [details])
> 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.

Ok, that looks reasonable. I wonder if the normal webkit style check script caught some of that, but I did not pay attention to that anyway when I was
running the upload script. Looks like I can fix above tonight and then the patch should be landeable.
Cheers,

Rob.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list