Bug 71293

Summary: Change values for WEBKIT_KEYFRAMES_RULE, WEBKIT_KEYFRAME_RULE
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: CSSAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, cmarrin, dglazkov, dino, eoconnor, igor.oliveira, macpherson, menard, ojan, simon.fraser, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Attachments
Patch (2.15 KB, patch)
2012-02-10 10:02 PST, Theresa O'Connor 		no flags
DetailsFormatted DiffDiff
Patch (3.38 KB, patch)
2012-02-10 10:22 PST, Theresa O'Connor 		no flags
DetailsFormatted DiffDiff
Patch (4.06 KB, patch)
2012-02-10 10:43 PST, Theresa O'Connor 		no flags
DetailsFormatted DiffDiff
Patch (4.14 KB, patch)
2012-02-10 12:35 PST, Theresa O'Connor 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Simon Fraser (smfr)
Comment 1 2012-02-08 07:34:46 PST
We should do this to help move animations forward.
Theresa O'Connor
Comment 2 2012-02-10 10:02:55 PST
Created attachment 126529 [details] Patch Patch for review
Chris Marrin
Comment 3 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.
Theresa O'Connor
Comment 4 2012-02-10 10:22:25 PST
Created attachment 126532 [details] Patch Added test per cmarrin's review
Alexis Menard (darktears)
Comment 5 2012-02-10 10:24:57 PST
Comment on attachment 126532 [details] Patch Where is the expected file change?
Igor Trindade Oliveira
Comment 6 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?
Alexis Menard (darktears)
Comment 7 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
Theresa O'Connor
Comment 8 2012-02-10 10:43:35 PST
Created attachment 126535 [details] Patch Updated -expected per Alexis' comment.
Alexis Menard (darktears)
Comment 9 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.
WebKit Review Bot
Comment 10 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
Theresa O'Connor
Comment 11 2012-02-10 12:35:50 PST
Created attachment 126557 [details] Patch Fixed failing test and removed redundant changes from the other test.
WebKit Review Bot
Comment 12 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>
WebKit Review Bot
Comment 13 2012-02-10 19:10:55 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.