Skip to content
Permalink
Browse files
[web-animations] support custom properties in JS-originated animations
https://bugs.webkit.org/show_bug.cgi?id=242005

Reviewed by Dean Jackson.

Add support for custom properties in JS-originated animations as created via the Element.animate()
method, the KeyframeEffect constructor and the KeyframeEffect.setKeyframes() method.

We add the notion of a custom property while parsing, first in the PropertyAndValues which is used
as an intermediary object while parsing, and then in ComputedKeyframe which is stored on the KeyframeEffect
as the final parsed data structure and as returned through getKeyframes().

The code changes themselves are pretty straightforward where everywhere we handle a CSSPropertyID we
add code to handle custom properties. And since we now explicitly allow a CSSPropertyCustom property
to sneak through, we add guards where necessary if that would otherwise be unexpected, for instance in
WebAnimation::commitStyles() until we handle custom properties there in a future patch.

This change gives us 6 new PASS results as well as modified FAIL results where the KeyframeEffect.setKeyframes()
method is used to set keyframes with custom properties which simply had no effect before.

* LayoutTests/imported/w3c/web-platform-tests/css/css-animations/animation-important-001-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-properties-values-api/at-property-animation-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/Animatable/animate-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/KeyframeEffect/constructor-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/KeyframeEffect/setKeyframes-expected.txt:
* Source/WebCore/animation/CSSPropertyAnimation.cpp:
(WebCore::CSSPropertyAnimation::isPropertyAnimatable):
* Source/WebCore/animation/KeyframeEffect.cpp:
(WebCore::KeyframeEffect::CSSPropertyIDToIDLAttributeName):
(WebCore::IDLAttributeNameToAnimationPropertyName):
(WebCore::processKeyframeLikeObject):
(WebCore::processIterableKeyframes):
(WebCore::processPropertyIndexedKeyframes):
(WebCore::KeyframeEffect::copyPropertiesFromSource):
(WebCore::KeyframeEffect::getKeyframes):
* Source/WebCore/animation/KeyframeEffect.h:
* Source/WebCore/animation/WebAnimation.cpp:
(WebCore::WebAnimation::commitStyles):
* Source/WebCore/bindings/js/JSKeyframeEffectCustom.cpp:
(WebCore::JSKeyframeEffect::getKeyframes):

Canonical link: https://commits.webkit.org/251856@main
  • Loading branch information
graouts committed Jun 26, 2022
1 parent 596af81 commit f5310499353d31e497b22697c75c9c8d9b847bf7
Show file tree
Hide file tree
Showing 10 changed files with 76 additions and 19 deletions.
@@ -5,5 +5,5 @@ FAIL Non-overriden interpolations are observable assert_equals: expected "rgb(0,
FAIL Important rules override animations (::before) assert_equals: expected "rgb(0, 128, 0)" but got "rgb(15, 15, 15)"
PASS Important rules do not override animations on :visited as seen from JS
FAIL Standard property animations appearing via setKeyframes do not override important declarations assert_equals: expected "rgb(255, 255, 255)" but got "rgb(15, 15, 15)"
FAIL Custom property animations appearing via setKeyframes do not override important declarations assert_equals: expected "150px" but got "10px"
FAIL Custom property animations appearing via setKeyframes do not override important declarations assert_equals: expected "150px" but got "200px"

@@ -1,11 +1,11 @@

