From ad3c63684c29bcf26da083d3b65ded8b2dd07006 Mon Sep 17 00:00:00 2001
From: Timothy Flynn <trflynn89@pm.me>
Date: Tue, 11 Apr 2023 19:02:45 -0400
Subject: [PATCH] LibWeb: Restrict toggling video playback to certain areas in
 a video

When the control bar is shown, do not toggle playback when clicking on
the control bar, unless the click target is the playback button. This
will let us implement clicking on the timeline to seek.

Note that this requires caching some layout rects on the video element.
We need to remember where the corresponding layout boxes are, and we
can't cache them on the layout box, as that may be destroyed any time.
---
 .../Libraries/LibWeb/HTML/HTMLVideoElement.h  |  7 +++++
 .../LibWeb/Painting/VideoPaintable.cpp        | 29 +++++++++++++++----
 2 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/Userland/Libraries/LibWeb/HTML/HTMLVideoElement.h b/Userland/Libraries/LibWeb/HTML/HTMLVideoElement.h
index 08305020c5..a581ba2407 100644
--- a/Userland/Libraries/LibWeb/HTML/HTMLVideoElement.h
+++ b/Userland/Libraries/LibWeb/HTML/HTMLVideoElement.h
@@ -36,6 +36,12 @@ public:
     void set_layout_mouse_position(Badge<Painting::VideoPaintable>, Optional<CSSPixelPoint> mouse_position) { m_mouse_position = move(mouse_position); }
     Optional<CSSPixelPoint> const& layout_mouse_position(Badge<Painting::VideoPaintable>) const { return m_mouse_position; }
 
+    struct CachedLayoutBoxes {
+        Optional<CSSPixelRect> control_box_rect;
+        Optional<CSSPixelRect> playback_button_rect;
+    };
+    CachedLayoutBoxes& cached_layout_boxes(Badge<Painting::VideoPaintable>) const { return m_layout_boxes; }
+
 private:
     HTMLVideoElement(DOM::Document&, DOM::QualifiedName);
 
@@ -56,6 +62,7 @@ private:
 
     // Cached state for layout
     Optional<CSSPixelPoint> m_mouse_position;
+    mutable CachedLayoutBoxes m_layout_boxes;
 };
 
 }
diff --git a/Userland/Libraries/LibWeb/Painting/VideoPaintable.cpp b/Userland/Libraries/LibWeb/Painting/VideoPaintable.cpp
index 21017e89db..6a4762a2de 100644
--- a/Userland/Libraries/LibWeb/Painting/VideoPaintable.cpp
+++ b/Userland/Libraries/LibWeb/Painting/VideoPaintable.cpp
@@ -106,6 +106,7 @@ void VideoPaintable::paint_loaded_video_controls(PaintContext& context, HTML::HT
         control_box_rect.take_from_top(control_box_rect.height() - maximum_control_box_size);
 
     context.painter().fill_rect(control_box_rect.to_type<int>(), control_box_color.with_alpha(0xd0));
+    layout_box().dom_node().cached_layout_boxes({}).control_box_rect = context.scale_to_css_rect(control_box_rect);
 
     control_box_rect = paint_control_bar_playback_button(context, video_element, control_box_rect, mouse_position);
     control_box_rect.take_from_left(playback_padding);
@@ -132,6 +133,7 @@ DevicePixelRect VideoPaintable::paint_control_bar_playback_button(PaintContext&
         control_box_rect.top_left(),
         { playback_button_size + playback_button_offset_x * 2, control_box_rect.height() }
     };
+    layout_box().dom_node().cached_layout_boxes({}).playback_button_rect = context.scale_to_css_rect(playback_button_hover_rect);
 
     auto playback_button_is_hovered = mouse_position.has_value() && playback_button_hover_rect.contains(*mouse_position);
     auto playback_button_color = control_button_color(playback_button_is_hovered);
@@ -256,24 +258,39 @@ void VideoPaintable::paint_placeholder_video_controls(PaintContext& context, Dev
     context.painter().draw_triangle(playback_button_location.to_type<int>(), play_button_coordinates, playback_button_color);
 }
 
-VideoPaintable::DispatchEventOfSameName VideoPaintable::handle_mouseup(Badge<EventHandler>, CSSPixelPoint, unsigned button, unsigned)
+VideoPaintable::DispatchEventOfSameName VideoPaintable::handle_mouseup(Badge<EventHandler>, CSSPixelPoint position, unsigned button, unsigned)
 {
     if (button != GUI::MouseButton::Primary)
         return DispatchEventOfSameName::No;
 
     auto& video_element = layout_box().dom_node();
+    auto const& cached_layout_boxes = video_element.cached_layout_boxes({});
 
     // FIXME: This runs from outside the context of any user script, so we do not have a running execution
     //        context. This pushes one to allow the promise creation hook to run.
     auto& environment_settings = document().relevant_settings_object();
     environment_settings.prepare_to_run_script();
 
-    if (video_element.paused())
-        video_element.play().release_value_but_fixme_should_propagate_errors();
-    else
-        video_element.pause().release_value_but_fixme_should_propagate_errors();
+    ScopeGuard guard { [&] { environment_settings.clean_up_after_running_script(); } };
 
-    environment_settings.clean_up_after_running_script();
+    auto toggle_playback = [&]() -> WebIDL::ExceptionOr<void> {
+        if (video_element.paused())
+            TRY(video_element.play());
+        else
+            TRY(video_element.pause());
+        return {};
+    };
+
+    if (cached_layout_boxes.control_box_rect.has_value() && cached_layout_boxes.control_box_rect->contains(position)) {
+        if (cached_layout_boxes.playback_button_rect.has_value() && cached_layout_boxes.playback_button_rect->contains(position)) {
+            toggle_playback().release_value_but_fixme_should_propagate_errors();
+            return DispatchEventOfSameName::Yes;
+        }
+
+        return DispatchEventOfSameName::No;
+    }
+
+    toggle_playback().release_value_but_fixme_should_propagate_errors();
     return DispatchEventOfSameName::Yes;
 }