nagai/fune
3
0
Fork 0
forked from mirrors/gecko-dev
Code Pull requests Activity

Bug 1461070: Skip starting other transitions based on specified, not already-started transitions. r=birtles,dbaron

Browse source 
MozReview-Commit-ID: 3D5elrj2Ypi
This commit is contained in:
Emilio Cobos Álvarez 2018-05-12 10:51:48 +02:00
parent bd7bc7fd80
commit 3a031c33a3
4 changed files with 112 additions and 60 deletions
repo.diff.show_diff_stats Download patch file Download diff file Expand all files Collapse all files

125
layout/style/nsTransitionManager.cpp
View file

@ -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;
}

7
layout/style/nsTransitionManager.h
View file

@ -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_) */

10
testing/web-platform/meta/MANIFEST.json
View file

@ -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"

30
testing/web-platform/tests/css/css-transitions/zero-duration-multiple-transition.html Normal file
View file

@ -0,0 +1,30 @@
<!doctype html>
<meta charset="utf-8">
<link rel="help" href="https://drafts.csswg.org/css-transitions/#starting">
<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1461070">
<link rel="author" title="Emilio Cobos Álvarez" href="emilio@crisal.io">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<style>
#t {
color: red;
transition-property: color, color;
transition-duration: 1s, 0s;
}
</style>
<div id="t"></div>
<script>
test(function() {
let div = document.getElementById("t");
assert_equals(getComputedStyle(div).color, "rgb(255, 0, 0)");
div.style.color = "green";
assert_equals(
div.getAnimations().length,
0,
"No transition should've started"
);
assert_equals(getComputedStyle(div).color, "rgb(0, 128, 0)");
}, "transition-duration of 0 prevents earlier transitions with the same property from starting.");
</script>