Bug 71293 - Change values for WEBKIT_KEYFRAMES_RULE, WEBKIT_KEYFRAME_RULE
Summary: Change values for WEBKIT_KEYFRAMES_RULE, WEBKIT_KEYFRAME_RULE
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-11-01 09:47 PDT by Simon Fraser (smfr)
Modified: 2012-02-10 19:10 PST (History)
11 users (show)

See Also:

Attachments
Patch (2.15 KB, patch)
2012-02-10 10:02 PST, Theresa O'Connor 		no flags Details | Formatted Diff | Diff
Patch (3.38 KB, patch)
2012-02-10 10:22 PST, Theresa O'Connor 		no flags Details | Formatted Diff | Diff
Patch (4.06 KB, patch)
2012-02-10 10:43 PST, Theresa O'Connor 		no flags Details | Formatted Diff | Diff
Patch (4.14 KB, patch)
2012-02-10 12:35 PST, Theresa O'Connor 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Simon Fraser (smfr) 2012-02-08 07:34:46 PST 
We should do this to help move animations forward.
Comment 2 Theresa O'Connor 2012-02-10 10:02:55 PST 
Created attachment 126529 [details]
Patch

Patch for review
Comment 3 Chris Marrin 2012-02-10 10:05:30 PST 
Comment on attachment 126529 [details]
Patch

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 Theresa O'Connor 2012-02-10 10:22:25 PST 
Created attachment 126532 [details]
Patch

Added test per cmarrin's review
Comment 5 Alexis Menard (darktears) 2012-02-10 10:24:57 PST 
Comment on attachment 126532 [details]
Patch

Where is the expected file change?
Comment 6 Igor Trindade Oliveira 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])
> Where is the expected file change?
Comment 7 Alexis Menard (darktears) 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])
> > 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 Theresa O'Connor 2012-02-10 10:43:35 PST 
Created attachment 126535 [details]
Patch

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

Cool thanks.
Comment 10 WebKit Review Bot 2012-02-10 11:42:57 PST 
Comment on attachment 126535 [details]
Patch

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 Theresa O'Connor 2012-02-10 12:35:50 PST 
Created attachment 126557 [details]
Patch

Fixed failing test and removed redundant changes from the other test.
Comment 12 WebKit Review Bot 2012-02-10 19:10:47 PST 
Comment on attachment 126557 [details]
Patch

Clearing flags on attachment: 126557

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