FAIL @keyframes works with @property assert_equals: expected "150px" but got "200px"
FAIL @keyframes picks up the latest @property in the document assert_equals: expected "rgb(150, 150, 150)" but got "rgb(200, 200, 200)"
FAIL Ongoing animation picks up redeclared custom property assert_equals: expected "0px" but got ""
FAIL Ongoing animation matches new keyframes against the current registration assert_equals: expected "0px" but got ""
FAIL Ongoing animation picks up redeclared intial value assert_equals: expected "200px" but got ""
FAIL Ongoing animation picks up redeclared inherits flag assert_equals: expected "200px" but got "100px"
FAIL Ongoing animation picks up redeclared meaning of 'unset' assert_equals: expected "200px" but got "100px"
FAIL Ongoing animation picks up redeclared custom property assert_equals: expected "0px" but got "rgb(200, 200, 200)"
FAIL Ongoing animation matches new keyframes against the current registration assert_equals: expected "0px" but got "rgb(200, 200, 200)"
FAIL Ongoing animation picks up redeclared intial value assert_equals: expected "200px" but got "400px"
FAIL Ongoing animation picks up redeclared inherits flag assert_equals: expected "200px" but got "400px"
FAIL Ongoing animation picks up redeclared meaning of 'unset' assert_equals: expected "200px" but got "400px"
FAIL Transitioning from initial value assert_equals: expected "rgb(255, 0, 0)" but got ""
FAIL Transitioning from specified value assert_equals: expected "rgb(0, 0, 255)" but got "blue"
FAIL Transition triggered by initial value change assert_equals: expected "100px" but got ""
@@ -20,7 +20,7 @@ PASS Element.animate() accepts a one property one value property-indexed keyfram
PASS Element.animate() accepts a one property one non-array value property-indexed keyframes specification
PASS Element.animate() accepts a one property two value property-indexed keyframes specification where the first value is invalid
PASS Element.animate() accepts a one property two value property-indexed keyframes specification where the second value is invalid
FAIL Element.animate() accepts a property-indexed keyframes specification with a CSS variable as the property assert_equals: number of frames expected 2 but got 0
PASS Element.animate() accepts a property-indexed keyframes specification with a CSS variable as the property
PASS Element.animate() accepts a property-indexed keyframe with a single offset
PASS Element.animate() accepts a property-indexed keyframe with an array of offsets
PASS Element.animate() accepts a property-indexed keyframe with an array of offsets that is too short
@@ -55,7 +55,7 @@ PASS Element.animate() accepts a two property keyframe sequence where one proper
PASS Element.animate() accepts a one property two keyframe sequence that needs to stringify its values
PASS Element.animate() accepts a keyframe sequence with a CSS variable reference
PASS Element.animate() accepts a keyframe sequence with a CSS variable reference in a shorthand property
FAIL Element.animate() accepts a keyframe sequence with a CSS variable as its property assert_equals: properties on ComputedKeyframe #0 should match expected "--custom,composite,computedOffset,easing,offset" but got "composite,computedOffset,easing,offset"
PASS Element.animate() accepts a keyframe sequence with a CSS variable as its property
PASS Element.animate() accepts a keyframe sequence with duplicate values for a given interior offset
PASS Element.animate() accepts a keyframe sequence with duplicate values for offsets 0 and 1
PASS Element.animate() accepts a two property four keyframe sequence
@@ -33,7 +33,7 @@ PASS A KeyframeEffect can be constructed with a one property two value property-
PASS A KeyframeEffect constructed with a one property two value property-indexed keyframes specification where the first value is invalid roundtrips
PASS A KeyframeEffect can be constructed with a one property two value property-indexed keyframes specification where the second value is invalid
PASS A KeyframeEffect constructed with a one property two value property-indexed keyframes specification where the second value is invalid roundtrips
FAIL A KeyframeEffect can be constructed with a property-indexed keyframes specification with a CSS variable as the property assert_equals: number of frames expected 2 but got 0
PASS A KeyframeEffect can be constructed with a property-indexed keyframes specification with a CSS variable as the property
PASS A KeyframeEffect constructed with a property-indexed keyframes specification with a CSS variable as the property roundtrips
PASS A KeyframeEffect can be constructed with a property-indexed keyframe with a single offset
PASS A KeyframeEffect constructed with a property-indexed keyframe with a single offset roundtrips
@@ -103,7 +103,7 @@ PASS A KeyframeEffect can be constructed with a keyframe sequence with a CSS var
PASS A KeyframeEffect constructed with a keyframe sequence with a CSS variable reference roundtrips
PASS A KeyframeEffect can be constructed with a keyframe sequence with a CSS variable reference in a shorthand property
PASS A KeyframeEffect constructed with a keyframe sequence with a CSS variable reference in a shorthand property roundtrips
FAIL A KeyframeEffect can be constructed with a keyframe sequence with a CSS variable as its property assert_equals: properties on ComputedKeyframe #0 should match expected "--custom,composite,computedOffset,easing,offset" but got "composite,computedOffset,easing,offset"
PASS A KeyframeEffect can be constructed with a keyframe sequence with a CSS variable as its property
PASS A KeyframeEffect constructed with a keyframe sequence with a CSS variable as its property roundtrips
PASS A KeyframeEffect can be constructed with a keyframe sequence with duplicate values for a given interior offset
PASS A KeyframeEffect constructed with a keyframe sequence with duplicate values for a given interior offset roundtrips
@@ -14,7 +14,7 @@ PASS Keyframes can be replaced with a one property one value property-indexed ke
PASS Keyframes can be replaced with a one property one non-array value property-indexed keyframes specification
PASS Keyframes can be replaced with a one property two value property-indexed keyframes specification where the first value is invalid
PASS Keyframes can be replaced with a one property two value property-indexed keyframes specification where the second value is invalid
FAIL Keyframes can be replaced with a property-indexed keyframes specification with a CSS variable as the property assert_equals: number of frames expected 2 but got 0
PASS Keyframes can be replaced with a property-indexed keyframes specification with a CSS variable as the property
PASS Keyframes can be replaced with a property-indexed keyframe with a single offset
PASS Keyframes can be replaced with a property-indexed keyframe with an array of offsets
PASS Keyframes can be replaced with a property-indexed keyframe with an array of offsets that is too short
@@ -49,7 +49,7 @@ PASS Keyframes can be replaced with a two property keyframe sequence where one p
PASS Keyframes can be replaced with a one property two keyframe sequence that needs to stringify its values
PASS Keyframes can be replaced with a keyframe sequence with a CSS variable reference
PASS Keyframes can be replaced with a keyframe sequence with a CSS variable reference in a shorthand property
FAIL Keyframes can be replaced with a keyframe sequence with a CSS variable as its property assert_equals: properties on ComputedKeyframe #0 should match expected "--custom,composite,computedOffset,easing,offset" but got "composite,computedOffset,easing,offset"
PASS Keyframes can be replaced with a keyframe sequence with a CSS variable as its property
PASS Keyframes can be replaced with a keyframe sequence with duplicate values for a given interior offset
PASS Keyframes can be replaced with a keyframe sequence with duplicate values for offsets 0 and 1
PASS Keyframes can be replaced with a two property four keyframe sequence
@@ -3697,7 +3697,7 @@ void CSSPropertyAnimation::blendCustomProperty(const AtomString& customProperty,

bool CSSPropertyAnimation::isPropertyAnimatable(CSSPropertyID property)
{
return CSSPropertyAnimationWrapperMap::singleton().wrapperForProperty(property);
return property == CSSPropertyCustom || CSSPropertyAnimationWrapperMap::singleton().wrapperForProperty(property);
}

bool CSSPropertyAnimation::isPropertyAdditiveOrCumulative(CSSPropertyID property)
@@ -32,6 +32,7 @@
#include "CSSKeyframeRule.h"
#include "CSSPropertyAnimation.h"
#include "CSSPropertyNames.h"
#include "CSSPropertyParser.h"
#include "CSSSelector.h"
#include "CSSStyleDeclaration.h"
#include "CSSTimingFunctionValue.h"
@@ -78,7 +79,6 @@ String KeyframeEffect::CSSPropertyIDToIDLAttributeName(CSSPropertyID cssProperty
{
// https://drafts.csswg.org/web-animations-1/#animation-property-name-to-idl-attribute-name
// 1. If property follows the <custom-property-name> production, return property.
// FIXME: We don't handle custom properties yet.

// 2. If property refers to the CSS float property, return the string "cssFloat".
if (cssPropertyId == CSSPropertyFloat)
@@ -96,7 +96,6 @@ static inline CSSPropertyID IDLAttributeNameToAnimationPropertyName(const AtomSt
{
// https://drafts.csswg.org/web-animations-1/#idl-attribute-name-to-animation-property-name
// 1. If attribute conforms to the <custom-property-name> production, return attribute.
// FIXME: We don't handle custom properties yet.

// 2. If attribute is the string "cssFloat", then return an animation property representing the CSS float property.
if (idlAttributeName == "cssFloat"_s)
@@ -109,6 +108,9 @@ static inline CSSPropertyID IDLAttributeNameToAnimationPropertyName(const AtomSt
// 4. Otherwise, return the result of applying the IDL attribute to CSS property algorithm [CSSOM] to attribute.
auto cssPropertyId = CSSStyleDeclaration::getCSSPropertyIDFromJavaScriptPropertyName(idlAttributeName);

if (cssPropertyId == CSSPropertyInvalid && isCustomPropertyName(idlAttributeName))
return CSSPropertyCustom;

// We need to check that converting the property back to IDL form yields the same result such that a property passed
// in non-IDL form is rejected, for instance "font-size".
if (idlAttributeName != KeyframeEffect::CSSPropertyIDToIDLAttributeName(cssPropertyId))
@@ -271,10 +273,14 @@ static inline ExceptionOr<KeyframeEffect::KeyframeLikeObject> processKeyframeLik
RETURN_IF_EXCEPTION(scope, Exception { TypeError });

// 4. Calculate the normalized property name as the result of applying the IDL attribute name to animation property name algorithm to property name.
auto cssPropertyID = IDLAttributeNameToAnimationPropertyName(animationProperties[i].string());
auto propertyName = animationProperties[i].string();
auto cssPropertyID = IDLAttributeNameToAnimationPropertyName(propertyName);

// 5. Add a property to to keyframe output with normalized property name as the property name, and property values as the property value.
keyframeOuput.propertiesAndValues.append({ cssPropertyID, propertyValues });
if (cssPropertyID == CSSPropertyCustom)
keyframeOuput.propertiesAndValues.append({ cssPropertyID, propertyName, propertyValues });
else
keyframeOuput.propertiesAndValues.append({ cssPropertyID, emptyAtom(), propertyValues });
}

// 7. Return keyframe output.
@@ -335,7 +341,11 @@ static inline ExceptionOr<void> processIterableKeyframes(JSGlobalObject& lexical
// there should only ever be a single value for a given property.
ASSERT(propertyAndValue.values.size() == 1);
auto stringValue = propertyAndValue.values[0];
if (keyframeOutput.style->setProperty(cssPropertyId, stringValue, false, parserContext))
if (cssPropertyId == CSSPropertyCustom) {
auto customProperty = propertyAndValue.customProperty;
if (keyframeOutput.style->setCustomProperty(&document, customProperty, stringValue, false, parserContext))
keyframeOutput.customStyleStrings.set(customProperty, stringValue);
} else if (keyframeOutput.style->setProperty(cssPropertyId, stringValue, false, parserContext))
keyframeOutput.styleStrings.set(cssPropertyId, stringValue);
}

@@ -373,7 +383,11 @@ static inline ExceptionOr<void> processPropertyIndexedKeyframes(JSGlobalObject&
// 1. Let k be a new keyframe with a null keyframe offset.
KeyframeEffect::ParsedKeyframe k;
// 2. Add the property-value pair, property name → v, to k.
if (k.style->setProperty(propertyName, v, false, parserContext))
if (propertyName == CSSPropertyCustom) {
auto customProperty = m.customProperty;
if (k.style->setCustomProperty(&document, customProperty, v, false, parserContext))
k.customStyleStrings.set(customProperty, v);
} else if (k.style->setProperty(propertyName, v, false, parserContext))
k.styleStrings.set(propertyName, v);
// 3. Append k to property keyframes.
propertyKeyframes.append(WTFMove(k));
@@ -412,6 +426,11 @@ static inline ExceptionOr<void> processPropertyIndexedKeyframes(JSGlobalObject&
for (auto& [property, value] : keyframe.styleStrings)
previousKeyframe.styleStrings.set(property, value);
}
if (keyframe.customStyleStrings.size()) {
previousKeyframe.style->mergeAndOverrideOnConflict(keyframe.style);
for (auto& [customProperty, value] : keyframe.customStyleStrings)
previousKeyframe.customStyleStrings.set(customProperty, value);
}
// Since we've processed this keyframe, we can remove it and keep i the same
// so that we process the next keyframe in the next loop iteration.
parsedKeyframes.remove(i);
@@ -569,6 +588,7 @@ void KeyframeEffect::copyPropertiesFromSource(Ref<KeyframeEffect>&& source)
parsedKeyframe.offset = sourceParsedKeyframe.offset;
parsedKeyframe.composite = sourceParsedKeyframe.composite;
parsedKeyframe.styleStrings = sourceParsedKeyframe.styleStrings;
parsedKeyframe.customStyleStrings = sourceParsedKeyframe.customStyleStrings;
parsedKeyframe.computedOffset = sourceParsedKeyframe.computedOffset;
parsedKeyframe.timingFunction = sourceParsedKeyframe.timingFunction;
parsedKeyframe.style = sourceParsedKeyframe.style->mutableCopy();
@@ -695,6 +715,32 @@ auto KeyframeEffect::getKeyframes(Document& document) -> Vector<ComputedKeyframe
addPropertyToKeyframe(cssPropertyId);
}

if (m_blendingKeyframesSource != BlendingKeyframesSource::CSSAnimation) {
auto addCustomPropertyToKeyframe = [&](const AtomString& customProperty) {
String styleString = emptyString();
if (keyframeRule) {
if (auto cssValue = keyframeRule->properties().getCustomPropertyCSSValue(customProperty)) {
if (!cssValue->hasVariableReferences())
styleString = keyframeRule->properties().getCustomPropertyValue(customProperty);
}
}
if (styleString.isEmpty()) {
if (auto cssValue = styleProperties->getCustomPropertyCSSValue(customProperty)) {
if (!cssValue->hasVariableReferences())
styleString = styleProperties->getCustomPropertyValue(customProperty);
}
}
if (styleString.isEmpty()) {
if (auto cssValue = computedStyleExtractor.customPropertyValue(customProperty))
styleString = cssValue->cssText();
}
computedKeyframe.customStyleStrings.set(customProperty, styleString);
};

for (auto customProperty : keyframe.customProperties())
addCustomPropertyToKeyframe(customProperty);
}

computedKeyframes.append(WTFMove(computedKeyframe));
}

@@ -40,6 +40,8 @@
#include "Styleable.h"
#include "WebAnimationTypes.h"
#include <wtf/Ref.h>
#include <wtf/text/AtomString.h>
#include <wtf/text/AtomStringHash.h>

namespace WebCore {

@@ -72,6 +74,7 @@ class KeyframeEffect : public AnimationEffect

struct PropertyAndValues {
CSSPropertyID property;
AtomString customProperty;
Vector<String> values;
};

@@ -86,6 +89,7 @@ class KeyframeEffect : public AnimationEffect

struct ComputedKeyframe : BaseComputedKeyframe {
HashMap<CSSPropertyID, String> styleStrings;
HashMap<AtomString, String> customStyleStrings;
};

struct ParsedKeyframe : ComputedKeyframe {
@@ -1482,6 +1482,9 @@ ExceptionOr<void> WebAnimation::commitStyles()
auto properties = effect->animatedProperties();
// 2.5 For each property, property, in targeted properties:
for (auto property : properties) {
// FIXME: handle custom properties (https://bugs.webkit.org/show_bug.cgi?id=242007).
if (property == CSSPropertyCustom)
continue;
// 1. Let partialEffectStack be a copy of the effect stack for property on target.
// 2. If animation's replace state is removed, add all animation effects associated with animation whose effect target is target and which include
// property as a target property to partialEffectStack.
@@ -49,6 +49,10 @@ JSValue JSKeyframeEffect::getKeyframes(JSGlobalObject& lexicalGlobalObject, Call
auto computedKeyframes = wrapped().getKeyframes(downcast<Document>(*context));
auto keyframeObjects = computedKeyframes.map([&](auto& computedKeyframe) -> Strong<JSObject> {
auto keyframeObject = convertDictionaryToJS(lexicalGlobalObject, domGlobalObject, { computedKeyframe });
for (auto& [customProperty, propertyValue] : computedKeyframe.customStyleStrings) {
auto value = toJS<IDLDOMString>(lexicalGlobalObject, propertyValue);
JSObject::defineOwnProperty(keyframeObject, &lexicalGlobalObject, customProperty.impl(), PropertyDescriptor(value, 0), false);
}
for (auto& [propertyID, propertyValue] : computedKeyframe.styleStrings) {
auto propertyName = KeyframeEffect::CSSPropertyIDToIDLAttributeName(propertyID);
auto value = toJS<IDLDOMString>(lexicalGlobalObject, propertyValue);

0 comments on commit f531049

Please sign in to comment.