bug#68796: xterm.c: Convert mouse-4/5/6/7 to wheel-up/down/left/right

[Po Lu]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>From 60e6903ae12546db93f149b1f184ffc7ccdc30c2 Mon Sep 17 00:00:00 2001
> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Mon, 29 Jan 2024 09:35:09 -0500
> Subject: [PATCH] xterm.c: Map old-style wheel buttons to actual wheel events
>
> Change the handling of the old X11 convention to use mouse-4/5/6/7
> events to represent wheel events: instead of asking downstream
> packages to use the `mouse-wheel-*-event` variables to know which events
> represent wheel events, use those variables in `xterm.c` to directly
> convert those events into the standard `wheel-up/down/left/right` events
> used everywhere else.
>
> This simplifies the work of packages which can now just bind their
> commands to `wheel-up/down/left/right`.

I'm still opposed to this for reasons I will try to communicate below.
First, there are several logic errors in this patch:

> +  /* Convert pre-XInput2 wheel events represented as mouse-clicks.  */
> +#ifdef HAVE_XINPUT2
> +  if (!dpyinfo->supports_xi2)
> +#endif

dpyinfo->supports_xi2 is no guarantee that exclusively input extension
events will be received, but only indicates that the X display supports
the input extension.  There are several scenarios where it may be true
while core events are still delivered to windows, such as a combination
of GTK 3 and GDK_CORE_DEVICE_EVENTS=1's being enabled.  The correct test
of event type is whether x_construct_mouse_click is being called from
the XI_ButtonPress or XI_ButtonRelease label in handle_one_xevent.

Incidentally, servers with the input extension always assign buttons 4
through 7 to wheel movement.

> +      Lisp_Object base = base_mouse_symbol (result->code);
> +      int wheel
> +       /* BEWARE: `mouse-wheel-UP-event' corresponds to
> +          `wheel-DOWN' events and vice versa!!  */
> +        = BASE_EQ   (base, find_symbol_value (Qmouse_wheel_up_event))    ? 0
> +          : BASE_EQ (base, find_symbol_value (Qmouse_wheel_down_event))  ? 1
> +          : BASE_EQ (base, find_symbol_value (Qmouse_wheel_left_event))  ? 2
> +          : BASE_EQ (base, find_symbol_value (Qmouse_wheel_right_event)) ? 3
> +          : -1;
> +      if (wheel >= 0)
> +        {
> +          result->kind = (event->type != ButtonRelease ? NO_EVENT
> +                       : wheel & 2 ? HORIZ_WHEEL_EVENT : WHEEL_EVENT);
> +          result->code = 0;         /* Not used.  */
> +          result->modifiers &= ~(up_modifier || down_modifier);
> +          result->modifiers |= wheel & 1 ? up_modifier : down_modifier;
> +        }
> +    }

Isn't this insufficient to completely supersede mouse events by wheel
events, considering that such events will still be delivered as mouse
button events to tab bars?

Lastly, plenty of old code that binds just the button events will be
broken, and the code that keeps track of button-down and up events in
read_key_sequence and friends will cease applying to X wheel events.

This area has given me a lot of grief over the years (as it has given
you), so I'm reluctant to see any change there, as no matter how well
the argument in favor of its safety is posed, some unforeseen problem
invariably emerges afterwards.