Package: emacs;
Reported by: Stefan Monnier <monnier <at> iro.umontreal.ca>
Date: Mon, 29 Jan 2024 14:46:01 UTC
Severity: wishlist
Tags: patch
Done: Stefan Monnier <monnier <at> iro.umontreal.ca>
Bug is archived. No further changes may be made.
View this message in rfc822 format
From: Stefan Monnier <monnier <at> iro.umontreal.ca> To: Po Lu <luangruo <at> yahoo.com> Cc: 68796 <at> debbugs.gnu.org Subject: bug#68796: xterm.c: Convert mouse-4/5/6/7 to wheel-up/down/left/right Date: Mon, 29 Jan 2024 22:32:29 -0500
[Message part 1 (text/plain, inline)]
>> + /* 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.
Hmm... so IIUC you're saying that even when `dpyinfo->supports_xi2` is
true, we may receive `mouse-4/5/6/7` events for wheel motions?
Yuck!
> 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.
OK, that sounds fixable. See additional patch below.
> Incidentally, servers with the input extension always assign buttons 4
> through 7 to wheel movement.
Interesting.
>> + 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?
Hmm... indeed there was a crash here, which I just fixed in the
patch below. But with that fix, I do see `wheel-up/down` events on the
tab-tab as well. Am I missing something?
> Lastly, plenty of old code that binds just the button events will be
> broken,
Indeed. But that should be a disappearing breed because such bindings
never worked under non-X11 GUIs and now they don't work either under X11
builds that use XInput2, which I expect is the vast majority of Emacsā„29
builds in GNU/Linux distributions.
> 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.
Hmm... the only code I can think of in `read_key_sequence` which cares
about button up/down is the code that drops the button down events and
the one that turns pairs into drag events. Is that ever useful
currently with the `mouse-4/5/6/7` events? No one has lamented this
under non-X11 builds so far nor under XInput2, so it seems unlikely to
be a problem.
> 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.
There's no doubt that event representation is a delicate/nasty part of
Emacs code, but I think it's really a trade-off between possible
short-term pain and the longer term simplification of unifying all wheel
handling under `wheel-*` events.
Also, I think we *will* have go through that pain sooner or later.
Stefan
[xterm-wheel-events.patch (text/x-diff, inline)]
diff --git a/src/keyboard.c b/src/keyboard.c
index 7c1822b3423..a881c3d6117 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -6628,8 +6628,13 @@ make_lispy_event (struct input_event *event)
if (CONSP (event->arg))
return list5 (head, position, make_fixnum (double_click_count),
- XCAR (event->arg), Fcons (XCAR (XCDR (event->arg)),
- XCAR (XCDR (XCDR (event->arg)))));
+ XCAR (event->arg),
+ /* FIXME: I don't know what I'm doing here. */
+ (CONSP (XCDR (event->arg))
+ && CONSP (XCDR (XCDR (event->arg))))
+ ? Fcons (XCAR (XCDR (event->arg)),
+ XCAR (XCDR (XCDR (event->arg))))
+ : Qnil);
else if (NUMBERP (event->arg))
return list4 (head, position, make_fixnum (double_click_count),
event->arg);
diff --git a/src/xterm.c b/src/xterm.c
index fcbe7a1ec4f..237b705b555 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -14548,7 +14548,7 @@ x_query_pointer (Display *dpy, Window w, Window *root_return,
static Lisp_Object
x_construct_mouse_click (struct input_event *result,
const XButtonEvent *event,
- struct frame *f)
+ struct frame *f, bool xi2)
{
int x = event->x;
int y = event->y;
@@ -14563,9 +14563,7 @@ x_construct_mouse_click (struct input_event *result,
: down_modifier));
/* Convert pre-XInput2 wheel events represented as mouse-clicks. */
-#ifdef HAVE_XINPUT2
- if (!dpyinfo->supports_xi2)
-#endif
+ if (!xi2)
{
Lisp_Object base = base_mouse_symbol (result->code);
int wheel
@@ -21895,13 +21893,14 @@ handle_one_xevent (struct x_display_info *dpyinfo,
&& event->xbutton.time > ignore_next_mouse_click_timeout)
{
ignore_next_mouse_click_timeout = 0;
- x_construct_mouse_click (&inev.ie, &event->xbutton, f);
+ x_construct_mouse_click (&inev.ie, &event->xbutton,
+ f, false);
}
if (event->type == ButtonRelease)
ignore_next_mouse_click_timeout = 0;
}
else
- x_construct_mouse_click (&inev.ie, &event->xbutton, f);
+ x_construct_mouse_click (&inev.ie, &event->xbutton, f, false);
*finish = X_EVENT_DROP;
goto OTHER;
@@ -21971,13 +21970,15 @@ handle_one_xevent (struct x_display_info *dpyinfo,
&& event->xbutton.time > ignore_next_mouse_click_timeout)
{
ignore_next_mouse_click_timeout = 0;
- x_construct_mouse_click (&inev.ie, &event->xbutton, f);
+ x_construct_mouse_click (&inev.ie, &event->xbutton,
+ f, false);
}
if (event->type == ButtonRelease)
ignore_next_mouse_click_timeout = 0;
}
else
- x_construct_mouse_click (&inev.ie, &event->xbutton, f);
+ x_construct_mouse_click (&inev.ie, &event->xbutton,
+ f, false);
if (!NILP (tab_bar_arg))
inev.ie.arg = tab_bar_arg;
@@ -23754,13 +23755,13 @@ handle_one_xevent (struct x_display_info *dpyinfo,
&& xev->time > ignore_next_mouse_click_timeout)
{
ignore_next_mouse_click_timeout = 0;
- x_construct_mouse_click (&inev.ie, &bv, f);
+ x_construct_mouse_click (&inev.ie, &bv, f, true);
}
if (xev->evtype == XI_ButtonRelease)
ignore_next_mouse_click_timeout = 0;
}
else
- x_construct_mouse_click (&inev.ie, &bv, f);
+ x_construct_mouse_click (&inev.ie, &bv, f, true);
if (!NILP (tab_bar_arg))
inev.ie.arg = tab_bar_arg;
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.