[PATCH v4 0/2] MR5634: win32u: Ignore WM_MOUSEMOVE when tracking scrollbar and arrow/track is clicked

[Fabian Maurer (＠DarkShadow44)]

This fixes a regression from bb496ea8.
Also closer mimics windows behavior, moving the mouse
should not reset the scrolling timer.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56582

--
  v4: win32u: Only set scroll timer if it's not running

https://gitlab.winehq.org/wine/wine/-/merge_requests/5634

[PATCH v4 1/2] win32u: Refactor out scroll timer handling

[Fabian Maurer]

From: Fabian Maurer

---
 dlls/win32u/scroll.c | 65 +++++++++++++-------------------------------
 1 file changed, 19 insertions(+), 46 deletions(-)

diff --git a/dlls/win32u/scroll.c b/dlls/win32u/scroll.c
index f72ef0c2929..5ea35f1d908 100644
--- a/dlls/win32u/scroll.c
+++ b/dlls/win32u/scroll.c
@@ -512,6 +512,21 @@ static POINT clip_scroll_pos( RECT *rect, POINT pt )
     return pt;
 }

+void update_scroll_timer(HWND hwnd, HWND owner_hwnd, HWND ctl_hwnd, enum SCROLL_HITTEST hittest, UINT msg, UINT msg_send, BOOL vertical )
+{
+    if (hittest == g_tracking_info.hit_test)
+    {
+        if (msg == WM_LBUTTONDOWN || msg == WM_SYSTIMER)
+        {
+            send_message( owner_hwnd, vertical ? WM_VSCROLL : WM_HSCROLL, msg_send, (LPARAM)ctl_hwnd );
+        }
+
+        NtUserSetSystemTimer( hwnd, SCROLL_TIMER,
+                msg == WM_LBUTTONDOWN ? SCROLL_FIRST_DELAY : SCROLL_REPEAT_DELAY );
+    }
+    else NtUserKillSystemTimer( hwnd, SCROLL_TIMER );
+}
+
 /***********************************************************************
  *           handle_scroll_event
  *
@@ -681,33 +696,12 @@ void handle_scroll_event( HWND hwnd, int bar, UINT msg, POINT pt )

case SCROLL_TOP_ARROW:
         draw_scroll_bar( hwnd, hdc, bar, hittest, &g_tracking_info, TRUE, FALSE );
-        if (hittest == g_tracking_info.hit_test)
-        {
-            if ((msg == WM_LBUTTONDOWN) || (msg == WM_SYSTIMER))
-            {
-                send_message( owner_hwnd, vertical ? WM_VSCROLL : WM_HSCROLL,
-                              SB_LINEUP, (LPARAM)ctl_hwnd );
-            }
-
-            NtUserSetSystemTimer( hwnd, SCROLL_TIMER,
-                                  msg == WM_LBUTTONDOWN ? SCROLL_FIRST_DELAY : SCROLL_REPEAT_DELAY );
-        }
-        else NtUserKillSystemTimer( hwnd, SCROLL_TIMER );
+        update_scroll_timer( hwnd, owner_hwnd, ctl_hwnd, hittest, msg, SB_LINEUP, vertical );
         break;

case SCROLL_TOP_RECT:
         draw_scroll_bar( hwnd, hdc, bar, hittest, &g_tracking_info, FALSE, TRUE );
-        if (hittest == g_tracking_info.hit_test)
-        {
-            if (msg == WM_LBUTTONDOWN || msg == WM_SYSTIMER)
-            {
-                send_message( owner_hwnd, vertical ? WM_VSCROLL : WM_HSCROLL,
-                              SB_PAGEUP, (LPARAM)ctl_hwnd );
-            }
-            NtUserSetSystemTimer( hwnd, SCROLL_TIMER,
-                                  msg == WM_LBUTTONDOWN ? SCROLL_FIRST_DELAY : SCROLL_REPEAT_DELAY );
-        }
-        else NtUserKillSystemTimer( hwnd, SCROLL_TIMER );
+        update_scroll_timer( hwnd, owner_hwnd, ctl_hwnd, hittest, msg, SB_PAGEUP, vertical );
         break;

case SCROLL_THUMB:
@@ -755,33 +749,12 @@ void handle_scroll_event( HWND hwnd, int bar, UINT msg, POINT pt )

case SCROLL_BOTTOM_RECT:
         draw_scroll_bar( hwnd, hdc, bar, hittest, &g_tracking_info, FALSE, TRUE );
-        if (hittest == g_tracking_info.hit_test)
-        {
-            if (msg == WM_LBUTTONDOWN || msg == WM_SYSTIMER)
-            {
-                send_message( owner_hwnd, vertical ? WM_VSCROLL : WM_HSCROLL,
-                              SB_PAGEDOWN, (LPARAM)ctl_hwnd );
-            }
-            NtUserSetSystemTimer( hwnd, SCROLL_TIMER,
-                                  msg == WM_LBUTTONDOWN ? SCROLL_FIRST_DELAY : SCROLL_REPEAT_DELAY );
-        }
-        else NtUserKillSystemTimer( hwnd, SCROLL_TIMER );
+        update_scroll_timer( hwnd, owner_hwnd, ctl_hwnd, hittest, msg, SB_PAGEDOWN, vertical );
         break;

case SCROLL_BOTTOM_ARROW:
         draw_scroll_bar( hwnd, hdc, bar, hittest, &g_tracking_info, TRUE, FALSE );
-        if (hittest == g_tracking_info.hit_test)
-        {
-            if (msg == WM_LBUTTONDOWN || msg == WM_SYSTIMER)
-            {
-                send_message( owner_hwnd, vertical ? WM_VSCROLL : WM_HSCROLL,
-                              SB_LINEDOWN, (LPARAM)ctl_hwnd );
-            }
-
-            NtUserSetSystemTimer( hwnd, SCROLL_TIMER,
-                                  msg == WM_LBUTTONDOWN ? SCROLL_FIRST_DELAY : SCROLL_REPEAT_DELAY );
-        }
-        else NtUserKillSystemTimer( hwnd, SCROLL_TIMER );
+        update_scroll_timer( hwnd, owner_hwnd, ctl_hwnd, hittest, msg, SB_LINEDOWN, vertical );
         break;
     }

[PATCH v4 2/2] win32u: Only set scroll timer if it's not running

[Fabian Maurer]

From: Fabian Maurer

This fixes a regression from bb496ea8 where we got a WM_MOUSEMOVE
and this leads to skipping the initial scroll delay.
Also closer mimics windows behavior, moving the mouse
should not reset the scrolling timer.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56582
---
 dlls/win32u/scroll.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/dlls/win32u/scroll.c b/dlls/win32u/scroll.c
index 5ea35f1d908..f34fe80c7e6 100644
--- a/dlls/win32u/scroll.c
+++ b/dlls/win32u/scroll.c
@@ -62,6 +62,9 @@ static struct SCROLL_TRACKING_INFO g_tracking_info;
 /* Is the moving thumb being displayed? */
 static BOOL scroll_moving_thumb = FALSE;

