From b2b889e1dab36422f3c2399f792c1eae31cf4fc6 Mon Sep 17 00:00:00 2001 From: Callum Law Date: Tue, 18 Nov 2025 22:15:15 +1300 Subject: [PATCH] LibWeb: Ensure registered transitions are reflective of properties Previously we would only update these if: a) We had a cascaded value for `transition-property` b) The source of that cascaded value had changed since we last registered transitions This meant that there were a lot of changes we didn't apply: - Changes exclusively to properties other than `transition-property` (e.g. `transition-duration`, `transition-behavior`, etc) - Removing the `transition-property` property - Updating the `transition-property` property in a way that didn't change it's source (e.g. setting it within inline-style) Unfortunately this does mean that we now register transitions for all properties on most elements since "all" is the initial value for "transition-property" which isn't great for performance, but that can be looked at in later commits. --- Libraries/LibWeb/Animations/Animatable.cpp | 18 ------- Libraries/LibWeb/Animations/Animatable.h | 3 -- Libraries/LibWeb/CSS/ComputedProperties.cpp | 1 - Libraries/LibWeb/CSS/ComputedProperties.h | 5 -- Libraries/LibWeb/CSS/StyleComputer.cpp | 14 ++--- ...-properties-updated-on-duration-change.txt | 1 + ...e-changed-in-same-css-style-properties.txt | 1 + ...rker-supported-properties-in-animation.txt | 8 +-- .../CSSTransition-ready.tentative.txt | 5 +- ...transition-remove-and-change-immediate.txt | 6 +++ ...properties-updated-on-duration-change.html | 15 ++++++ ...-changed-in-same-css-style-properties.html | 15 ++++++ ...ransition-remove-and-change-immediate.html | 51 +++++++++++++++++++ 13 files changed, 98 insertions(+), 45 deletions(-) create mode 100644 Tests/LibWeb/Text/expected/css/transition-properties-updated-on-duration-change.txt create mode 100644 Tests/LibWeb/Text/expected/css/transition-properties-updated-on-property-name-changed-in-same-css-style-properties.txt create mode 100644 Tests/LibWeb/Text/expected/wpt-import/css/css-transitions/transition-remove-and-change-immediate.txt create mode 100644 Tests/LibWeb/Text/input/css/transition-properties-updated-on-duration-change.html create mode 100644 Tests/LibWeb/Text/input/css/transition-properties-updated-on-property-name-changed-in-same-css-style-properties.html create mode 100644 Tests/LibWeb/Text/input/wpt-import/css/css-transitions/transition-remove-and-change-immediate.html diff --git a/Libraries/LibWeb/Animations/Animatable.cpp b/Libraries/LibWeb/Animations/Animatable.cpp index a46accf1c80..01ab7a843d7 100644 --- a/Libraries/LibWeb/Animations/Animatable.cpp +++ b/Libraries/LibWeb/Animations/Animatable.cpp @@ -22,7 +22,6 @@ namespace Web::Animations { struct Animatable::Transition { HashMap transition_attribute_indices; Vector transition_attributes; - GC::Ptr cached_transition_property_source; HashMap> associated_transitions; }; @@ -255,7 +254,6 @@ void Animatable::visit_edges(JS::Cell::Visitor& visitor) for (auto const& transition : impl.transitions) { if (transition) { - visitor.visit(transition->cached_transition_property_source); visitor.visit(transition->associated_transitions); } } @@ -291,22 +289,6 @@ HashMap>* Animatable::css_defined_animations(Optio return impl.css_defined_animations[index]; } -GC::Ptr Animatable::cached_transition_property_source(Optional pseudo_element) const -{ - auto* maybe_transition = ensure_transition(pseudo_element); - if (!maybe_transition) - return {}; - return maybe_transition->cached_transition_property_source; -} - -void Animatable::set_cached_transition_property_source(Optional pseudo_element, GC::Ptr value) -{ - auto* maybe_transition = ensure_transition(pseudo_element); - if (!maybe_transition) - return; - maybe_transition->cached_transition_property_source = value; -} - Animatable::Impl& Animatable::ensure_impl() const { if (!m_impl) diff --git a/Libraries/LibWeb/Animations/Animatable.h b/Libraries/LibWeb/Animations/Animatable.h index 137ed004c81..c70d9e3e282 100644 --- a/Libraries/LibWeb/Animations/Animatable.h +++ b/Libraries/LibWeb/Animations/Animatable.h @@ -58,9 +58,6 @@ public: void add_css_animation(FlyString name, Optional, GC::Ref); void remove_css_animation(FlyString name, Optional); - GC::Ptr cached_transition_property_source(Optional) const; - void set_cached_transition_property_source(Optional, GC::Ptr value); - void add_transitioned_properties(Optional, Vector> properties, CSS::StyleValueVector delays, CSS::StyleValueVector durations, CSS::StyleValueVector timing_functions, CSS::StyleValueVector transition_behaviors); Vector property_ids_with_matching_transition_property_entry(Optional) const; Optional property_transition_attributes(Optional, CSS::PropertyID) const; diff --git a/Libraries/LibWeb/CSS/ComputedProperties.cpp b/Libraries/LibWeb/CSS/ComputedProperties.cpp index 54b2af4cc0c..f5085a775a3 100644 --- a/Libraries/LibWeb/CSS/ComputedProperties.cpp +++ b/Libraries/LibWeb/CSS/ComputedProperties.cpp @@ -55,7 +55,6 @@ ComputedProperties::~ComputedProperties() = default; void ComputedProperties::visit_edges(Visitor& visitor) { Base::visit_edges(visitor); - visitor.visit(m_transition_property_source); } bool ComputedProperties::is_property_important(PropertyID property_id) const diff --git a/Libraries/LibWeb/CSS/ComputedProperties.h b/Libraries/LibWeb/CSS/ComputedProperties.h index 6fd6bd5d130..60d47fb2501 100644 --- a/Libraries/LibWeb/CSS/ComputedProperties.h +++ b/Libraries/LibWeb/CSS/ComputedProperties.h @@ -70,9 +70,6 @@ public: StyleValue const& property(PropertyID, WithAnimationsApplied = WithAnimationsApplied::Yes) const; void revert_property(PropertyID, ComputedProperties const& style_for_revert); - GC::Ptr transition_property_source() const { return m_transition_property_source; } - void set_transition_property_source(GC::Ptr declaration) { m_transition_property_source = declaration; } - Size size_value(PropertyID) const; [[nodiscard]] Variant gap_value(PropertyID) const; Length length(PropertyID) const; @@ -291,8 +288,6 @@ private: Vector shadow(PropertyID, Layout::Node const&) const; Position position_value(PropertyID) const; - GC::Ptr m_transition_property_source; - Array, number_of_longhand_properties> m_property_values; Array m_property_important {}; Array m_property_inherited {}; diff --git a/Libraries/LibWeb/CSS/StyleComputer.cpp b/Libraries/LibWeb/CSS/StyleComputer.cpp index b9b266eb8f5..5b17ae9f8c2 100644 --- a/Libraries/LibWeb/CSS/StyleComputer.cpp +++ b/Libraries/LibWeb/CSS/StyleComputer.cpp @@ -1221,19 +1221,15 @@ static void apply_dimension_attribute(CascadedProperties& cascaded_properties, D static void compute_transitioned_properties(ComputedProperties const& style, DOM::AbstractElement abstract_element) { - auto const source_declaration = style.transition_property_source(); - if (!source_declaration) - return; + // FIXME: For now we don't bother registering transitions on the first computation since they can't run (because + // there is nothing to transition from) but this will change once we implement @starting-style if (!abstract_element.computed_properties()) return; // FIXME: Add transition helpers on AbstractElement. auto& element = abstract_element.element(); auto pseudo_element = abstract_element.pseudo_element(); - if (source_declaration == element.cached_transition_property_source(pseudo_element)) - return; - // Reparse this transition property + element.clear_registered_transitions(pseudo_element); - element.set_cached_transition_property_source(pseudo_element, *source_declaration); auto coordinated_transition_list = style.assemble_coordinated_value_list( PropertyID::TransitionProperty, @@ -2541,10 +2537,6 @@ GC::Ref StyleComputer::compute_properties(DOM::AbstractEleme computed_style->set_property(property_id, value.release_nonnull(), inherited, cascaded_properties.is_property_important(property_id) ? Important::Yes : Important::No); if (animated_value.has_value()) computed_style->set_animated_property(property_id, animated_value.value(), inherited); - - if (property_id == PropertyID::TransitionProperty) { - computed_style->set_transition_property_source(cascaded_properties.property_source(property_id)); - } } // Compute the value of custom properties diff --git a/Tests/LibWeb/Text/expected/css/transition-properties-updated-on-duration-change.txt b/Tests/LibWeb/Text/expected/css/transition-properties-updated-on-duration-change.txt new file mode 100644 index 00000000000..8bd1af11bf2 --- /dev/null +++ b/Tests/LibWeb/Text/expected/css/transition-properties-updated-on-duration-change.txt @@ -0,0 +1 @@ +2000 diff --git a/Tests/LibWeb/Text/expected/css/transition-properties-updated-on-property-name-changed-in-same-css-style-properties.txt b/Tests/LibWeb/Text/expected/css/transition-properties-updated-on-property-name-changed-in-same-css-style-properties.txt new file mode 100644 index 00000000000..d00491fd7e5 --- /dev/null +++ b/Tests/LibWeb/Text/expected/css/transition-properties-updated-on-property-name-changed-in-same-css-style-properties.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/LibWeb/Text/expected/wpt-import/css/css-pseudo/parsing/marker-supported-properties-in-animation.txt b/Tests/LibWeb/Text/expected/wpt-import/css/css-pseudo/parsing/marker-supported-properties-in-animation.txt index eb77fa9df43..a1aae813dcf 100644 --- a/Tests/LibWeb/Text/expected/wpt-import/css/css-pseudo/parsing/marker-supported-properties-in-animation.txt +++ b/Tests/LibWeb/Text/expected/wpt-import/css/css-pseudo/parsing/marker-supported-properties-in-animation.txt @@ -2,8 +2,8 @@ Harness status: OK Found 96 tests -39 Pass -57 Fail +41 Pass +55 Fail Pass Animation of font in ::marker Pass Animation of font-family in ::marker Fail Animation of font-feature-settings in ::marker @@ -59,7 +59,7 @@ Pass Transition of font-kerning in ::marker Fail Transition of font-size in ::marker Fail Transition of font-size-adjust in ::marker Fail Transition of font-stretch in ::marker -Fail Transition of font-style in ::marker +Pass Transition of font-style in ::marker Fail Transition of font-synthesis in ::marker Fail Transition of font-synthesis-small-caps in ::marker Fail Transition of font-synthesis-style in ::marker @@ -91,7 +91,7 @@ Fail Transition of text-emphasis in ::marker Fail Transition of text-emphasis-color in ::marker Fail Transition of text-emphasis-position in ::marker Fail Transition of text-emphasis-style in ::marker -Fail Transition of text-shadow in ::marker +Pass Transition of text-shadow in ::marker Pass Transition of cursor in ::marker Fail Transition of display in ::marker Fail Transition of position in ::marker diff --git a/Tests/LibWeb/Text/expected/wpt-import/css/css-transitions/CSSTransition-ready.tentative.txt b/Tests/LibWeb/Text/expected/wpt-import/css/css-transitions/CSSTransition-ready.tentative.txt index 40233613e6e..645a5346139 100644 --- a/Tests/LibWeb/Text/expected/wpt-import/css/css-transitions/CSSTransition-ready.tentative.txt +++ b/Tests/LibWeb/Text/expected/wpt-import/css/css-transitions/CSSTransition-ready.tentative.txt @@ -2,7 +2,6 @@ Harness status: OK Found 2 tests -1 Pass -1 Fail -Fail ready promise is rejected when a transition is canceled by updating transition-property +2 Pass +Pass ready promise is rejected when a transition is canceled by updating transition-property Pass ready promise is rejected when a transition is canceled by changing the transition property to something not interpolable \ No newline at end of file diff --git a/Tests/LibWeb/Text/expected/wpt-import/css/css-transitions/transition-remove-and-change-immediate.txt b/Tests/LibWeb/Text/expected/wpt-import/css/css-transitions/transition-remove-and-change-immediate.txt new file mode 100644 index 00000000000..a97814d07ba --- /dev/null +++ b/Tests/LibWeb/Text/expected/wpt-import/css/css-transitions/transition-remove-and-change-immediate.txt @@ -0,0 +1,6 @@ +Harness status: OK + +Found 1 tests + +1 Pass +Pass Removing transition and changing width applies change immediately \ No newline at end of file diff --git a/Tests/LibWeb/Text/input/css/transition-properties-updated-on-duration-change.html b/Tests/LibWeb/Text/input/css/transition-properties-updated-on-duration-change.html new file mode 100644 index 00000000000..989646886af --- /dev/null +++ b/Tests/LibWeb/Text/input/css/transition-properties-updated-on-duration-change.html @@ -0,0 +1,15 @@ + +
+ + diff --git a/Tests/LibWeb/Text/input/css/transition-properties-updated-on-property-name-changed-in-same-css-style-properties.html b/Tests/LibWeb/Text/input/css/transition-properties-updated-on-property-name-changed-in-same-css-style-properties.html new file mode 100644 index 00000000000..103beb1a560 --- /dev/null +++ b/Tests/LibWeb/Text/input/css/transition-properties-updated-on-property-name-changed-in-same-css-style-properties.html @@ -0,0 +1,15 @@ + +
+ + diff --git a/Tests/LibWeb/Text/input/wpt-import/css/css-transitions/transition-remove-and-change-immediate.html b/Tests/LibWeb/Text/input/wpt-import/css/css-transitions/transition-remove-and-change-immediate.html new file mode 100644 index 00000000000..2ab927e4d34 --- /dev/null +++ b/Tests/LibWeb/Text/input/wpt-import/css/css-transitions/transition-remove-and-change-immediate.html @@ -0,0 +1,51 @@ + + + + +CSS Transitions Test: Removing transition and changing width applies change immediately + + + + + + + + + +
+ + + + +