Bug 71293 - Change values for WEBKIT_KEYFRAMES_RULE, WEBKIT_KEYFRAME_RULE
: Change values for WEBKIT_KEYFRAMES_RULE, WEBKIT_KEYFRAME_RULE
Status: RESOLVED FIXED
: WebKit
CSS
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2011-11-01 09:47 PST by
Modified: 2012-02-10 19:10 PST (History)


Attachments
Patch (2.15 KB, patch)
2012-02-10 10:02 PST, Edward O'Connor
no flags Review Patch | Details | Formatted Diff | Diff
Patch (3.38 KB, patch)
2012-02-10 10:22 PST, Edward O'Connor
no flags Review Patch | Details | Formatted Diff | Diff
Patch (4.06 KB, patch)
2012-02-10 10:43 PST, Edward O'Connor
no flags Review Patch | Details | Formatted Diff | Diff
Patch (4.14 KB, patch)
2012-02-10 12:35 PST, Edward O'Connor
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


------- Comment #1 From 2012-02-08 07:34:46 PST -------
We should do this to help move animations forward.
------- Comment #2 From 2012-02-10 10:02:55 PST -------
Created an attachment (id=126529) [details]
Patch

Patch for review
------- Comment #3 From 2012-02-10 10:05:30 PST -------
(From update of attachment 126529 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=126529&action=review

Please add a test to verify the correct values

> Source/WebCore/ChangeLog:8
> +        No tests.

We really should have a test. The fact that we didn't before means the error in the values went undetected.
------- Comment #4 From 2012-02-10 10:22:25 PST -------
Created an attachment (id=126532) [details]
Patch

Added test per cmarrin's review
------- Comment #5 From 2012-02-10 10:24:57 PST -------
(From update of attachment 126532 [details])
Where is the expected file change?
------- Comment #6 From 2012-02-10 10:34:05 PST -------
i think we do not need changes in expected file, because with the change in the test case, it will just show things in the screen if the test fails.

1(In reply to comment #5)
> (From update of attachment 126532 [details] [details])
> Where is the expected file change?
------- Comment #7 From 2012-02-10 10:40:26 PST -------
(In reply to comment #6)
> i think we do not need changes in expected file, because with the change in the test case, it will just show things in the screen if the test fails.
> 
> 1(In reply to comment #5)
> > (From update of attachment 126532 [details] [details] [details])
> > Where is the expected file change?

We need as shouldBe will display :

PASS window.CSSRule.WEBKIT_KEYFRAMES_RULE is 7

...

when the condition is verified.

The existing -expected file is full of PASS ... is .... :D
------- Comment #8 From 2012-02-10 10:43:35 PST -------
Created an attachment (id=126535) [details]
Patch

Updated -expected per Alexis' comment.
------- Comment #9 From 2012-02-10 11:02:58 PST -------
(In reply to comment #8)
> Created an attachment (id=126535) [details] [details]
> Patch
> 
> Updated -expected per Alexis' comment.

Cool thanks.
------- Comment #10 From 2012-02-10 11:42:57 PST -------
(From update of attachment 126535 [details])
Attachment 126535 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11473735

New failing tests:
animations/animation-css-rule-types.html
animations/keyframes-rule.html
------- Comment #11 From 2012-02-10 12:35:50 PST -------
Created an attachment (id=126557) [details]
Patch

Fixed failing test and removed redundant changes from the other test.
------- Comment #12 From 2012-02-10 19:10:47 PST -------
(From update of attachment 126557 [details])
Clearing flags on attachment: 126557

Committed r107470: <http://trac.webkit.org/changeset/107470>
------- Comment #13 From 2012-02-10 19:10:55 PST -------
All reviewed patches have been landed.  Closing bug.