[@Property] CSS custom properties containing var() are not updated when the referenced property is animated

anttijk · anttijk · commit fee955ee6904 · 2023-01-11T07:36:59.000-08:00
https://bugs.webkit.org/show_bug.cgi?id=250448 rdar://104080598 Reviewed by Antoine Quint. * LayoutTests/imported/w3c/web-platform-tests/css/css-properties-values-api/at-property-animation-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-properties-values-api/at-property-animation.html: * Source/WebCore/style/PropertyCascade.cpp: (WebCore::Style::PropertyCascade::shouldApplyAfterAnimation): Also check for custom properties containing var() when figuring out the post-animation cascade. Canonical link: https://commits.webkit.org/258786@main
diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-properties-values-api/at-property-animation-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/css/css-properties-values-api/at-property-animation-expected.txt
@@ -11,4 +11,6 @@ PASS Transitioning from specified value
 FAIL Transition triggered by initial value change assert_equals: expected "150px" but got "200px"
 PASS No transition when changing types
 PASS No transition when removing @property rule
+PASS Unregistered properties referencing animated properties update correctly.
+PASS Registered properties referencing animated properties update correctly.
 
diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-properties-values-api/at-property-animation.html b/LayoutTests/imported/w3c/web-platform-tests/css/css-properties-values-api/at-property-animation.html
@@ -295,4 +295,47 @@
   });
 }, 'No transition when removing @property rule');
 
+test_with_at_property({
+  syntax: '"<length>"',
+  inherits: false,
+  initialValue: '0px'
+}, (name) => {
+  with_style_node(`
+    @keyframes test {
+      from { ${name}: 100px; }
+      to { ${name}: 200px; }
+    }
+    #div {
+        animation: test 100s -50s linear;
+        --unregistered: var(${name});
+    }
+  `, () => {
+    assert_equals(getComputedStyle(div).getPropertyValue('--unregistered'), '150px');
+  });
+}, 'Unregistered properties referencing animated properties update correctly.');
+
+test_with_at_property({
+  syntax: '"<length>"',
+  inherits: false,
+  initialValue: '0px'
+}, (name) => {
+  with_style_node(`
+    @keyframes test {
+      from { ${name}: 100px; }
+      to { ${name}: 200px; }
+    }
+    @property --registered {
+        syntax: "<length>";
+        inherits: false;
+        initialValue: 0px;
+    }
+    #div {
+        animation: test 100s -50s linear;
+        --registered: var(${name});
+    }
+  `, () => {
+    assert_equals(getComputedStyle(div).getPropertyValue('--registered'), '150px');
+  });
+}, 'Registered properties referencing animated properties update correctly.');
+
 </script>
diff --git a/Source/WebCore/style/PropertyCascade.cpp b/Source/WebCore/style/PropertyCascade.cpp
@@ -261,9 +261,12 @@ bool PropertyCascade::shouldApplyAfterAnimation(const StyleProperties::PropertyR
     }
 
     // If we are animating custom properties they may affect other properties so we need to re-resolve them.
-    if (m_animationLayer->hasCustomProperties && property.value()->isVariableReferenceValue()) {
+    if (m_animationLayer->hasCustomProperties) {
         // We could check if the we are actually animating the referenced variable. Indirect cases would need to be taken into account.
-        return true;
+        if (customProperty && !customProperty->isResolved())
+            return true;
+        if (property.value()->isVariableReferenceValue())
+            return true;
     }
 
     // Check for 'em' units and similar property dependencies.

Original file line number	Diff line number	Diff line change
`@@ -261,9 +261,12 @@ bool PropertyCascade::shouldApplyAfterAnimation(const StyleProperties::PropertyR`
`261`	`261`	`}`
`262`	`262`
`263`	`263`	`// If we are animating custom properties they may affect other properties so we need to re-resolve them.`
`264`		`- if (m_animationLayer->hasCustomProperties && property.value()->isVariableReferenceValue()) {`
	`264`	`+ if (m_animationLayer->hasCustomProperties) {`
`265`	`265`	`// We could check if the we are actually animating the referenced variable. Indirect cases would need to be taken into account.`
`266`		`- return true;`
	`266`	`+ if (customProperty && !customProperty->isResolved())`
	`267`	`+ return true;`
	`268`	`+ if (property.value()->isVariableReferenceValue())`
	`269`	`+ return true;`
`267`	`270`	`}`
`268`	`271`
`269`	`272`	`// Check for 'em' units and similar property dependencies.`