Bug 20057

Created attachment 22310 [details] SVG file showing requested behavior Open attachment and click on the red circle. A zooming effect should be created because the viewBox is animated via script. Repeated click result in continual zooming. Works in IE/ASV and Opera(has a hiccup).

Created attachment 22312 [details] correction to previous attachment

Upload a patch to support viewBox animation soon.

Created attachment 98148 [details] Patch

Comment on attachment 98148 [details] Patch 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 :-)

(In reply to comment #5) > (From update of attachment 98148 [details]) > 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. Really? I asked for it on your other reviews, but you never answered to the question. I was not aware of this change. Just thought this is the case for round numbers like 0 or 1. Isn't it a double if you omit the .f? > > > Source/WebCore/svg/SVGAnimatedRect.cpp:99 > > + return -1; > > Does this affect a specific animation mode which won't work with rects? Yes, paced animations. Just supported by Opera so far. The specification is vague about that. > > > 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 :-) Hehe. I know. You told me to use it that way before ;) Thanks for the review.

Committed r89431: <http://trac.webkit.org/changeset/89431>