Change css3-animations behavior for repeated keys in an @keyframes rule: do replacement on a per-property basis. (Bug 738003) r=bzbarsky

This implements my proposal in
http://lists.w3.org/Archives/Public/www-style/2011Apr/0381.html and
http://lists.w3.org/Archives/Public/www-style/2011Apr/0387.html .  I
think it was a serious mistake to implement what the spec says, and I'm
fixing that mistake so that we have a chance to change the spec.

In other words, when an @keyframes rule has two key selectors at the
same time, the later one no longer overrides the entirety of the earlier
one.  The overriding is done for each property that's in the later rule.
(And the -moz-animation-timing-function is taken only from the keyframe
actually used for the given property; if there's no declaration there
then the computed value of the property is used.)

The test for @keyframes cascade fails without the patch; the test for
@keyframes cascade2 tests behavior that works both before and after the
patch.
This commit is contained in:
L. David Baron 2012-03-21 22:10:02 -07:00
parent 3b8fa5f259
commit c9197983ca
2 changed files with 102 additions and 27 deletions

View File

@ -587,27 +587,16 @@ private:
struct KeyframeData {
float mKey;
PRUint32 mIndex; // store original order since sort algorithm is not stable
nsCSSKeyframeRule *mRule;
};
typedef InfallibleTArray<KeyframeData> KeyframeDataArray;
static PLDHashOperator
AppendKeyframeData(const float &aKey, nsCSSKeyframeRule *aRule, void *aData)
{
KeyframeDataArray *array = static_cast<KeyframeDataArray*>(aData);
KeyframeData *data = array->AppendElement();
data->mKey = aKey;
data->mRule = aRule;
return PL_DHASH_NEXT;
}
struct KeyframeDataComparator {
bool Equals(const KeyframeData& A, const KeyframeData& B) const {
return A.mKey == B.mKey;
return A.mKey == B.mKey && A.mIndex == B.mIndex;
}
bool LessThan(const KeyframeData& A, const KeyframeData& B) const {
return A.mKey < B.mKey;
return A.mKey < B.mKey || (A.mKey == B.mKey && A.mIndex < B.mIndex);
}
};
@ -685,11 +674,15 @@ nsAnimationManager::BuildAnimations(nsStyleContext* aStyleContext,
continue;
}
// Build the set of unique keyframes in the @keyframes rule. Per
// css3-animations, later keyframes with the same key replace
// earlier ones (no cascading).
nsDataHashtable<PercentageHashKey, nsCSSKeyframeRule*> keyframes;
keyframes.Init(16); // FIXME: make infallible!
// While current drafts of css3-animations say that later keyframes
// with the same key entirely replace earlier ones (no cascading),
// this is a bad idea and contradictory to the rest of CSS. So
// we're going to keep all the keyframes for each key and then do
// the replacement on a per-property basis rather than a per-rule
// basis, just like everything else in CSS.
AutoInfallibleTArray<KeyframeData, 16> sortedKeyframes;
for (PRUint32 ruleIdx = 0, ruleEnd = rule->StyleRuleCount();
ruleIdx != ruleEnd; ++ruleIdx) {
css::Rule* cssRule = rule->GetStyleRuleAt(ruleIdx);
@ -707,13 +700,14 @@ nsAnimationManager::BuildAnimations(nsStyleContext* aStyleContext,
// (And PercentageHashKey currently assumes we either ignore or
// clamp them.)
if (0.0f <= key && key <= 1.0f) {
keyframes.Put(key, kfRule);
KeyframeData *data = sortedKeyframes.AppendElement();
data->mKey = key;
data->mIndex = ruleIdx;
data->mRule = kfRule;
}
}
}
KeyframeDataArray sortedKeyframes;
keyframes.EnumerateRead(AppendKeyframeData, &sortedKeyframes);
sortedKeyframes.Sort(KeyframeDataComparator());
if (sortedKeyframes.Length() == 0) {
@ -742,18 +736,37 @@ nsAnimationManager::BuildAnimations(nsStyleContext* aStyleContext,
continue;
}
// Build a list of the keyframes to use for this property. This
// means we need every keyframe with the property in it, except
// for those keyframes where a later keyframe with the *same key*
// also has the property.
AutoInfallibleTArray<PRUint32, 16> keyframesWithProperty;
float lastKey = 100.0f; // an invalid key
for (PRUint32 kfIdx = 0, kfEnd = sortedKeyframes.Length();
kfIdx != kfEnd; ++kfIdx) {
KeyframeData &kf = sortedKeyframes[kfIdx];
if (!kf.mRule->Declaration()->HasProperty(prop)) {
continue;
}
if (kf.mKey == lastKey) {
// Replace previous occurrence of same key.
keyframesWithProperty[keyframesWithProperty.Length() - 1] = kfIdx;
} else {
keyframesWithProperty.AppendElement(kfIdx);
}
lastKey = kf.mKey;
}
AnimationProperty &propData = *aDest.mProperties.AppendElement();
propData.mProperty = prop;
KeyframeData *fromKeyframe = nsnull;
nsRefPtr<nsStyleContext> fromContext;
bool interpolated = true;
for (PRUint32 kfIdx = 0, kfEnd = sortedKeyframes.Length();
kfIdx != kfEnd; ++kfIdx) {
for (PRUint32 wpIdx = 0, wpEnd = keyframesWithProperty.Length();
wpIdx != wpEnd; ++wpIdx) {
PRUint32 kfIdx = keyframesWithProperty[wpIdx];
KeyframeData &toKeyframe = sortedKeyframes[kfIdx];
if (!toKeyframe.mRule->Declaration()->HasProperty(prop)) {
continue;
}
nsRefPtr<nsStyleContext> toContext =
resolvedStyles.Get(mPresContext, aStyleContext, toKeyframe.mRule);

View File

@ -87,6 +87,20 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=435442
0%, 100% { line-height: 3; margin-top: 20px }
50% { margin-top: 120px }
}
@-moz-keyframes cascade {
0%, 25%, 100% { top: 0 }
50%, 75% { top: 100px }
0%, 75%, 100% { left: 0 }
25%, 50% { left: 100px }
}
@-moz-keyframes cascade2 {
0% { text-indent: 0 }
25% { text-indent: 30px; -moz-animation-timing-function: ease-in } /* beaten by rule below */
50% { text-indent: 0 }
25% { text-indent: 50px }
100% { text-indent: 100px }
}
</style>
</head>
<body>
@ -1252,6 +1266,54 @@ is(cs.marginTop, "0px", "bug 686656 test 2 at 1000ms");
done_div();
display.style.overflow = "";
// Test that cascading between keyframes rules is per-property rather
// than per-rule (bug ), and that the timing function isn't taken from a
// rule that's skipped. (Bug 738003)
new_div("-moz-animation: cascade 1s linear forwards; position: relative");
is(cs.top, "0px", "cascade test (top) at 0ms");
is(cs.left, "0px", "cascade test (top) at 0ms");
advance_clock(125);
is(cs.top, "0px", "cascade test (top) at 125ms");
is(cs.left, "50px", "cascade test (top) at 125ms");
advance_clock(125);
is(cs.top, "0px", "cascade test (top) at 250ms");
is(cs.left, "100px", "cascade test (top) at 250ms");
advance_clock(125);
is(cs.top, "50px", "cascade test (top) at 375ms");
is(cs.left, "100px", "cascade test (top) at 375ms");
advance_clock(125);
is(cs.top, "100px", "cascade test (top) at 500ms");
is(cs.left, "100px", "cascade test (top) at 500ms");
advance_clock(125);
is(cs.top, "100px", "cascade test (top) at 625ms");
is(cs.left, "50px", "cascade test (top) at 625ms");
advance_clock(125);
is(cs.top, "100px", "cascade test (top) at 750ms");
is(cs.left, "0px", "cascade test (top) at 750ms");
advance_clock(125);
is(cs.top, "50px", "cascade test (top) at 875ms");
is(cs.left, "0px", "cascade test (top) at 875ms");
advance_clock(125);
is(cs.top, "0px", "cascade test (top) at 1000ms");
is(cs.left, "0px", "cascade test (top) at 1000ms");
done_div();
new_div("-moz-animation: cascade2 8s linear forwards");
is(cs.textIndent, "0px", "cascade2 test at 0s");
advance_clock(1000);
is(cs.textIndent, "25px", "cascade2 test at 1s");
advance_clock(1000);
is(cs.textIndent, "50px", "cascade2 test at 2s");
advance_clock(1000);
is(cs.textIndent, "25px", "cascade2 test at 3s");
advance_clock(1000);
is(cs.textIndent, "0px", "cascade2 test at 4s");
advance_clock(3000);
is(cs.textIndent, "75px", "cascade2 test at 7s");
advance_clock(1000);
is(cs.textIndent, "100px", "cascade2 test at 8s");
done_div();
SpecialPowers.DOMWindowUtils.restoreNormalRefresh();
</script>