[webkit-reviews] review granted: [Bug 20057] Animate viewBox attribute in SVG : [Attachment 98148] Patch

[bugzilla-daemon at webkit.org]

Nikolas Zimmermann <zimmermann at kde.org> has granted Dirk Schulze
<krit at webkit.org>'s request for review:
Bug 20057: Animate viewBox attribute in SVG
https://bugs.webkit.org/show_bug.cgi?id=20057

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

------- Additional Comments from Nikolas Zimmermann <zimmermann at kde.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=98148&action=review

Great patch, r=me.

> Source/WebCore/svg/SVGAnimatedRect.cpp:36
> +    OwnPtr<SVGAnimatedType> animatedType = SVGAnimatedType::createRect(new
FloatRect());

you can omit the braces after the new FloatRect.

> Source/WebCore/svg/SVGAnimatedRect.cpp:75
> +	   newRect = percentage < 0.5f ? fromRect : toRect;

Again the .f, we don't need that anymore - the style guide says we should omit
this.

> Source/WebCore/svg/SVGAnimatedRect.cpp:99
> +    return -1;

Does this affect a specific animation mode which won't work with rects?

> Source/WebCore/svg/SVGAnimatedType.cpp:129
> +	   return String::number(m_data.rect->x()) + ' ' +
String::number(m_data.rect->y()) + ' '
> +		+ String::number(m_data.rect->width()) + ' ' +
String::number(m_data.rect->height());

Great, that's exactly how we can construct strings like this efficiently with
the new StringAppend operator+ trickery :-)