Last Comment Bug 435442 - Implement Webkit's CSS Animation proposal
: Implement Webkit's CSS Animation proposal
Status: RESOLVED FIXED
[css-animations]
: css3, dev-doc-complete
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: All All
: P4 enhancement with 29 votes (vote)
: mozilla5
Assigned To: David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
:
Mentors:
http://www.w3.org/TR/css3-animations/
Depends on: 332335 435441 577974 636029 650469 652976 CVE-2012-1953
Blocks: 829369
  Show dependency treegraph
 
Reported: 2008-05-23 09:20 PDT by Zack Weinberg (:zwol)
Modified: 2014-03-04 04:59 PST (History)
61 users (show)
roc: blocking1.9.2-
roc: wanted1.9.2-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch 1: add new properties (85.63 KB, patch)
2011-04-01 15:10 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
no flags Details | Diff | Splinter Review
patch 2: share GetCSSParsingEnvironment (5.97 KB, patch)
2011-04-01 15:11 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
no flags Details | Diff | Splinter Review
patch 3: handle modifications coming from grandchildren of sheet better (7.77 KB, patch)
2011-04-01 15:11 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
bzbarsky: review+
Details | Diff | Splinter Review
patch 4: fix style rule inherit macros (3.21 KB, patch)
2011-04-01 15:11 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
no flags Details | Diff | Splinter Review
patch 5: implement and parse @keyframes rules (49.33 KB, patch)
2011-04-01 15:12 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
no flags Details | Diff | Splinter Review
patch 6: cascade @keyframes rules (9.49 KB, patch)
2011-04-01 15:12 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
bzbarsky: review+
Details | Diff | Splinter Review
patch 7: move parts of transitions code into common base class to share with animation (31.58 KB, patch)
2011-04-01 15:13 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
no flags Details | Diff | Splinter Review
patch 8: make AddEmptyValue explicitly Infallible, and assume it is (2.03 KB, patch)
2011-04-01 15:14 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
bzbarsky: review+
Details | Diff | Splinter Review
patch 0: implement step-start, step-end, and steps() (36.13 KB, patch)
2011-04-01 18:54 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
bzbarsky: review+
Details | Diff | Splinter Review
patch 1: add new properties (79.87 KB, patch)
2011-04-01 18:55 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
bzbarsky: review+
Details | Diff | Splinter Review
patch 7: move parts of transitions code into common base class to share with animation (35.92 KB, patch)
2011-04-01 18:55 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
no flags Details | Diff | Splinter Review
patch 9: allow tests to take over control of refresh driver time and refreshes (8.65 KB, patch)
2011-04-02 12:36 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
no flags Details | Diff | Splinter Review
patch 10: copy some bits of test_transitions.html into a shared file (6.22 KB, patch)
2011-04-02 12:37 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
bzbarsky: review+
Details | Diff | Splinter Review
patch 9: allow tests to take over control of refresh driver time and refreshes (9.40 KB, patch)
2011-04-02 16:16 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
bzbarsky: review+
Details | Diff | Splinter Review
patch 11: implement and test animations of css animations (103.45 KB, patch)
2011-04-07 14:52 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
no flags Details | Diff | Splinter Review
patch 12: animation event structures (21.69 KB, patch)
2011-04-07 16:40 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
no flags Details | Diff | Splinter Review
patch 11: implement and test animations of css animations (103.37 KB, patch)
2011-04-09 09:30 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
no flags Details | Diff | Splinter Review
patch 12: animation event structures (26.68 KB, patch)
2011-04-09 09:31 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
bugs: review+
Details | Diff | Splinter Review
patch 11: implement and test animations of css animations (103.55 KB, patch)
2011-04-09 17:40 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
no flags Details | Diff | Splinter Review
patch 13: fix layout-dependent computed style for pseudo-elements (2.54 KB, patch)
2011-04-09 17:41 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
bzbarsky: review+
Details | Diff | Splinter Review
patch 14: fire animation events (25.28 KB, patch)
2011-04-09 17:42 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
bzbarsky: review+
Details | Diff | Splinter Review
patch 2: share GetCSSParsingEnvironment (5.93 KB, patch)
2011-04-10 22:14 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
bzbarsky: review+
Details | Diff | Splinter Review
patch 4: fix style rule inherit macros (1.94 KB, patch)
2011-04-10 22:15 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
bzbarsky: review+
Details | Diff | Splinter Review
patch 5: implement and parse @keyframes rules (48.86 KB, patch)
2011-04-10 22:15 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
no flags Details | Diff | Splinter Review
patch 11: implement and test animations of css animations (103.49 KB, patch)
2011-04-10 22:16 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
bzbarsky: review+
Details | Diff | Splinter Review
patch 7: move parts of transitions code into common base class to share with animation (36.03 KB, patch)
2011-04-10 23:36 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
bzbarsky: review+
Details | Diff | Splinter Review
patch 15: #ifdef the feature (82.78 KB, patch)
2011-04-11 20:24 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
bzbarsky: review+
Details | Diff | Splinter Review
patch 5: implement and parse @keyframes rules (47.56 KB, patch)
2011-04-11 20:41 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
bzbarsky: review+
Details | Diff | Splinter Review

