Bug 69531 – animation-iteration-count does not handle floating point values correctly.

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Chris Marrin]

The CSS Animation spec defines this value as a Number and non-integers should end the animation partway through a cycle. This is not currently implemented in WebKit.

[Igor Trindade Oliveira]

Created attachment 130948 [details]
Patch

Proposed patch.

[Dean Jackson]

I'm out Monday. I can look at this Tue.

[Alexis Menard (darktears)]

Comment on attachment 130948 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=130948&action=review

Could you please add in the changelog the link to the spec?

http://dev.w3.org/csswg/css3-animations/#the-animation-iteration-count-property-

> LayoutTests/animations/fill-mode-iteration-count-non-integer.html:134
> +</html>

I think this test should be converted to use the animation JS helpers if possible : http://trac.webkit.org/browser/branches/chromium/1025/LayoutTests/animations/animation-border-overflow.html

[Igor Trindade Oliveira]

(In reply to comment #3)
> (From update of attachment 130948 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=130948&action=review
> 
> Could you please add in the changelog the link to the spec?
> 
> http://dev.w3.org/csswg/css3-animations/#the-animation-iteration-count-property-
> 

Ok

> > LayoutTests/animations/fill-mode-iteration-count-non-integer.html:134
> > +</html>
> 
> I think this test should be converted to use the animation JS helpers if possible : http://trac.webkit.org/browser/branches/chromium/1025/LayoutTests/animations/animation-border-overflow.html

Yeah, but the animation JS helpers just works when the animation are running.  When the animation ends i can't get the elapsed time or any other animation property. In fact, initially i tried to use animation JS helpers.

[Dean Jackson]

Comment on attachment 130948 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=130948&action=review

I think Alexis asked for a link to the spec in the ChangeLog. Sorry that you inherited the bad {} style from existing tests (probably my fault).

> LayoutTests/animations/fill-mode-iteration-count-non-integer.html:86
> +            if (Math.abs(expectedValue - realValue) < allowance) {
> +              result += "PASS";
> +            } else {
> +              result += "FAIL";
> +            }

Single lines - no {}

> LayoutTests/animations/fill-mode-iteration-count-non-integer.html:104
> +            if (Math.abs(expectedValue - realValue) < allowance) {
> +              result += "PASS";
> +            } else {
> +              result += "FAIL";
> +            }

Single lines - no {}

[WebKit Review Bot]

Comment on attachment 130948 [details]
Patch

Clearing flags on attachment: 130948

Committed r110588: <http://trac.webkit.org/changeset/110588>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Dean Jackson]

Oops! I set cq+ when reviewing, even though there were a few small changes to be made :(

They are not showstoppers, but maybe we should clean them up.

[Igor Trindade Oliveira]

Ok, doing that.
(In reply to comment #8)
> Oops! I set cq+ when reviewing, even though there were a few small changes to be made :(
> 
> They are not showstoppers, but maybe we should clean them up.