From 0a462dac14a3e05324e4aa4f69c5d4492a282f3c Mon Sep 17 00:00:00 2001
From: Nico Weber <thakis@chromium.org>
Date: Tue, 25 Aug 2020 11:04:53 -0400
Subject: [PATCH] LibGUI: Keep scrollbar timer active while mouse is down

Rather than disable and re-enable the timer, always keep it active
and make it do collision checks to decide if it should have an effect.

This is because set_automatic_scrolling_active(true) calls the
timeout callback immediately before starting the timer, and
when clicking the gutter this callback could disable the timer
again (if the first page scroll put the scrubber under the cursor).

Intead of making set_automatic_scrolling_active() work when it's
called reentrantly (which is easy: just swap the order of
on_automatic_scrolling_timer_fired() and timer->start() so that
on_automatic_scrolling_timer_fired() can immediately stop the
timer again, but it's confusing), make the timer check if it
should do anything.

This is keyed off m_last_mouse_position instead of
m_hovered_component because m_hovered_component is a visual state
and we arguably shouldn't modify it while the left mouse button
is down (as it indicated what part is activated on click).
---
 Libraries/LibGUI/ScrollBar.cpp | 29 +++++++++++------------------
 Libraries/LibGUI/ScrollBar.h   |  1 +
 2 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/Libraries/LibGUI/ScrollBar.cpp b/Libraries/LibGUI/ScrollBar.cpp
index 9771ce7eb5..d33be96876 100644
--- a/Libraries/LibGUI/ScrollBar.cpp
+++ b/Libraries/LibGUI/ScrollBar.cpp
@@ -261,11 +261,11 @@ void ScrollBar::paint_event(PaintEvent& event)
 
 void ScrollBar::on_automatic_scrolling_timer_fired()
 {
-    if (m_automatic_scrolling_kind == AutomaticScrollingKind::DecrementButton) {
+    if (m_automatic_scrolling_kind == AutomaticScrollingKind::DecrementButton && component_at_position(m_last_mouse_position) == Component::DecrementButton) {
         set_value(value() - m_step);
         return;
     }
-    if (m_automatic_scrolling_kind == AutomaticScrollingKind::IncrementButton) {
+    if (m_automatic_scrolling_kind == AutomaticScrollingKind::IncrementButton && component_at_position(m_last_mouse_position) == Component::IncrementButton) {
         set_value(value() + m_step);
         return;
     }
@@ -278,7 +278,8 @@ void ScrollBar::mousedown_event(MouseEvent& event)
     if (!has_scrubber())
         return;
 
-    Component clicked_component = component_at_position(event.position());
+    m_last_mouse_position = event.position();
+    Component clicked_component = component_at_position(m_last_mouse_position);
 
     if (clicked_component == Component::DecrementButton) {
         set_automatic_scrolling_active(true, AutomaticScrollingKind::DecrementButton);
@@ -377,23 +378,15 @@ ScrollBar::Component ScrollBar::component_at_position(const Gfx::IntPoint& posit
     return Component::Invalid;
 }
 
-void ScrollBar::update_hovered_component(const Gfx::IntPoint& position)
-{
-    auto old_hovered_component = m_hovered_component;
-    m_hovered_component = component_at_position(event.position());
-    if (old_hovered_component != m_hovered_component) {
-        update();
-
-        if (m_automatic_scrolling_kind == AutomaticScrollingKind::DecrementButton)
-            set_automatic_scrolling_active(m_hovered_component == Component::DecrementButton, m_automatic_scrolling_kind);
-        else if (m_automatic_scrolling_kind == AutomaticScrollingKind::IncrementButton)
-            set_automatic_scrolling_active(m_hovered_component == Component::IncrementButton, m_automatic_scrolling_kind);
-    }
-}
-
 void ScrollBar::mousemove_event(MouseEvent& event)
 {
-    update_hovered_component(event.position());
+    m_last_mouse_position = event.position();
+
+    auto old_hovered_component = m_hovered_component;
+    m_hovered_component = component_at_position(m_last_mouse_position);
+    if (old_hovered_component != m_hovered_component) {
+        update();
+    }
     if (!m_scrubbing)
         return;
     float delta = orientation() == Orientation::Vertical ? (event.y() - m_scrub_origin.y()) : (event.x() - m_scrub_origin.x());
diff --git a/Libraries/LibGUI/ScrollBar.h b/Libraries/LibGUI/ScrollBar.h
index 2c40b5fb60..d885f494ae 100644
--- a/Libraries/LibGUI/ScrollBar.h
+++ b/Libraries/LibGUI/ScrollBar.h
@@ -118,6 +118,7 @@ private:
 
     Gfx::Orientation m_orientation { Gfx::Orientation::Vertical };
     Component m_hovered_component { Component::Invalid };
+    Gfx::IntPoint m_last_mouse_position;
     bool m_scrubber_in_use { false };
 
     AutomaticScrollingKind m_automatic_scrolling_kind { AutomaticScrollingKind::None };