The Webkit team has proposed a CSS extension for explicit specification of animations: http://webkit.org/specs/CSSVisualEffects/CSSAnimation.html It would be nice to implement this in Mozilla as well (using -webkit- names for compatibility, until standardized).
The draft spec has moved to w3.org, updating the bug's URL accordingly.
This is a mass change. Every comment has "assigned-to-new" in it. I didn't look through the bugs, so I'm sorry if I change a bug which shouldn't be changed. But I guess these bugs are just bugs that were once assigned and people forgot to change the Status back when unassigning.
How does this bug relate to columns support - CSS3 (MultiCol) as per https://developer.mozilla.org/en/Mozilla_CSS_support_chart?
It doesn't.
http://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/8a3a30ec0d54/css-animations-properties is a patch to parse and compute the values of the animation-* properties. (There's still a lot more work here: parsing @keyframes rules, and actually doing the animations.)
Created attachment 523699 [details] [diff] [review] patch 1: add new properties Much like transitions.
Created attachment 523700 [details] [diff] [review] patch 2: share GetCSSParsingEnvironment
Created attachment 523701 [details] [diff] [review] patch 3: handle modifications coming from grandchildren of sheet better
Created attachment 523702 [details] [diff] [review] patch 4: fix style rule inherit macros
Created attachment 523703 [details] [diff] [review] patch 5: implement and parse @keyframes rules
Created attachment 523704 [details] [diff] [review] patch 6: cascade @keyframes rules Much like @font-face rules.
Created attachment 523705 [details] [diff] [review] patch 7: move parts of transitions code into common base class to share with animation
Created attachment 523706 [details] [diff] [review] patch 8: make AddEmptyValue explicitly Infallible, and assume it is
Patch 9 (doing the animation) is still to come; it works in simple cases, but needs a lot more tests, and probably a bunch of bug fixes to match.
Comment on attachment 523699 [details] [diff] [review] patch 1: add new properties Actually, hold off on patch one. I meant to stick in another patch before patch one to deal with the steps() function, and apply the steps() and step-end/step-start to transitions as well. So hold
Created attachment 523743 [details] [diff] [review] patch 0: implement step-start, step-end, and steps()
Created attachment 523744 [details] [diff] [review] patch 1: add new properties
Created attachment 523745 [details] [diff] [review] patch 7: move parts of transitions code into common base class to share with animation
Created attachment 523808 [details] [diff] [review] patch 9: allow tests to take over control of refresh driver time and refreshes
Created attachment 523809 [details] [diff] [review] patch 10: copy some bits of test_transitions.html into a shared file
Created attachment 523829 [details] [diff] [review] patch 9: allow tests to take over control of refresh driver time and refreshes Er, since refresh drivers are shared, I need a way to restore normalcy.
Created attachment 524497 [details] [diff] [review] patch 11: implement and test animations of css animations
Created attachment 524534 [details] [diff] [review] patch 12: animation event structures This adds animation events, which are almost the same as transition events except for a few names.
Created attachment 524856 [details] [diff] [review] patch 11: implement and test animations of css animations With a slight tweak to end animations more promptly, since that makes more sense for events (although I explicitly don't test it in the animation test code).
Created attachment 524857 [details] [diff] [review] patch 12: animation event structures I missed a few pieces related to the events.
Comment on attachment 524857 [details] [diff] [review] patch 12: animation event structures >+nsresult >+NS_NewDOMAnimationEvent(nsIDOMEvent **aInstancePtrResult, >+ nsPresContext *aPresContext, >+ nsAnimationEvent *aEvent) >+{ >+ nsDOMAnimationEvent *it = new nsDOMAnimationEvent(aPresContext, aEvent); >+ if (!it) { >+ return NS_ERROR_OUT_OF_MEMORY; >+ } new is infallible, so no need for OOM check. >+ * The Original Code is nsDOMAnimationEvent. >+ * >+ * The Initial Developer of the Original Code is the Mozilla Foundation. >+ * Portions created by the Initial Developer are Copyright (C) 2009 >+ * the Initial Developer. All Rights Reserved. 2009? >+class nsDOMAnimationEvent : public nsDOMEvent, >+ public nsIDOMAnimationEvent Nit, extra space before the second 'public' >+{ >+public: >+ nsDOMAnimationEvent(nsPresContext *aPresContext, >+ nsAnimationEvent *aEvent); similar thing here
Created attachment 524895 [details] [diff] [review] patch 11: implement and test animations of css animations With some minor test updates (be a little stricter about the end boundary condition in one test -- I'd loosened it in the last revision instead of changing it as I probably should).
Created attachment 524896 [details] [diff] [review] patch 13: fix layout-dependent computed style for pseudo-elements Layout-dependent computed style was broken on pseudo-elements, as exposed by tests in the next patch. This could perhaps use its own simple test (a very simple one could work), but I didn't bother since it is tested in the next patch.
Created attachment 524897 [details] [diff] [review] patch 14: fire animation events
Comment on attachment 524897 [details] [diff] [review] patch 14: fire animation events >diff --git a/layout/style/nsAnimationManager.h b/layout/style/nsAnimationManager.h >--- a/layout/style/nsAnimationManager.h >+++ b/layout/style/nsAnimationManager.h >@@ -59,16 +62,38 @@ class nsAnimationManager : public mozill > public: > nsAnimationManager(nsPresContext *aPresContext) > : mozilla::css::CommonAnimationManager(aPresContext), > mKeyframesListIsDirty(true) > { > mKeyframesRules.Init(16); // FIXME: make infallible! > } > >+ struct AnimationEventInfo { >+ nsCOMPtr<mozilla::dom::Element> mElement; nsRefPtr, right?
Some of these patches required a bit of merging with bug 577976 and perhaps other de-COM patches. I have merged versions in my patch queue; will upload in a bit after testing.
Created attachment 525035 [details] [diff] [review] patch 2: share GetCSSParsingEnvironment
Created attachment 525036 [details] [diff] [review] patch 4: fix style rule inherit macros
Created attachment 525037 [details] [diff] [review] patch 5: implement and parse @keyframes rules
Created attachment 525038 [details] [diff] [review] patch 11: implement and test animations of css animations
Created attachment 525042 [details] [diff] [review] patch 7: move parts of transitions code into common base class to share with animation CommonAnimationManager needs a virtual destructor to avoid leaking during pretty much any unit tests.
Comment on attachment 525035 [details] [diff] [review] patch 2: share GetCSSParsingEnvironment >+ nsresult GetCSSParsingEnvironmentForRule(nsICSSRule* aRule, This can be a static method, right? r=me with that. Also, can you please file a followup bug on me to change this method to not need to addref all those options?
(In reply to comment #37) > >+ nsresult GetCSSParsingEnvironmentForRule(nsICSSRule* aRule, > > This can be a static method, right? right. > Also, can you please file a followup bug on me to change this method to not > need to addref all those options? Filed bug 649163.
Comment on attachment 523701 [details] [diff] [review] patch 3: handle modifications coming from grandchildren of sheet better >- sheet->SetModified(PR_FALSE); >+ NS_ABORT_IF_FALSE(!sheet->IsModified(), >+ "should not get marked modified during parsing"); I believe this change is wrong (and the one in css::Loader). In particular, parsing an @import rule will call css::Loader::LoadChildSheet which calls css::Loader::InsertChildSheet which calls nsCSSStyleSheet::AppendStyleSheet which calls DidDirty(). I'm somewhat surprised this abort wasn't hit in testing..... We probably need to guard the DidDirty() call or the mDirty set in DidDirty() on mInner->mComplete. Same for the quivalent CSSLoader code. >+++ b/layout/style/nsCSSStyleSheet.cpp > nsCSSStyleSheet::EnsureUniqueInner() > { >+ mDirty = PR_TRUE; This is wrong too. It will cause use to treat as non-dirty sheets that may well have been modified. Say a sheet is modified and then GetItemAt is called on its .rules. This change will cause mDirty to be set to false. Then the css::Loader will think it can coalesce other sheets with this one based on URI (see the IsModified() check in Loader::CreateSheet), which is wrong.
Er, ignore comment 39. I misread the EnsureUniqueInner change.
Comment on attachment 523701 [details] [diff] [review] patch 3: handle modifications coming from grandchildren of sheet better r=me
Comment on attachment 523743 [details] [diff] [review] patch 0: implement step-start, step-end, and steps() r=me
Created attachment 525288 [details] [diff] [review] patch 15: #ifdef the feature
Created attachment 525293 [details] [diff] [review] patch 5: implement and parse @keyframes rules I noticed the StyleRule.h changes were a remnant of an earlier approach, so removed them.
Comment on attachment 523744 [details] [diff] [review] patch 1: add new properties >+++ b/layout/style/nsCSSParser.cpp >@@ -4474,18 +4488,22 @@ CSSParserImpl::ParseVariant(nsCSSValue& We should probably assert up front that the variant mask doesn't have both VARIANT_IDENTIFIER and VARIANT_IDENTIFIER_NO_INHERIT set. >+ if (((aVariantMask & >+ (VARIANT_IDENTIFIER | VARIANT_IDENTIFIER_NO_INHERIT)) != 0) && >+ (eCSSToken_Ident == tk->mType) && >+ ((aVariantMask & VARIANT_IDENTIFIER) != 0 || >+ !(tk->mIdent.LowerCaseEqualsLiteral("inherit") || >+ tk->mIdent.LowerCaseEqualsLiteral("initial")))) { And then I think I would prefer that last clause after the tk->mType check to read: !((aVariantMask & VARIANT_IDENTIFIER_NO_INHERIT) && (tk->mIdent.LowerCaseEqualsLiteral("inherit") || tk->mIdent.LowerCaseEqualsLiteral("initial"))) but it's not a terribly strong preference. >+CSSParserImpl::ParseAnimationOrTransitionShorthand( >+ for (;;) { // loop over comma-separated transitions "or animations" >+ // whether a particular subproperty was specified for this transition "or animation" >+ for (;;) { // loop over values within a transition "or animation" >+ // check to see if we're at the end of one full transition definition "or animation definition" >+ // else, try to parse the next transition sub-property "or animation sub-property" >+ // We hit the end of the property or the end of one transition "or animation" >@@ -3873,16 +3894,331 @@ nsRuleNode::ComputeDisplayData(void* aSt >+ case eCSSUnit_Enumerated: >+ animation->SetIterationCount(NS_IEEEPositiveInfinity()); Could use an assert on the value of the enumeration there. r=me with those nits.
Comment on attachment 525293 [details] [diff] [review] patch 5: implement and parse @keyframes rules r=me with the comment nit I mentioned.
Comment on attachment 523704 [details] [diff] [review] patch 6: cascade @keyframes rules >+++ b/layout/style/nsCSSRuleProcessor.cpp >+// Append all the currently-active font face rules to aArray. Return s/font face/key frame/ r=me with that.
Comment on attachment 525042 [details] [diff] [review] patch 7: move parts of transitions code into common base class to share with animation >+++ b/layout/style/AnimationCommon.cpp >+ // Content nodes might outlive the transition manager. s/transition/animation/ >+AnimValuesStyleRule::MapRuleInfoInto(nsRuleData* aRuleData) >+ // Don't apply transitions to things inside of pseudo-elements. "or animations" r=me
Comment on attachment 523706 [details] [diff] [review] patch 8: make AddEmptyValue explicitly Infallible, and assume it is Fix the comment about "non-null" too? r=me
Comment on attachment 523829 [details] [diff] [review] patch 9: allow tests to take over control of refresh driver time and refreshes r=me
Comment on attachment 523809 [details] [diff] [review] patch 10: copy some bits of test_transitions.html into a shared file r=me
Comment on attachment 524896 [details] [diff] [review] patch 13: fix layout-dependent computed style for pseudo-elements r=me
Comment on attachment 525038 [details] [diff] [review] patch 11: implement and test animations of css animations >+++ b/layout/style/nsAnimationManager.cpp >+ * Data about one animation (i.e., one of the values of >+ * 'animation-name') animation running on an element. s/animation running/running/ >+nsAnimationManager::BuildSegment(nsTArray<AnimationSegment, >+ nsTArrayInfallibleAllocator>& aSegments, InfallibleTArray, and same in the header. r=me
I filed bug 649238 as a followup (to a FIXME in patch 11) on removing from the refresh driver more aggressively.
Comment on attachment 524897 [details] [diff] [review] patch 14: fire animation events r=bzbarsky
Comment on attachment 525288 [details] [diff] [review] patch 15: #ifdef the feature r=me
https://hg.mozilla.org/mozilla-central/rev/6bf7649f64a6 https://hg.mozilla.org/mozilla-central/rev/0a0314bdf5c6 https://hg.mozilla.org/mozilla-central/rev/b579b02a57af https://hg.mozilla.org/mozilla-central/rev/9e703bf91ff5 https://hg.mozilla.org/mozilla-central/rev/2597d6ff2793 https://hg.mozilla.org/mozilla-central/rev/f8dba37f4761 https://hg.mozilla.org/mozilla-central/rev/23d79d8f5eda https://hg.mozilla.org/mozilla-central/rev/548241dd0c12 https://hg.mozilla.org/mozilla-central/rev/f4d2a9cb8e06 https://hg.mozilla.org/mozilla-central/rev/618c5d784dac https://hg.mozilla.org/mozilla-central/rev/6ab8e5df08ec https://hg.mozilla.org/mozilla-central/rev/1c17ed72040c https://hg.mozilla.org/mozilla-central/rev/3a3c77941d26 https://hg.mozilla.org/mozilla-central/rev/6645b30313c5 https://hg.mozilla.org/mozilla-central/rev/37cc67bd29b0 https://hg.mozilla.org/mozilla-central/rev/5f6f0204b682
bustage fix for mac compiler: http://hg.mozilla.org/mozilla-central/rev/a1ab5a4ed1f3
(In reply to comment #58) > bustage fix for mac compiler: > http://hg.mozilla.org/mozilla-central/rev/a1ab5a4ed1f3 And I double-checked the mochitest log for the 32-bit Mac optimized run of test_value_computation.html (and some of the other related mochitests) to make sure the animation-iteration-count output looked as I expected it to.
And, for the record, the error that the bustage fix in comment 58 was fixing was: /var/folders/7I/7I253dv+HLesSBUBGCX08E+++TM/-Tmp-//ccFCi3BG.s:6479:non-relocatable subtraction expression, "LC14" minus "L00000000025$pb" /var/folders/7I/7I253dv+HLesSBUBGCX08E+++TM/-Tmp-//ccFCi3BG.s:6479:symbol: "L00000000025$pb" can't be undefined in a subtraction expression /var/folders/7I/7I253dv+HLesSBUBGCX08E+++TM/-Tmp-//ccFCi3BG.s:unknown:Undefined local symbol L00000000025$pb make[7]: *** [nsComputedDOMStyle.o] Error 1
Documentation has been written; a review would be appreciated: https://developer.mozilla.org/en/CSS/CSS_animations https://developer.mozilla.org/en/DOM/event/AnimationEvent https://developer.mozilla.org/en/CSS/animation https://developer.mozilla.org/en/CSS/animation-name https://developer.mozilla.org/en/CSS/animation-duration https://developer.mozilla.org/en/CSS/animation-timing-function https://developer.mozilla.org/en/CSS/animation-iteration-count https://developer.mozilla.org/en/CSS/animation-direction https://developer.mozilla.org/en/CSS/animation-play-state https://developer.mozilla.org/en/CSS/animation-delay https://developer.mozilla.org/en/CSS/@keyframes Links have also been added to Firefox 5 for developers and to the main CSS Reference index page.
or