Description Zack Weinberg (:zwol) 2008-05-23 09:20:42 PDT
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).
Comment 1 Marek Stępień [:marcoos, inactive] 2009-08-06 05:39:25 PDT
The draft spec has moved to w3.org, updating the bug's URL accordingly.
Comment 2 Michael Kohler [:mkohler] 2010-05-13 10:04:21 PDT
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.
Comment 3 Jose Fandos 2010-09-05 11:20:40 PDT
How does this bug relate to columns support - CSS3 (MultiCol) as per https://developer.mozilla.org/en/Mozilla_CSS_support_chart?
Comment 4 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2010-09-05 11:51:19 PDT
It doesn't.
Comment 5 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-03-16 08:33:22 PDT
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.)
Comment 6 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-01 15:10:41 PDT
Created attachment 523699 [details] [diff] [review]
patch 1: add new properties

Much like transitions.
Comment 7 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-01 15:11:02 PDT
Created attachment 523700 [details] [diff] [review]
patch 2: share GetCSSParsingEnvironment
Comment 8 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-01 15:11:31 PDT
Created attachment 523701 [details] [diff] [review]
patch 3: handle modifications coming from grandchildren of sheet better
Comment 9 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-01 15:11:52 PDT
Created attachment 523702 [details] [diff] [review]
patch 4: fix style rule inherit macros
Comment 10 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-01 15:12:22 PDT
Created attachment 523703 [details] [diff] [review]
patch 5: implement and parse @keyframes rules
Comment 11 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-01 15:12:57 PDT
Created attachment 523704 [details] [diff] [review]
patch 6: cascade @keyframes rules

Much like @font-face rules.
Comment 12 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-01 15:13:23 PDT
Created attachment 523705 [details] [diff] [review]
patch 7: move parts of transitions code into common base class to share with animation
Comment 13 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-01 15:14:00 PDT
Created attachment 523706 [details] [diff] [review]
patch 8: make AddEmptyValue explicitly Infallible, and assume it is
Comment 14 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-01 15:14:35 PDT
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 15 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-01 15:16:18 PDT
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
Comment 16 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-01 18:54:51 PDT
Created attachment 523743 [details] [diff] [review]
patch 0: implement step-start, step-end, and steps()
Comment 17 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-01 18:55:22 PDT
Created attachment 523744 [details] [diff] [review]
patch 1: add new properties
Comment 18 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-01 18:55:51 PDT
Created attachment 523745 [details] [diff] [review]
patch 7: move parts of transitions code into common base class to share with animation
Comment 19 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-02 12:36:52 PDT
Created attachment 523808 [details] [diff] [review]
patch 9: allow tests to take over control of refresh driver time and refreshes
Comment 20 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-02 12:37:54 PDT
Created attachment 523809 [details] [diff] [review]
patch 10: copy some bits of test_transitions.html into a shared file
Comment 21 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-02 16:16:16 PDT
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.
Comment 22 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-07 14:52:20 PDT
Created attachment 524497 [details] [diff] [review]
patch 11: implement and test animations of css animations
Comment 23 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-07 16:40:54 PDT
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.
Comment 24 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-09 09:30:39 PDT
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).
Comment 25 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-09 09:31:17 PDT
Created attachment 524857 [details] [diff] [review]
patch 12: animation event structures

