[webkit-reviews] review granted: [Bug 85929] regression(111639): Issue with simultaneous CSS animations : [Attachment 141827] Patch V1.
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon May 14 18:28:45 PDT 2012
Dean Jackson <dino at apple.com> has granted Igor Trindade Oliveira
<igor.oliveira at webkit.org>'s request for review:
Bug 85929: regression(111639): Issue with simultaneous CSS animations
https://bugs.webkit.org/show_bug.cgi?id=85929
Attachment 141827: Patch V1.
https://bugs.webkit.org/attachment.cgi?id=141827&action=review
------- Additional Comments from Dean Jackson <dino at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=141827&action=review
> LayoutTests/animations/fill-mode-forwards.html:12
> + function addPopAnimation () {
Space between function name and ()
> LayoutTests/animations/fill-mode-forwards.html:15
> + popAnimationCounter = popAnimationCounter + 1;
Just popAnimationCounter++
> LayoutTests/animations/fill-mode-forwards.html:18
> + function removePopAnimation () {
Ditto on the space.
> LayoutTests/animations/fill-mode-forwards.html:93
> + <div id="pulsing-square"
onwebkitanimationiteration="setTimeout(addPopAnimation(), 50);"
> + onwebkitanimationend="pulsingSquareEnded()"
> + ></div>
I think you can move the ></div> to the previous line.
> Source/WebCore/ChangeLog:10
> + Currently, previousTimeToNextService is just saving the previous
CompositeAnimation::timeToNextService
> + for AnimationControllerPrivate::updateAnimationTimerForRenderer,
however CompositeAnimation::timeToNextService
> + is also called and used by
AnimationControllerPrivate::updateAnimationTimer and we
> + are not saving the previousTimeToNextService for
AnimationControllerPrivate::updateAnimationTimer
> + doing AnimationController have unexpected behavior.
This last line is not correct English. I think you could probably just change
the bit after "used by AnimationControllerPrivate::updateAnimationTimer" to be
"updateAnimationTimer. Make sure we save the existing timeToNextService from
both places, updateAnimationTimerForRenderer and updateAnimationTimer." I don't
think you need the class names (e.g. AnimationControllerPrivate::) here.
More information about the webkit-reviews
mailing list