diff --git a/layout/style/nsTransitionManager.cpp b/layout/style/nsTransitionManager.cpp index 1214ddad0582..c97f7f08d5f8 100755 --- a/layout/style/nsTransitionManager.cpp +++ b/layout/style/nsTransitionManager.cpp @@ -462,47 +462,55 @@ nsTransitionManager::DoUpdateTransitions( // Per http://lists.w3.org/Archives/Public/www-style/2009Aug/0109.html // I'll consider only the transitions from the number of items in // 'transition-property' on down, and later ones will override earlier - // ones (tracked using |whichStarted|). + // ones (tracked using |propertiesChecked|). bool startedAny = false; - nsCSSPropertyIDSet whichStarted; - for (uint32_t i = aDisp.mTransitionPropertyCount; i-- != 0; ) { - // Check the combined duration (combination of delay and duration) - // first, since it defaults to zero, which means we can ignore the - // transition. - if (aDisp.GetTransitionCombinedDuration(i) > 0.0f) { - // We might have something to transition. See if any of the - // properties in question changed and are animatable. - // FIXME: Would be good to find a way to share code between this - // interpretation of transition-property and the one below. - nsCSSPropertyID property = aDisp.GetTransitionProperty(i); - if (property == eCSSPropertyExtra_no_properties || - property == eCSSPropertyExtra_variable || - property == eCSSProperty_UNKNOWN) { - // Nothing to do, but need to exclude this from cases below. - } else if (property == eCSSPropertyExtra_all_properties) { - for (nsCSSPropertyID p = nsCSSPropertyID(0); - p < eCSSProperty_COUNT_no_shorthands; - p = nsCSSPropertyID(p + 1)) { + nsCSSPropertyIDSet propertiesChecked; + for (uint32_t i = aDisp.mTransitionPropertyCount; i--; ) { + // We're not going to look at any further transitions, so we can just avoid + // looking at this if we know it will not start any transitions. + if (i == 0 && aDisp.GetTransitionCombinedDuration(i) <= 0.0f) { + continue; + } + + nsCSSPropertyID property = aDisp.GetTransitionProperty(i); + if (property == eCSSPropertyExtra_no_properties || + property == eCSSPropertyExtra_variable || + property == eCSSProperty_UNKNOWN) { + // Nothing to do. + continue; + } + // We might have something to transition. See if any of the + // properties in question changed and are animatable. + // FIXME: Would be good to find a way to share code between this + // interpretation of transition-property and the one below. + // FIXME(emilio): This should probably just use the "all" shorthand id, and + // we should probably remove eCSSPropertyExtra_all_properties. + if (property == eCSSPropertyExtra_all_properties) { + for (nsCSSPropertyID p = nsCSSPropertyID(0); + p < eCSSProperty_COUNT_no_shorthands; + p = nsCSSPropertyID(p + 1)) { + startedAny |= ConsiderInitiatingTransition(p, aDisp, i, aElement, aPseudoType, aElementTransitions, aOldStyle, aNewStyle, - &startedAny, &whichStarted); - } - } else if (nsCSSProps::IsShorthand(property)) { - CSSPROPS_FOR_SHORTHAND_SUBPROPERTIES(subprop, property, - CSSEnabledState::eForAllContent) - { + propertiesChecked); + } + } else if (nsCSSProps::IsShorthand(property)) { + CSSPROPS_FOR_SHORTHAND_SUBPROPERTIES(subprop, property, + CSSEnabledState::eForAllContent) + { + startedAny |= ConsiderInitiatingTransition(*subprop, aDisp, i, aElement, aPseudoType, aElementTransitions, aOldStyle, aNewStyle, - &startedAny, &whichStarted); - } - } else { + propertiesChecked); + } + } else { + startedAny |= ConsiderInitiatingTransition(property, aDisp, i, aElement, aPseudoType, aElementTransitions, aOldStyle, aNewStyle, - &startedAny, &whichStarted); - } + propertiesChecked); } } @@ -631,7 +639,7 @@ IsTransitionable(nsCSSPropertyID aProperty) return Servo_Property_IsTransitionable(aProperty); } -void +bool nsTransitionManager::ConsiderInitiatingTransition( nsCSSPropertyID aProperty, const nsStyleDisplay& aStyleDisplay, @@ -641,8 +649,7 @@ nsTransitionManager::ConsiderInitiatingTransition( CSSTransitionCollection*& aElementTransitions, const ComputedStyle& aOldStyle, const ComputedStyle& aNewStyle, - bool* aStartedAny, - nsCSSPropertyIDSet* aWhichStarted) + nsCSSPropertyIDSet& aPropertiesChecked) { // IsShorthand itself will assert if aProperty is not a property. MOZ_ASSERT(!nsCSSProps::IsShorthand(aProperty), @@ -650,23 +657,36 @@ nsTransitionManager::ConsiderInitiatingTransition( NS_ASSERTION(!aElementTransitions || aElementTransitions->mElement == aElement, "Element mismatch"); + // A later item in transition-property already specified a transition for this + // property, so we ignore this one. + // See http://lists.w3.org/Archives/Public/www-style/2009Aug/0109.html . + if (aPropertiesChecked.HasProperty(aProperty)) { + return false; + } + + aPropertiesChecked.AddProperty(aProperty); + // Ignore disabled properties. We can arrive here if the transition-property // is 'all' and the disabled property has a default value which derives value // from another property, e.g. color. if (!nsCSSProps::IsEnabled(aProperty, CSSEnabledState::eForAllContent)) { - return; - } - - if (aWhichStarted->HasProperty(aProperty)) { - // A later item in transition-property already started a - // transition for this property, so we ignore this one. - // See comment above and - // http://lists.w3.org/Archives/Public/www-style/2009Aug/0109.html . - return; + return false; } if (!IsTransitionable(aProperty)) { - return; + return false; + } + + float delay = aStyleDisplay.GetTransitionDelay(transitionIdx); + + // The spec says a negative duration is treated as zero. + float duration = + std::max(aStyleDisplay.GetTransitionDuration(transitionIdx), 0.0f); + + // If the combined duration of this transition is 0 or less don't start a + // transition. + if (delay + duration <= 0.0f) { + return false; } dom::DocumentTimeline* timeline = aElement->OwnerDoc()->Timeline(); @@ -717,7 +737,7 @@ nsTransitionManager::ConsiderInitiatingTransition( if (haveCurrentTransition && haveValues && aElementTransitions->mAnimations[currentIndex]->ToValue() == endValue) { // GetAnimationRule already called RestyleForAnimation. - return; + return false; } if (!shouldAnimate) { @@ -745,17 +765,11 @@ nsTransitionManager::ConsiderInitiatingTransition( } // GetAnimationRule already called RestyleForAnimation. } - return; + return false; } const nsTimingFunction &tf = aStyleDisplay.GetTransitionTimingFunction(transitionIdx); - float delay = aStyleDisplay.GetTransitionDelay(transitionIdx); - float duration = aStyleDisplay.GetTransitionDuration(transitionIdx); - if (duration < 0.0) { - // The spec says a negative duration is treated as zero. - duration = 0.0; - } AnimationValue startForReversingTest = startValue; double reversePortion = 1.0; @@ -839,7 +853,7 @@ nsTransitionManager::ConsiderInitiatingTransition( if (!aElementTransitions) { MOZ_ASSERT(!createdCollection, "outparam should agree with return value"); NS_WARNING("allocating collection failed"); - return; + return false; } if (createdCollection) { @@ -886,7 +900,7 @@ nsTransitionManager::ConsiderInitiatingTransition( } else { if (!animations.AppendElement(animation)) { NS_WARNING("out of memory"); - return; + return false; } } @@ -895,7 +909,6 @@ nsTransitionManager::ConsiderInitiatingTransition( effectSet->UpdateAnimationGeneration(mPresContext); } - *aStartedAny = true; - aWhichStarted->AddProperty(aProperty); + return true; } diff --git a/layout/style/nsTransitionManager.h b/layout/style/nsTransitionManager.h index 7d291f99bd8b..7947f10e42a2 100644 --- a/layout/style/nsTransitionManager.h +++ b/layout/style/nsTransitionManager.h @@ -339,7 +339,8 @@ protected: const mozilla::ComputedStyle& aOldStyle, const mozilla::ComputedStyle& aNewStyle); - void ConsiderInitiatingTransition(nsCSSPropertyID aProperty, + // Returns whether the transition actually started. + bool ConsiderInitiatingTransition(nsCSSPropertyID aProperty, const nsStyleDisplay& aStyleDisplay, uint32_t transitionIdx, mozilla::dom::Element* aElement, @@ -347,9 +348,7 @@ protected: CSSTransitionCollection*& aElementTransitions, const mozilla::ComputedStyle& aOldStyle, const mozilla::ComputedStyle& aNewStyle, - bool* aStartedAny, - nsCSSPropertyIDSet* aWhichStarted); - + nsCSSPropertyIDSet& aPropertiesChecked); }; #endif /* !defined(nsTransitionManager_h_) */ diff --git a/testing/web-platform/meta/MANIFEST.json b/testing/web-platform/meta/MANIFEST.json index 304c8b11f747..319487b1c083 100644 --- a/testing/web-platform/meta/MANIFEST.json +++ b/testing/web-platform/meta/MANIFEST.json @@ -321543,6 +321543,12 @@ {} ] ], + "css/css-transitions/zero-duration-multiple-transition.html": [ + [ + "/css/css-transitions/zero-duration-multiple-transition.html", + {} + ] + ], "css/css-typed-om/CSSMatrixComponent-DOMMatrix-mutable.html": [ [ "/css/css-typed-om/CSSMatrixComponent-DOMMatrix-mutable.html", @@ -532016,6 +532022,10 @@ "538b95863c061da60e95c1a61ef9dc93da007aa4", "testharness" ], + "css/css-transitions/zero-duration-multiple-transition.html": [ + "7db6bc8641a6a2eb5f1ee1fdbc9b6215a0462bbc", + "testharness" + ], "css/css-typed-om/CSSMatrixComponent-DOMMatrix-mutable.html": [ "f6056e2480829c7aa9885673d332496faf7777b5", "testharness" diff --git a/testing/web-platform/tests/css/css-transitions/zero-duration-multiple-transition.html b/testing/web-platform/tests/css/css-transitions/zero-duration-multiple-transition.html new file mode 100644 index 000000000000..4268ce766d76 --- /dev/null +++ b/testing/web-platform/tests/css/css-transitions/zero-duration-multiple-transition.html @@ -0,0 +1,30 @@ + + + + + + + + +
+