From 6f8d19cd85068115052db71b6702482d87e28d51 Mon Sep 17 00:00:00 2001 From: Antoine Quint Date: Sun, 20 Nov 2022 07:59:06 -0800 Subject: [PATCH] [Web Animations] Make WPT test at animation-model/keyframe-effects/effect-value-context.html pass reliably https://bugs.webkit.org/show_bug.cgi?id=186490 rdar://41000137 Reviewed by Antti Koivisto. We added support for recomputing keyframes when the computed font-size changes in bug 237357, allowing any keyframe values depending on font-size to produce the expected values. However, we have a bug when font-size changes on a parent element where the logic under ComputedStyleExtractor::propertyValue() will not invalidate style because the code does not think the style needs updating. So we update hasValidStyleForProperty() to account for animations affecting the property for which the computed style is requested. * LayoutTests/imported/w3c/web-platform-tests/web-animations/animation-model/keyframe-effects/effect-value-context-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/web-animations/animation-model/keyframe-effects/effect-value-context-filling-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/web-animations/responsive/boxShadow-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/web-animations/responsive/shapeOutside-expected.txt: * Source/WebCore/animation/KeyframeEffectStack.cpp: (WebCore::KeyframeEffectStack::containsProperty const): * Source/WebCore/animation/KeyframeEffectStack.h: * Source/WebCore/css/ComputedStyleExtractor.cpp: (WebCore::hasValidStyleForProperty): Canonical link: https://commits.webkit.org/256889@main --- .../keyframe-effects/effect-value-context-expected.txt | 2 +- .../effect-value-context-filling-expected.txt | 6 +++--- .../web-animations/responsive/boxShadow-expected.txt | 2 +- .../web-animations/responsive/shapeOutside-expected.txt | 2 +- Source/WebCore/animation/KeyframeEffectStack.cpp | 7 +++++++ Source/WebCore/animation/KeyframeEffectStack.h | 1 + Source/WebCore/css/ComputedStyleExtractor.cpp | 5 +++++ 7 files changed, 19 insertions(+), 6 deletions(-) diff --git a/LayoutTests/imported/w3c/web-platform-tests/web-animations/animation-model/keyframe-effects/effect-value-context-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/web-animations/animation-model/keyframe-effects/effect-value-context-expected.txt index 93fa351ffe81..dd6eb0240848 100644 --- a/LayoutTests/imported/w3c/web-platform-tests/web-animations/animation-model/keyframe-effects/effect-value-context-expected.txt +++ b/LayoutTests/imported/w3c/web-platform-tests/web-animations/animation-model/keyframe-effects/effect-value-context-expected.txt @@ -1,6 +1,6 @@ PASS Effect values reflect changes to font-size on element -FAIL Effect values reflect changes to font-size on parent element assert_equals: Effect value after updating font-size on parent element expected "300px" but got "150px" +PASS Effect values reflect changes to font-size on parent element PASS Effect values reflect changes to font-size when computed style is not immediately flushed PASS Effect values reflect changes to font-size from reparenting PASS Effect values reflect changes to target element diff --git a/LayoutTests/imported/w3c/web-platform-tests/web-animations/animation-model/keyframe-effects/effect-value-context-filling-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/web-animations/animation-model/keyframe-effects/effect-value-context-filling-expected.txt index 924f03fedf8e..0ab91d831d7e 100644 --- a/LayoutTests/imported/w3c/web-platform-tests/web-animations/animation-model/keyframe-effects/effect-value-context-filling-expected.txt +++ b/LayoutTests/imported/w3c/web-platform-tests/web-animations/animation-model/keyframe-effects/effect-value-context-filling-expected.txt @@ -1,6 +1,6 @@ PASS Filling effect values reflect changes to font-size on element -FAIL Filling effect values reflect changes to font-size on parent element assert_equals: Effect value after updating font-size on parent element expected "400px" but got "200px" +PASS Filling effect values reflect changes to font-size on parent element FAIL Filling effect values reflect changes to variables on element assert_equals: Effect value after updating variable expected "200px" but got "100px" FAIL Filling effect values reflect changes to variables on parent element assert_equals: Effect value after updating variable expected "400px" but got "200px" PASS Filling effect values reflect changes to the the animation's keyframes @@ -9,8 +9,8 @@ PASS Filling effect values reflect changes to the the animation's iteration comp PASS Filling effect values reflect changes to the base value when using additive animation PASS Filling effect values reflect changes to the base value when using additive animation on a single keyframe PASS Filling effect values reflect changes to the base value when using the fill value is an implicit keyframe -FAIL Filling effect values reflect changes to the base value via a parent element assert_equals: Effect value after updating font-size on parent expected "400px" but got "300px" +PASS Filling effect values reflect changes to the base value via a parent element PASS Filling effect values reflect changes to underlying animations -FAIL Filling effect values reflect changes to underlying animations via a a parent element assert_equals: Effect value after updating parent font-size expected "400px" but got "300px" +PASS Filling effect values reflect changes to underlying animations via a a parent element PASS Filling effect values reflect changes to underlying animations made by directly changing the keyframes diff --git a/LayoutTests/imported/w3c/web-platform-tests/web-animations/responsive/boxShadow-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/web-animations/responsive/boxShadow-expected.txt index d18c975199c9..b5bd04a100f3 100644 --- a/LayoutTests/imported/w3c/web-platform-tests/web-animations/responsive/boxShadow-expected.txt +++ b/LayoutTests/imported/w3c/web-platform-tests/web-animations/responsive/boxShadow-expected.txt @@ -1,3 +1,3 @@ -FAIL boxShadow responsive to style changes assert_not_equals: got disallowed value "rgb(255, 0, 0) 100px 100px 0px 0px" +PASS boxShadow responsive to style changes diff --git a/LayoutTests/imported/w3c/web-platform-tests/web-animations/responsive/shapeOutside-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/web-animations/responsive/shapeOutside-expected.txt index 3cb9c7e7499d..7398a19edd84 100644 --- a/LayoutTests/imported/w3c/web-platform-tests/web-animations/responsive/shapeOutside-expected.txt +++ b/LayoutTests/imported/w3c/web-platform-tests/web-animations/responsive/shapeOutside-expected.txt @@ -1,4 +1,4 @@ -FAIL shapeOutside responsive to style changes assert_not_equals: got disallowed value "circle(100px at 50% 50%)" +PASS shapeOutside responsive to style changes FAIL shapeOutside responsive to inherited shapeOutside changes assert_equals: expected "circle(200px at 50% 50%)" but got "circle(150px at 50% 50%)" diff --git a/Source/WebCore/animation/KeyframeEffectStack.cpp b/Source/WebCore/animation/KeyframeEffectStack.cpp index ce9f2ed550ae..428f12517383 100644 --- a/Source/WebCore/animation/KeyframeEffectStack.cpp +++ b/Source/WebCore/animation/KeyframeEffectStack.cpp @@ -77,6 +77,13 @@ bool KeyframeEffectStack::hasMatchingEffect(const Function> sortedEffects(); const AnimationList* cssAnimationList() const { return m_cssAnimationList.get(); } void setCSSAnimationList(RefPtr&&); + bool containsProperty(CSSPropertyID) const; bool isCurrentlyAffectingProperty(CSSPropertyID) const; bool requiresPseudoElement() const; OptionSet applyKeyframeEffects(RenderStyle& targetStyle, const RenderStyle* previousLastStyleChangeEventStyle, const Style::ResolutionContext&); diff --git a/Source/WebCore/css/ComputedStyleExtractor.cpp b/Source/WebCore/css/ComputedStyleExtractor.cpp index 6837c89a1df0..a8225d1a97a5 100644 --- a/Source/WebCore/css/ComputedStyleExtractor.cpp +++ b/Source/WebCore/css/ComputedStyleExtractor.cpp @@ -2484,6 +2484,11 @@ static inline bool hasValidStyleForProperty(Element& element, CSSPropertyID prop if (!element.document().childNeedsStyleRecalc()) return true; + if (auto* keyframeEffectStack = Styleable(element, PseudoId::None).keyframeEffectStack()) { + if (keyframeEffectStack->containsProperty(propertyID)) + return false; + } + auto isQueryContainer = [&](Element& element) { auto* style = element.renderStyle(); return style && style->containerType() != ContainerType::Normal;