Changeset 287678 in webkit for trunk/Source/WebCore/css/CSSComputedStyleDeclaration.cpp

Timestamp:

Jan 6, 2022, 2:46:20 AM (3 years ago)

Author:

[email protected]

Message:

computed style for transition longhand properties is wrong
​https://bugs.webkit.org/show_bug.cgi?id=234880

Reviewed by Antti Koivisto.

LayoutTests/imported/w3c:

Mark WPT progressions.

• web-platform-tests/css/css-transitions/parsing/transition-delay-computed-expected.txt:
• web-platform-tests/css/css-transitions/parsing/transition-duration-computed-expected.txt:
• web-platform-tests/css/css-transitions/parsing/transition-property-computed-expected.txt:
• web-platform-tests/css/css-transitions/parsing/transition-timing-function-computed-expected.txt:
• web-platform-tests/css/css-transitions/transition-delay-001-expected.txt:
• web-platform-tests/css/css-transitions/transition-duration-001-expected.txt:

Source/WebCore:

We had two issues in the way we treated animation lists.

First, we would always output the filled-in values in the computed style, which
meant that setting animation-name: one, two; animation-duration: 1s would yield
1s, 1s instead of just 1s for animation-duration. So we now maintain a bit
on Animation to indicate whether a given value was set for filling and
ComputedStyleExtractor::addValueForAnimationPropertyToList() can now query it
before adding this value when returning the computed style.

Second, in RenderStyle::adjustTransitions(), we would mistakenly try to remove entries
if we found a transition-property repeated. However, this is valid so we remove the
code related to this entirely.

• css/CSSComputedStyleDeclaration.cpp:

(WebCore::ComputedStyleExtractor::addValueForAnimationPropertyToList):

• platform/animation/Animation.cpp:

(WebCore::Animation::Animation):

• platform/animation/Animation.h:

(WebCore::Animation::clearDelay):
(WebCore::Animation::clearDirection):
(WebCore::Animation::clearDuration):
(WebCore::Animation::clearFillMode):
(WebCore::Animation::clearIterationCount):
(WebCore::Animation::clearPlayState):
(WebCore::Animation::clearProperty):
(WebCore::Animation::clearTimingFunction):
(WebCore::Animation::fillDelay):
(WebCore::Animation::fillDirection):
(WebCore::Animation::fillDuration):
(WebCore::Animation::fillFillMode):
(WebCore::Animation::fillIterationCount):
(WebCore::Animation::fillPlayState):
(WebCore::Animation::fillProperty):
(WebCore::Animation::fillTimingFunction):
(WebCore::Animation::isDelayFilled const):
(WebCore::Animation::isDirectionFilled const):
(WebCore::Animation::isDurationFilled const):
(WebCore::Animation::isFillModeFilled const):
(WebCore::Animation::isIterationCountFilled const):
(WebCore::Animation::isPlayStateFilled const):
(WebCore::Animation::isPropertyFilled const):
(WebCore::Animation::isTimingFunctionFilled const):

• platform/animation/AnimationList.cpp:

(WebCore::AnimationList::fillUnsetProperties):

• rendering/style/RenderStyle.cpp:

(WebCore::RenderStyle::adjustTransitions):

LayoutTests:

This test was incorrect since it assumed that filled-in values in animation lists
would be part of the computed style. We update the expectation and add other properties
that weren't being tested.

• animations/fill-unset-properties-expected.txt:
• animations/fill-unset-properties.html:

File:

• trunk/Source/WebCore/css/CSSComputedStyleDeclaration.cpp (modified) (1 diff)

trunk/Source/WebCore/css/CSSComputedStyleDeclaration.cpp

@@ -1435 +1435 @@
 void ComputedStyleExtractor::addValueForAnimationPropertyToList(CSSValueList& list, CSSPropertyID property, const Animation* animation)
 {
-    if (property == CSSPropertyAnimationDuration || property == CSSPropertyTransitionDuration)
-        list.append(valueForAnimationDuration(animation ? animation->duration() : Animation::initialDuration()));
-    else if (property == CSSPropertyAnimationDelay || property == CSSPropertyTransitionDelay)
-        list.append(valueForAnimationDelay(animation ? animation->delay() : Animation::initialDelay()));
-    else if (property == CSSPropertyAnimationIterationCount)
-        list.append(valueForAnimationIterationCount(animation ? animation->iterationCount() : Animation::initialIterationCount()));
-    else if (property == CSSPropertyAnimationDirection)
-        list.append(valueForAnimationDirection(animation ? animation->direction() : Animation::initialDirection()));
-    else if (property == CSSPropertyAnimationFillMode)
-        list.append(valueForAnimationFillMode(animation ? animation->fillMode() : Animation::initialFillMode()));
-    else if (property == CSSPropertyAnimationPlayState)
-        list.append(valueForAnimationPlayState(animation ? animation->playState() : Animation::initialPlayState()));
-    else if (property == CSSPropertyAnimationName)
+    if (property == CSSPropertyAnimationDuration || property == CSSPropertyTransitionDuration) {
+        if (!animation || !animation->isDurationFilled())
+            list.append(valueForAnimationDuration(animation ? animation->duration() : Animation::initialDuration()));
+    } else if (property == CSSPropertyAnimationDelay || property == CSSPropertyTransitionDelay) {
+        if (!animation || !animation->isDelayFilled())
+            list.append(valueForAnimationDelay(animation ? animation->delay() : Animation::initialDelay()));
+    } else if (property == CSSPropertyAnimationIterationCount) {
+        if (!animation || !animation->isIterationCountFilled())
+            list.append(valueForAnimationIterationCount(animation ? animation->iterationCount() : Animation::initialIterationCount()));
+    } else if (property == CSSPropertyAnimationDirection) {
+        if (!animation || !animation->isDirectionFilled())
+            list.append(valueForAnimationDirection(animation ? animation->direction() : Animation::initialDirection()));
+    } else if (property == CSSPropertyAnimationFillMode) {
+        if (!animation || !animation->isFillModeFilled())
+            list.append(valueForAnimationFillMode(animation ? animation->fillMode() : Animation::initialFillMode()));
+    } else if (property == CSSPropertyAnimationPlayState) {
+        if (!animation || !animation->isPlayStateFilled())
+            list.append(valueForAnimationPlayState(animation ? animation->playState() : Animation::initialPlayState()));
+    } else if (property == CSSPropertyAnimationName)
         list.append(valueForAnimationName(animation ? animation->name() : Animation::initialName()));
     else if (property == CSSPropertyTransitionProperty) {
-        if (animation)
-            list.append(createTransitionPropertyValue(*animation));
-        else
+        if (animation) {
+            if (!animation->isPropertyFilled())
+                list.append(createTransitionPropertyValue(*animation));
+        } else
             list.append(CSSValuePool::singleton().createIdentifierValue(CSSValueAll));
     } else if (property == CSSPropertyAnimationTimingFunction || property == CSSPropertyTransitionTimingFunction) {
-        if (animation)
-            list.append(valueForAnimationTimingFunction(*animation->timingFunction()));
-        else
+        if (animation) {
+            if (!animation->isTimingFunctionFilled())
+                list.append(valueForAnimationTimingFunction(*animation->timingFunction()));
+        } else
             list.append(valueForAnimationTimingFunction(CubicBezierTimingFunction::defaultTimingFunction()));
     } else