Bug 69531 - animation-iteration-count does not handle floating point values correctly.
: animation-iteration-count does not handle floating point values correctly.
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To: Nobody
http://dev.w3.org/csswg/css3-animatio...
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-06 09:18 PDT by Chris Marrin
Modified: 2012-03-13 12:51 PDT (History)
7 users (show)

See Also:


Attachments
Patch (22.12 KB, patch)
2012-03-08 18:43 PST, Igor Trindade Oliveira
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Marrin 2011-10-06 09:18:02 PDT
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.
Comment 1 Igor Trindade Oliveira 2012-03-08 18:43:31 PST
Created attachment 130948 [details]
Patch

Proposed patch.
Comment 2 Dean Jackson 2012-03-12 05:23:39 PDT
I'm out Monday. I can look at this Tue.
Comment 3 Alexis Menard (darktears) 2012-03-12 05:40:47 PDT
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
Comment 4 Igor Trindade Oliveira 2012-03-12 07:50:03 PDT
(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.
Comment 5 Dean Jackson 2012-03-13 11:12:44 PDT
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 {}
Comment 6 WebKit Review Bot 2012-03-13 12:07:25 PDT
Comment on attachment 130948 [details]
Patch

Clearing flags on attachment: 130948

Committed r110588: <http://trac.webkit.org/changeset/110588>
Comment 7 WebKit Review Bot 2012-03-13 12:07:31 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Dean Jackson 2012-03-13 12:49:35 PDT
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.
Comment 9 Igor Trindade Oliveira 2012-03-13 12:51:16 PDT
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.