I missed a few pieces related to the events.
Comment 26 Olli Pettay [:smaug] 2011-04-09 12:18:03 PDT
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
Comment 27 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-09 17:40:46 PDT
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).
Comment 28 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-09 17:41:46 PDT
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.
Comment 29 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-09 17:42:12 PDT
Created attachment 524897 [details] [diff] [review]
patch 14: fire animation events
Comment 30 :Ms2ger (⌚ UTC+1/+2) 2011-04-10 08:22:27 PDT
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?
Comment 31 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-10 17:44:29 PDT
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.
Comment 32 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-10 22:14:59 PDT
Created attachment 525035 [details] [diff] [review]
patch 2: share GetCSSParsingEnvironment
Comment 33 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-10 22:15:26 PDT
Created attachment 525036 [details] [diff] [review]
patch 4: fix style rule inherit macros
Comment 34 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-10 22:15:59 PDT
Created attachment 525037 [details] [diff] [review]
patch 5: implement and parse @keyframes rules
Comment 35 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-10 22:16:34 PDT
Created attachment 525038 [details] [diff] [review]
patch 11: implement and test animations of css animations
Comment 36 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-10 23:36:20 PDT
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 37 Boris Zbarsky [:bz] 2011-04-11 15:46:00 PDT
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?
Comment 38 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-11 15:53:44 PDT
(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 39 Boris Zbarsky [:bz] 2011-04-11 16:14:22 PDT
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.
Comment 40 Boris Zbarsky [:bz] 2011-04-11 16:20:16 PDT
Er, ignore comment 39.  I misread the EnsureUniqueInner change.
Comment 41 Boris Zbarsky [:bz] 2011-04-11 16:20:58 PDT
Comment on attachment 523701 [details] [diff] [review]
patch 3: handle modifications coming from grandchildren of sheet better

r=me
Comment 42 Boris Zbarsky [:bz] 2011-04-11 18:31:36 PDT
Comment on attachment 523743 [details] [diff] [review]
patch 0: implement step-start, step-end, and steps()

r=me
Comment 43 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-11 20:24:32 PDT
Created attachment 525288 [details] [diff] [review]
patch 15: #ifdef the feature
Comment 44 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-11 20:41:40 PDT
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 45 Boris Zbarsky [:bz] 2011-04-11 20:59:08 PDT
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 46 Boris Zbarsky [:bz] 2011-04-11 21:25:40 PDT
Comment on attachment 525293 [details] [diff] [review]
patch 5: implement and parse @keyframes rules

r=me with the comment nit I mentioned.
Comment 47 Boris Zbarsky [:bz] 2011-04-11 21:28:04 PDT
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 48 Boris Zbarsky [:bz] 2011-04-11 21:46:14 PDT
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 49 Boris Zbarsky [:bz] 2011-04-11 21:47:05 PDT
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 50 Boris Zbarsky [:bz] 2011-04-11 21:49:32 PDT
Comment on attachment 523829 [details] [diff] [review]
patch 9: allow tests to take over control of refresh driver time and refreshes

r=me
Comment 51 Boris Zbarsky [:bz] 2011-04-11 21:50:43 PDT
Comment on attachment 523809 [details] [diff] [review]
patch 10: copy some bits of test_transitions.html into a shared file

r=me
Comment 52 Boris Zbarsky [:bz] 2011-04-11 21:51:41 PDT
Comment on attachment 524896 [details] [diff] [review]
patch 13: fix layout-dependent computed style for pseudo-elements

r=me
Comment 53 Boris Zbarsky [:bz] 2011-04-11 22:36:29 PDT
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
Comment 54 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-11 22:37:57 PDT
I filed bug 649238 as a followup (to a FIXME in patch 11) on removing from the refresh driver more aggressively.
Comment 55 Boris Zbarsky [:bz] 2011-04-11 22:47:10 PDT
Comment on attachment 524897 [details] [diff] [review]
patch 14: fire animation events

r=bzbarsky
Comment 56 Boris Zbarsky [:bz] 2011-04-11 22:50:21 PDT
Comment on attachment 525288 [details] [diff] [review]
patch 15: #ifdef the feature

r=me
Comment 58 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-12 01:14:17 PDT
bustage fix for mac compiler:
http://hg.mozilla.org/mozilla-central/rev/a1ab5a4ed1f3
Comment 59 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-12 08:40:24 PDT
(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.
Comment 60 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-12 10:02:55 PDT
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

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