Summary: | parse CSS Animations | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dean Jackson <dino> | ||||||||
Component: | CSS | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | simon.fraser | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Mac | ||||||||||
OS: | All | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 19028 | ||||||||||
Attachments: |
|
Description
Dean Jackson
2008-07-17 16:24:20 PDT
Created attachment 22354 [details]
patch to provide parsing for css animation properties
Created attachment 22395 [details]
updated patch for css parsing
Removed an old keyframes @rule from the grammar. I'd also left out the keyword in the tokenizer so keyframes were not being noticed.
Comment on attachment 22395 [details]
updated patch for css parsing
CSSComputedStyleDeclaration should not be part of this patch.
Need a followup bug that keyframes should support a comma-separated list of percentages and not just a single percentage.
The way keyframe rules are added is wrong. You need to add keyframe styles as part of the normal addRulesFromSheet process. You can't add them in a separate walk or you'll miss @import and @media behavior.
Make a test with a <style> element that @imports a sheet with keyframes and you'll see that it doesn't work. Similarly try putting a @keyframes rule inside @media print { } and watch it incorrectly apply to screen.
You already have the hashtable from name -> keyframes rule held by the CSSStyleSelector. Just build it up during the normal addRulesFromSheet process instead.
The code that aggressively constructs RenderStyles at keyframe-buildup time in the style selector is wrong, but that's the part I'm looking the other way on.
This patch duplicates stuff from 19938.... the Node/Document/Element changes. Pick one patch to put them in. :)
Created attachment 22646 [details]
Updated patch that adds keyframes correctly
This (hopefully) addresses the previous review comments about adding Keyframes at the wrong time during parsing. They should now be picked up within @media rules, for example.
Comment on attachment 22646 [details]
Updated patch that adds keyframes correctly
This looks good.
I think we need tests of error recovery from messed up keyframes, since I'm a bit worried that malformed keyframes rules aren't going to error recover properly, but I think that could be a followup bug.
r35579 = 684ed79bb9f8a782bb5f4d789adef25c161ec0e7 (trunk) M WebCore/GNUmakefile.am M WebCore/WebCore.xcodeproj/project.pbxproj M WebCore/WebCore.pro M WebCore/ChangeLog M WebCore/WebCore.vcproj/WebCore.vcproj M WebCore/css/CSSValueKeywords.in M WebCore/css/CSSStyleSelector.cpp M WebCore/css/CSSStyleSelector.h M WebCore/css/CSSRule.h A WebCore/css/WebKitCSSKeyframesRule.cpp M WebCore/css/CSSParser.cpp M WebCore/css/CSSGrammar.y A WebCore/css/WebKitCSSKeyframeRule.cpp M WebCore/css/tokenizer.flex M WebCore/css/StyleBase.h M WebCore/css/CSSPropertyNames.in A WebCore/css/WebKitCSSKeyframeRule.h A WebCore/css/WebKitCSSKeyframesRule.h M WebCore/css/CSSParser.h M WebCore/WebCoreSources.bkl |