From 121b831ff7b9a6a7f6f645dda57e1db7dbc13837 Mon Sep 17 00:00:00 2001 From: Brian Birtles Date: Tue, 14 Apr 2015 09:13:27 +0900 Subject: [PATCH] Bug 1122414 part 2 - Return the transitionProperty from Animation.name for CSS transitions; r=jwatt This is a bit awkward. We lazily set mName to the transition property and then return it. The reasons for this approach are: * We don't really want to eagerly fill in mName for all transitions since in 99% of cases we'll never use it and this will lead to wasted allocations. * The signature of Name() returns a const nsString reference. This is because Name() is used when building CSS Animations (to compare different copies of the same animation when updating). For that case we don't really want to generate unnecessary copies of nsString objects so we return a reference. However, that means for transitions as well we need to return a reference so we can't just generate a temporary string on-demand. As a result we also have to const-cast ourselves so we can update the mName member. We could make mName mutable but seeing as it's only set once, the const_cast seems more appropriate. --- dom/animation/Animation.h | 2 +- .../css-transitions/test_animation-effect-name.html | 5 +++-- layout/style/nsTransitionManager.cpp | 11 +++++++++++ layout/style/nsTransitionManager.h | 2 ++ 4 files changed, 17 insertions(+), 3 deletions(-) diff --git a/dom/animation/Animation.h b/dom/animation/Animation.h index 9bccdf63da33..0fe71ad68d49 100644 --- a/dom/animation/Animation.h +++ b/dom/animation/Animation.h @@ -249,7 +249,7 @@ public: return mTiming; } - const nsString& Name() const { + virtual const nsString& Name() const { return mName; } diff --git a/dom/animation/test/css-transitions/test_animation-effect-name.html b/dom/animation/test/css-transitions/test_animation-effect-name.html index 637e995fee3b..b764723cfc9f 100644 --- a/dom/animation/test/css-transitions/test_animation-effect-name.html +++ b/dom/animation/test/css-transitions/test_animation-effect-name.html @@ -16,8 +16,9 @@ test(function(t) { div.style.transition = 'all 100s'; div.style.left = '100px'; - assert_equals(div.getAnimations()[0].source.effect.name, '', - 'Animation effects for transitions have an empty name'); + assert_equals(div.getAnimations()[0].source.effect.name, 'left', + 'The name for the transitions corresponds to the property ' + + 'being transitioned'); }, 'Effect name for transitions'); diff --git a/layout/style/nsTransitionManager.cpp b/layout/style/nsTransitionManager.cpp index c2cfe1cb3801..5779569a4f2d 100644 --- a/layout/style/nsTransitionManager.cpp +++ b/layout/style/nsTransitionManager.cpp @@ -24,6 +24,7 @@ #include "nsIFrame.h" #include "Layers.h" #include "FrameLayerBuilder.h" +#include "nsCSSProps.h" #include "nsDisplayList.h" #include "nsStyleChangeList.h" #include "nsStyleSet.h" @@ -39,6 +40,16 @@ using namespace mozilla; using namespace mozilla::layers; using namespace mozilla::css; +const nsString& +ElementPropertyTransition::Name() const +{ + if (!mName.Length()) { + const_cast(this)->mName = + NS_ConvertUTF8toUTF16(nsCSSProps::GetStringValue(TransitionProperty())); + } + return dom::Animation::Name(); +} + double ElementPropertyTransition::CurrentValuePortion() const { diff --git a/layout/style/nsTransitionManager.h b/layout/style/nsTransitionManager.h index a197e5b43f7c..3b376741096e 100644 --- a/layout/style/nsTransitionManager.h +++ b/layout/style/nsTransitionManager.h @@ -42,6 +42,8 @@ struct ElementPropertyTransition : public dom::Animation virtual ElementPropertyTransition* AsTransition() { return this; } virtual const ElementPropertyTransition* AsTransition() const { return this; } + virtual const nsString& Name() const; + nsCSSProperty TransitionProperty() const { MOZ_ASSERT(Properties().Length() == 1, "Transitions should have exactly one animation property. "