bug#43379: [PATCH] Double-click events can occur without preceding single-click events

[Daniel Koning]

According to the documentation (21.7.7 of the Lisp ref, "Repeat Events"), a
double-click binding should "assume that the single-click command has already
run," since Emacs only produces a double-click event after an equivalent
single-click event. But (so long as 'double-click-time' is set generously
enough) it's easy to trigger a double-click event without a corresponding
single-click event, because the event handling primitives aren't quite
particular enough about what they count as a repeat.

For one thing, the code ignores whether the modifier keys of one input action
differ from the next. So the sequence "M-mouse-1 mouse-1" produces <M-mouse-1>
<double-mouse-1>. This is no good: <M-mouse-1> is likely bound to something
different from <mouse-1>, so the <double-mouse-1> command will run under false
premises.

It also uses just one repeat click counter, despite the fact that down/up click
event pairs can be interleaved. The counter increments when a button-down
matches the last button-up, and it carries over to whatever button-up comes in
next. Therefore, the mouse sequence

    down on mouse-1
    down on mouse-3
    up on mouse-1
    down on mouse-1
    up on mouse-3

produces a <double-mouse-3> event with no preceding <mouse-3>.

The function 'make_lispy_event', which translates raw events to Lisp objects, is
responsible for tagging a mouse event (button-down, click, drag, or wheel) as a
double. This patch against master revises that function to avoid creating "bare"
double mouse events. It does so by consolidating the scattered repeat-tracking
logic into its own function, where more relevant state can be maintained without
any extra global cruft. (With less cruft, in fact: 'button_down_time' is no
longer punned as an interrupt flag, wheel event codes need not be coerced to
negative to stuff them into the same variable as clicks, etc.)

Just as before, a repeat mouse event has to use the same button, happen in
roughly the same place, happen at roughly the same time, and so on. Aside from
the specific deliberate narrowing I've described, I believe this code duplicates
the logic of the old code exactly. That includes nuances that strike me as
strange. For instance,

    down on mouse-1
    up on mouse-1
    down on mouse-1
    keystroke
    up on mouse-1
    down on mouse-1

finishes with <double-mouse-1> <down-mouse-1>, even though it would make more
sense to me if the keystroke reset the count right away. But that's how it was
before.

There is still at least one fringy situation in which a <double-mouse-1> command
could run without the <mouse-1> command running first: when the first and second
click land in different windows. I would have liked to add a test for this, but
I don't fully understand the type constraint on the 'frame_or_window' member of
struct input_event (specifically when it's a mouse event). 'make_lispy_event'
seems to assume it's a frame: it does an XFRAME without a check. But then the
comparison against 'double-click-fuzz', which I folded into the new function,
tries both possibilities. I left the more cautious code in place, and I haven't
added any more logic related to windows.

This also has no effect on xterm-mouse-mode, which doesn't interact with the C
event translator at all. It just takes key sequences which have already passed
through 'make_lispy_event' and translates them to mouse-event lists.
xterm-mouse-mode has some major inconsistencies with the GUI repeat-handling
behavior, but it doesn't seem to exhibit the bug this report describes.

This patch rewords the docs slightly to say explicitly that modifier keys make a
difference. It also makes a small tweak to the criteria for drag events, and
another tweak to the Cocoa-specific trackpad code to fix a conspicuous bug
caused by a similar oversight. Namely, newly pressed modifier keys can attach to
invisible so-called "momentum" events generated by a swipe on the trackpad that
has already ended. One result is that if a Cocoa user scrolls up to the top of
the buffer and immediately presses (say) C-e, Emacs gets barraged with
<C-wheel-up> events and scales the buffer text up to a ridiculous size.

Daniel

repeat-events.patch
Description: Text Data