From cf54ba01ac9fc6c40f62fb2e88a7ee1e6a5fe14b Mon Sep 17 00:00:00 2001
From: Matthew Olsson <matthewcolsson@gmail.com>
Date: Mon, 25 Mar 2024 17:40:38 -0700
Subject: [PATCH] LibWeb: Be more strict about invoking .play() on element
 animations

We don't want to always invoke the .play() method if an animation is
relevant and had a play state of running, as the play state is
essentially just "paused" or "not paused". We replace that relevancy
check with the inverse of the pause check so we don't constantly call
.play() on animations that are finished.

Also duplicates and moves the temporary context into both blocks, since
most of the time neither condition will be true.
---
 Userland/Libraries/LibWeb/CSS/StyleComputer.cpp | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp
index fc78d42668a..0496ee7d5c4 100644
--- a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp
+++ b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp
@@ -1410,7 +1410,11 @@ void StyleComputer::collect_animation_into(DOM::Element& element, Optional<CSS::
     }
 }
 
-static void apply_animation_properties(DOM::Document& document, StyleProperties& style, Animations::Animation& animation)
+enum class AnimationIsNew {
+    Yes,
+    No,
+};
+static void apply_animation_properties(DOM::Document& document, StyleProperties& style, Animations::Animation& animation, AnimationIsNew animation_is_new)
 {
     auto& effect = verify_cast<Animations::KeyframeEffect>(*animation.effect());
 
@@ -1468,10 +1472,12 @@ static void apply_animation_properties(DOM::Document& document, StyleProperties&
     effect.set_fill_mode(Animations::css_fill_mode_to_bindings_fill_mode(fill_mode));
     effect.set_playback_direction(Animations::css_animation_direction_to_bindings_playback_direction(direction));
 
-    HTML::TemporaryExecutionContext context(document.relevant_settings_object());
-    if (play_state == CSS::AnimationPlayState::Running && !animation.is_relevant()) {
+    // If this is a new animation, we always want to play it
+    if (play_state == CSS::AnimationPlayState::Running && (animation_is_new == AnimationIsNew::Yes || animation.play_state() == Bindings::AnimationPlayState::Paused)) {
+        HTML::TemporaryExecutionContext context(document.relevant_settings_object());
         animation.play().release_value_but_fixme_should_propagate_errors();
     } else if (play_state == CSS::AnimationPlayState::Paused && animation.play_state() != Bindings::AnimationPlayState::Paused) {
+        HTML::TemporaryExecutionContext context(document.relevant_settings_object());
         animation.pause().release_value_but_fixme_should_propagate_errors();
     }
 }
@@ -1553,7 +1559,7 @@ void StyleComputer::compute_cascaded_values(StyleProperties& style, DOM::Element
                 animation->set_timeline(m_document->timeline());
                 animation->set_owning_element(element);
                 animation->set_effect(effect);
-                apply_animation_properties(m_document, style, animation);
+                apply_animation_properties(m_document, style, animation, AnimationIsNew::Yes);
                 if (pseudo_element.has_value())
                     effect->set_pseudo_element(Selector::PseudoElement { pseudo_element.value() });
 
@@ -1565,7 +1571,7 @@ void StyleComputer::compute_cascaded_values(StyleProperties& style, DOM::Element
                 element.set_cached_animation_name_animation(animation);
             } else {
                 // The animation hasn't changed, but some properties of the animation may have
-                apply_animation_properties(m_document, style, *element.cached_animation_name_animation());
+                apply_animation_properties(m_document, style, *element.cached_animation_name_animation(), AnimationIsNew::No);
             }
         }
     } else {