From 2bd8093449205e9e7a7c8432224cb79d48f5918e Mon Sep 17 00:00:00 2001
From: Matthew Olsson <matthewcolsson@gmail.com>
Date: Mon, 27 May 2024 07:17:29 -0700
Subject: [PATCH] LibWeb: Detect explicit null timeline given in
 KeyframeAnimationOptions

We already do this for the timeline argument in the KeyframeEffect
constructor
---
 .../WebAnimations/animation-properties/timeline.txt         | 2 +-
 .../input/WebAnimations/animation-properties/timeline.html  | 2 +-
 Userland/Libraries/LibWeb/Animations/Animatable.cpp         | 6 +++---
 Userland/Libraries/LibWeb/Animations/Animatable.h           | 2 +-
 Userland/Libraries/LibWeb/Animations/Animation.cpp          | 3 ++-
 5 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/Tests/LibWeb/Text/expected/WebAnimations/animation-properties/timeline.txt b/Tests/LibWeb/Text/expected/WebAnimations/animation-properties/timeline.txt
index 0facc1843cf..24fe9999653 100644
--- a/Tests/LibWeb/Text/expected/WebAnimations/animation-properties/timeline.txt
+++ b/Tests/LibWeb/Text/expected/WebAnimations/animation-properties/timeline.txt
@@ -1,2 +1,2 @@
    Animation's default timeline is the document's timeline: true
-Animation created with null timeline has the document's timeline: true
+Animation created with null timeline has no timeline: true
diff --git a/Tests/LibWeb/Text/input/WebAnimations/animation-properties/timeline.html b/Tests/LibWeb/Text/input/WebAnimations/animation-properties/timeline.html
index 0960c040787..02c5dcb0a91 100644
--- a/Tests/LibWeb/Text/input/WebAnimations/animation-properties/timeline.html
+++ b/Tests/LibWeb/Text/input/WebAnimations/animation-properties/timeline.html
@@ -8,6 +8,6 @@
         println(`Animation's default timeline is the document's timeline: ${animation.timeline === document.timeline}`);
 
         animation = foo.animate({ opacity: [0, 1] }, { timeline: null });
-        println(`Animation created with null timeline has the document's timeline: ${animation.timeline === document.timeline}`);
+        println(`Animation created with null timeline has no timeline: ${animation.timeline === null}`);
     });
 </script>
diff --git a/Userland/Libraries/LibWeb/Animations/Animatable.cpp b/Userland/Libraries/LibWeb/Animations/Animatable.cpp
index 672f9b91454..a9cde62ae16 100644
--- a/Userland/Libraries/LibWeb/Animations/Animatable.cpp
+++ b/Userland/Libraries/LibWeb/Animations/Animatable.cpp
@@ -32,15 +32,15 @@ WebIDL::ExceptionOr<JS::NonnullGCPtr<Animation>> Animatable::animate(Optional<JS
     // 3. If options is a KeyframeAnimationOptions object, let timeline be the timeline member of options or, if
     //    timeline member of options is missing, be the default document timeline of the node document of the element
     //    on which this method was called.
-    JS::GCPtr<AnimationTimeline> timeline;
+    Optional<JS::GCPtr<AnimationTimeline>> timeline;
     if (options.has<KeyframeAnimationOptions>())
         timeline = options.get<KeyframeAnimationOptions>().timeline;
-    if (!timeline)
+    if (!timeline.has_value())
         timeline = target->document().timeline();
 
     // 4. Construct a new Animation object, animation, in the relevant Realm of target by using the same procedure as
     //    the Animation() constructor, passing effect and timeline as arguments of the same name.
-    auto animation = TRY(Animation::construct_impl(realm, effect, timeline));
+    auto animation = TRY(Animation::construct_impl(realm, effect, move(timeline)));
 
     // 5. If options is a KeyframeAnimationOptions object, assign the value of the id member of options to animation’s
     //    id attribute.
diff --git a/Userland/Libraries/LibWeb/Animations/Animatable.h b/Userland/Libraries/LibWeb/Animations/Animatable.h
index b38d02d8926..61b700894d5 100644
--- a/Userland/Libraries/LibWeb/Animations/Animatable.h
+++ b/Userland/Libraries/LibWeb/Animations/Animatable.h
@@ -14,7 +14,7 @@ namespace Web::Animations {
 // https://www.w3.org/TR/web-animations-1/#dictdef-keyframeanimationoptions
 struct KeyframeAnimationOptions : public KeyframeEffectOptions {
     FlyString id { ""_fly_string };
-    JS::GCPtr<AnimationTimeline> timeline;
+    Optional<JS::GCPtr<AnimationTimeline>> timeline;
 };
 
 // https://www.w3.org/TR/web-animations-1/#dictdef-getanimationsoptions
diff --git a/Userland/Libraries/LibWeb/Animations/Animation.cpp b/Userland/Libraries/LibWeb/Animations/Animation.cpp
index f4fbac63707..c64a6615972 100644
--- a/Userland/Libraries/LibWeb/Animations/Animation.cpp
+++ b/Userland/Libraries/LibWeb/Animations/Animation.cpp
@@ -668,7 +668,8 @@ WebIDL::ExceptionOr<void> Animation::play_an_animation(AutoRewind auto_rewind)
     //     If a user agent determines that animation is immediately ready, it may schedule the above task as a microtask
     //     such that it runs at the next microtask checkpoint, but it must not perform the task synchronously.
     m_pending_play_task = TaskState::Scheduled;
-    m_saved_play_time = m_timeline->current_time().value();
+    if (m_timeline)
+        m_saved_play_time = m_timeline->current_time().value();
 
     // 13. Run the procedure to update an animation’s finished state for animation with the did seek flag set to false,
     //     and the synchronously notify flag set to false.