+/* is there currently a running timer for scrolling delay ?*/
+static BOOL scroll_timer_running = FALSE;
+
 /* data for window that has (one or two) scroll bars */
 struct win_scroll_bar_info
 {
@@ -516,15 +519,19 @@ void update_scroll_timer(HWND hwnd, HWND owner_hwnd, HWND ctl_hwnd, enum SCROLL_
 {
     if (hittest == g_tracking_info.hit_test)
     {
-        if (msg == WM_LBUTTONDOWN || msg == WM_SYSTIMER)
+        if (!scroll_timer_running || msg == WM_LBUTTONDOWN || msg == WM_SYSTIMER)
         {
             send_message( owner_hwnd, vertical ? WM_VSCROLL : WM_HSCROLL, msg_send, (LPARAM)ctl_hwnd );
+            scroll_timer_running = TRUE;
+            NtUserSetSystemTimer( hwnd, SCROLL_TIMER,
+                    msg == WM_LBUTTONDOWN ? SCROLL_FIRST_DELAY : SCROLL_REPEAT_DELAY );
         }
-
-        NtUserSetSystemTimer( hwnd, SCROLL_TIMER,
-                msg == WM_LBUTTONDOWN ? SCROLL_FIRST_DELAY : SCROLL_REPEAT_DELAY );
     }
-    else NtUserKillSystemTimer( hwnd, SCROLL_TIMER );
+    else
+    {
+        scroll_timer_running = FALSE;
+        NtUserKillSystemTimer( hwnd, SCROLL_TIMER );
+    }
 }

/***********************************************************************
@@ -789,6 +796,7 @@ void handle_scroll_event( HWND hwnd, int bar, UINT msg, POINT pt )
         /* Terminate tracking */
         g_tracking_info.win = 0;
         scroll_moving_thumb = FALSE;
+        scroll_timer_running = FALSE;
         hittest = SCROLL_NOWHERE;
         draw_scroll_bar( hwnd, hdc, bar, hittest, &g_tracking_info, TRUE, TRUE );
     }

[PATCH v4 2/2] win32u: Only set scroll timer if it's not running

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=145628

Your paranoid android.

=== build (build log) ===

error: patch failed: dlls/win32u/scroll.c:516
Task: Patch failed to apply

=== debian11 (build log) ===

error: patch failed: dlls/win32u/scroll.c:516
Task: Patch failed to apply

=== debian11b (build log) ===

error: patch failed: dlls/win32u/scroll.c:516
Task: Patch failed to apply

[Fabian Maurer (＠DarkShadow44)]

On Thu May 16 15:28:00 2024 +0000, Fabian Maurer wrote:
> changed this line in [version 4 of the diff](/wine/wine/-/merge_requests/5634/diffs?diff_id=114038&start_sha=2065757651acb69e39e395b00171aeede3a3c7f4#1309fd48153be56cdd4228d6e4bc8022a837b2de_529_525)

I pushed a new version merging the two ifs as well, the only change is that now the "send_message" is also called when entering the scrollbar again. Seems reasonable to me, unless you want tests for that?

[PATCH v4 0/2] MR5634: win32u: Ignore WM_MOUSEMOVE when tracking scrollbar and arrow/track is clicked - approved

[Rémi Bernon (＠rbernon)]

This merge request was approved by Rémi Bernon.