bug#66655: 29.1; Clicking buttons sometimes doesn't work

[Stefan Monnier]

> With the current code, if I move the mouse between the events, but the
> coordinates differ by less than double-click-fuzz, we will not
> generate a drag event.  If the mouse-moved flag overrides that, we
> will generate a drag event where previously we didn't, isn't that so?

Good point.

The patch below should fix this problem (as well as the problem that we
only considered the mouse to move when we emitted a `mouse-movement`
event).


        Stefan

diff --git a/src/indent.c b/src/indent.c
index 7d34d3638d9..ca8d59ff8f0 100644
--- a/src/indent.c
+++ b/src/indent.c
@@ -2031,7 +2031,7 @@ vmotion (ptrdiff_t from, ptrdiff_t from_byte,
 }
 
 /* Return the width taken by line-number display in window W.  */
-void
+static void
 line_number_display_width (struct window *w, int *width, int *pixel_width)
 {
   if (NILP (Vdisplay_line_numbers))
diff --git a/src/keyboard.c b/src/keyboard.c
index dc2f78a7c26..56672b7d453 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -5530,10 +5530,9 @@ #define ISO_FUNCTION_KEY_OFFSET 0xfe00
 /* A cons recording the original frame-relative x and y coordinates of
    the down mouse event.  */
 static Lisp_Object frame_relative_event_pos;
-
-/* The line-number display width, in columns, at the time of most
-   recent down mouse event.  */
-static int down_mouse_line_number_width;
+/* True iff the mouse has stayed within `double_click_fuzz` of
+   `frame_relative_event_pos`.  */
+static bool mouse_has_moved;
 
 /* Information about the most recent up-going button event:  Which
    button, what location, and what time.  */
@@ -5931,55 +5930,19 @@ coords_in_tab_bar_window (struct frame *f, int x, int y)
 
 #endif /* HAVE_WINDOW_SYSTEM */
 
-static void
-save_line_number_display_width (struct input_event *event)
-{
-  struct window *w;
-  int pixel_width;
-
-  if (WINDOWP (event->frame_or_window))
-    w = XWINDOW (event->frame_or_window);
-  else if (FRAMEP (event->frame_or_window))
-    w = XWINDOW (XFRAME (event->frame_or_window)->selected_window);
-  else
-    w = XWINDOW (selected_window);
-  line_number_display_width (w, &down_mouse_line_number_width, &pixel_width);
-}
-
-/* Return non-zero if the change of position from START_POS to END_POS
-   is likely to be the effect of horizontal scrolling due to a change
-   in line-number width produced by redisplay between two mouse
-   events, like mouse-down followed by mouse-up, at those positions.
-   This is used to decide whether to converts mouse-down followed by
-   mouse-up event into a mouse-drag event.  */
-static bool
-line_number_mode_hscroll (Lisp_Object start_pos, Lisp_Object end_pos)
+/*  */
+void
+check_mouse_click_fuzz (EMACS_INT x, EMACS_INT y)
 {
-  if (!EQ (Fcar (start_pos), Fcar (end_pos)) /* different window */
-      || list_length (start_pos) < 7        /* no COL/ROW info */
-      || list_length (end_pos) < 7)
-    return false;
+  EMACS_INT xdiff = x - XFIXNUM (XCAR (frame_relative_event_pos));
+  EMACS_INT ydiff = y - XFIXNUM (XCDR (frame_relative_event_pos));
 
-  Lisp_Object start_col_row = Fnth (make_fixnum (6), start_pos);
-  Lisp_Object end_col_row = Fnth (make_fixnum (6), end_pos);
-  Lisp_Object window = Fcar (end_pos);
-  int col_width, pixel_width;
-  Lisp_Object start_col, end_col;
-  struct window *w;
-  if (!WINDOW_VALID_P (window))
-    {
-      if (WINDOW_LIVE_P (window))
-       window = XFRAME (window)->selected_window;
-      else
-       window = selected_window;
-    }
-  w = XWINDOW (window);
-  line_number_display_width (w, &col_width, &pixel_width);
-  start_col = Fcar (start_col_row);
-  end_col = Fcar (end_col_row);
-  return EQ (start_col, end_col)
-        && down_mouse_line_number_width >= 0
-        && col_width != down_mouse_line_number_width;
+  if (0 < double_click_fuzz
+      && - double_click_fuzz < xdiff
+      && xdiff < double_click_fuzz
+      && - double_click_fuzz < ydiff
+      && ydiff < double_click_fuzz)
+    mouse_has_moved = true;
 }
 
 /* Given a struct input_event, build the lisp event which represents
@@ -6383,9 +6346,8 @@ make_lispy_event (struct input_event *event)
            button_down_time = event->timestamp;
            *start_pos_ptr = Fcopy_alist (position);
            frame_relative_event_pos = Fcons (event->x, event->y);
+           mouse_has_moved = false;
            ignore_mouse_drag_p = false;
-           /* Squirrel away the line-number width, if any.  */
-           save_line_number_display_width (event);
          }
 
        /* Now we're releasing a button - check the coordinates to
@@ -6407,42 +6369,16 @@ make_lispy_event (struct input_event *event)
              ignore_mouse_drag_p = false;
            else
              {
-               intmax_t xdiff = double_click_fuzz, ydiff = double_click_fuzz;
-
-               xdiff = XFIXNUM (event->x)
-                 - XFIXNUM (XCAR (frame_relative_event_pos));
-               ydiff = XFIXNUM (event->y)
-                 - XFIXNUM (XCDR (frame_relative_event_pos));
-
-               if (! (0 < double_click_fuzz
-                      && - double_click_fuzz < xdiff
-                      && xdiff < double_click_fuzz
-                      && - double_click_fuzz < ydiff
-                      && ydiff < double_click_fuzz
-                      /* Maybe the mouse has moved a lot, caused scrolling, and
-                         eventually ended up at the same screen position (but
-                         not buffer position) in which case it is a drag, not
-                         a click.  */
-                      /* FIXME: OTOH if the buffer position has changed
-                         because of a timer or process filter rather than
-                         because of mouse movement, it should be considered as
-                         a click.  But mouse-drag-region completely ignores
-                         this case and it hasn't caused any real problem, so
-                         it's probably OK to ignore it as well.  */
-                      && (EQ (Fcar (Fcdr (start_pos)),
-                              Fcar (Fcdr (position))) /* Same buffer pos */
-                          /* Redisplay hscrolled text between down- and
-                              up-events due to display-line-numbers-mode.  */
-                          || line_number_mode_hscroll (start_pos, position)
-                          || !EQ (Fcar (start_pos),
-                                  Fcar (position))))) /* Different window */
-
+               /* Check if this position is within the fuzz.  */
+               check_mouse_click_fuzz (XFIXNUM (event->x), XFIXNUM (event->y));
+               if (mouse_has_moved
+                   && (!EQ (Fcar (Fcdr (start_pos)),
+                            Fcar (Fcdr (position)))) /* Different buffer pos */
+                   && EQ (Fcar (start_pos), Fcar (position))) /* Same window */
                  {
                    /* Mouse has moved enough.  */
                    button_down_time = 0;
                    click_or_drag_modifier = drag_modifier;
-                   /* Reset the value for future clicks.  */
-                   down_mouse_line_number_width = -1;
                  }
                else if (((!EQ (Fcar (start_pos), Fcar (position)))
                          || (!EQ (Fcar (Fcdr (start_pos)),
@@ -7084,6 +7020,7 @@ make_lispy_movement (struct frame *frame, Lisp_Object 
bar_window, enum scroll_ba
     {
       Lisp_Object position;
       position = make_lispy_position (frame, x, y, t);
+      mouse_has_moved = true;
       return list2 (Qmouse_movement, position);
     }
 }
diff --git a/src/keyboard.h b/src/keyboard.h
index 9f6e65f9a09..29591d1c39f 100644
--- a/src/keyboard.h
+++ b/src/keyboard.h
@@ -507,6 +507,7 @@ kbd_buffer_store_event_hold (struct input_event *event,
                             Lisp_Object);
 extern void gen_help_event (Lisp_Object, Lisp_Object, Lisp_Object,
                             Lisp_Object, ptrdiff_t);
+extern void check_mouse_click_fuzz (EMACS_INT x, EMACS_INT y);
 extern void kbd_buffer_store_help_event (Lisp_Object, Lisp_Object);
 extern Lisp_Object menu_item_eval_property (Lisp_Object);
 extern bool kbd_buffer_events_waiting (void);
diff --git a/src/lisp.h b/src/lisp.h
index df6cf1df544..39aa51531fe 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -4883,7 +4883,6 @@ fast_c_string_match_ignore_case (Lisp_Object regexp,
 
 /* Defined in indent.c.  */
 extern ptrdiff_t current_column (void);
-extern void line_number_display_width (struct window *, int *, int *);
 extern void invalidate_current_column (void);
 extern bool indented_beyond_p (ptrdiff_t, ptrdiff_t, EMACS_INT);
 extern void syms_of_indent (void);
diff --git a/src/xdisp.c b/src/xdisp.c
index b9009df5df9..9eff1f6dd91 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -35414,6 +35414,8 @@ note_mouse_highlight (struct frame *f, int x, int y)
   Lisp_Object pointer = Qnil;  /* Takes precedence over cursor!  */
   struct buffer *b;
 
+  check_mouse_click_fuzz (x, y);
+
   /* When a menu is active, don't highlight because this looks odd.  */
 #if defined (HAVE_X_WINDOWS) || defined (HAVE_NS) || defined (MSDOS) \
   || defined (HAVE_ANDROID)