10BC0 [css-transitions] `transition-property: all` does not apply to custom… · WebKit/WebKit@fcdce8f · GitHub
[go: up one dir, main page]

Skip to content

Commit fcdce8f

Browse files
committed
[css-transitions] transition-property: all does not apply to custom properties
https://bugs.webkit.org/show_bug.cgi?id=252312 Reviewed by Antti Koivisto. When setting `transition-property: all`, which also happens to be that property's initial value, we would correctly consider all properties known within WebCore as a CSSPropertyID, but we would fail to consider registered custom properties that could otherwise be interpolated. We now iterate through all custom properties in both the before-change and after-change styles to gather custom properties that can be interpolated (ie. either a SyntaxValue or SyntaxValueList) and run the transition update logic on those. * LayoutTests/imported/w3c/web-platform-tests/css/css-properties-values-api/animation/custom-property-transition-property-all-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-properties-values-api/animation/custom-property-transition-property-all.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-properties-values-api/resources/utils.js: * Source/WebCore/style/Styleable.cpp: (WebCore::Styleable::updateCSSTransitions const): Canonical link: https://commits.webkit.org/260384@main
1 parent 47ace80 commit fcdce8f

File tree

4 files changed

+44
-2
lines changed

4 files changed

+44
-2
lines changed
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
2+
PASS A custom property can yield a CSS Transition with transition-property: all
3+
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<!DOCTYPE html>
2+
<link rel="help" href="https://drafts.css-houdini.org/css-properties-values-api-1">
3+
<script src="/resources/testharness.js"></script>
4+
<script src="/resources/testharnessreport.js"></script>
5+
<script src="../resources/utils.js"></script>
6+
<div id="target"></div>
7+
<script>
8+
9+
transition_test({
10+
syntax: "<length>",
11+
from: "100px",
12+
to: "200px",
13+
expected: "150px",
14+
transitionProperty: "all"
15+
}, 'A custom property can yield a CSS Transition with transition-property: all');
16+
17+
</script>

LayoutTests/imported/w3c/web-platform-tests/css/css-properties-values-api/resources/utils.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,8 @@ function transition_test(options, description) {
183183
promise_test(async () => {
184184
const customProperty = generate_name();
185185

186+
options.transitionProperty ??= customProperty;
187+
186188
CSS.registerProperty({
187189
name: customProperty,
188190
syntax: options.syntax,
@@ -199,7 +201,7 @@ function transition_test(options, description) {
199201
});
200202
});
201203

202-
target.style.transition = `${customProperty} 1s -500ms linear`;
204+
target.style.transition = `${options.transitionProperty} 1s -500ms linear`;
203205
target.style.setProperty(customProperty, options.to);
204206

205207
const animations = target.getAnimations();

Source/WebCore/style/Styleable.cpp

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#include "AnimationList.h"
3232
#include "AnimationTimeline.h"
3333
#include "CSSAnimation.h"
34+
#include "CSSCustomPropertyValue.h"
3435
#include "CSSPropertyAnimation.h"
3536
#include "CSSTransition.h"
3637
#include "CommonAtomStrings.h"
@@ -44,6 +45,7 @@
4445
#include "RenderListItem.h"
4546
#include "RenderListMarker.h"
4647
#include "RenderStyle.h"
48+
#include "StyleCustomPropertyData.h"
4749
#include "StylePropertyShorthand.h"
4850
#include "StyleResolver.h"
4951
#include "StyleScope.h"
@@ -663,7 +665,6 @@ void Styleable::updateCSSTransitions(const RenderStyle& currentStyle, const Rend
663665
compileTransitionPropertiesInStyle(newStyle, transitionProperties, transitionPropertiesContainAll);
664666

665667
if (transitionPropertiesContainAll) {
666-
// FIXME: this does not deal with custom properties.
667668
auto numberOfProperties = CSSPropertyAnimation::getNumProperties();
668669
for (int propertyIndex = 0; propertyIndex < numberOfProperties; ++propertyIndex) {
669670
std::optional<bool> isShorthand;
@@ -672,6 +673,25 @@ void Styleable::updateCSSTransitions(const RenderStyle& currentStyle, const Rend
672673
continue;
673674
updateCSSTransitionsForStyleableAndProperty(*this, property, currentStyle, newStyle, generationTime);
674675
}
676+
677+
HashSet<AtomString> animatableCustomProperties;
678+
auto gatherAnimatableCustomProperties = [&](const StyleCustomPropertyData& customPropertyData) {
679+
customPropertyData.forEach([&](auto& customPropertyAndValuePair) {
680+
auto [customProperty, customPropertyValue] = customPropertyAndValuePair;
681+
if (!customPropertyValue)
682+
return;
683+
auto& variantValue = customPropertyValue->value();
684+
if (std::holds_alternative<CSSCustomPropertyValue::SyntaxValue>(variantValue)
685+
|| std::holds_alternative<CSSCustomPropertyValue::SyntaxValueList>(variantValue))
686+
animatableCustomProperties.add(customProperty);
687+
});
688+
};
689+
gatherAnimatableCustomProperties(currentStyle.inheritedCustomProperties());
690+
gatherAnimatableCustomProperties(currentStyle.nonInheritedCustomProperties());
691+
gatherAnimatableCustomProperties(newStyle.inheritedCustomProperties());
692+
gatherAnimatableCustomProperties(newStyle.nonInheritedCustomProperties());
693+
for (auto& customProperty : animatableCustomProperties)
694+
updateCSSTransitionsForStyleableAndProperty(*this, customProperty, currentStyle, newStyle, generationTime);
675695
return;
676696
}
677697

0 commit comments

Comments
 (0)
0