we get extra single-click events due to GDK

RESOLVED FIXED

Status

RESOLVED FIXED
15 years ago
10 years ago

People

(Reporter: dbaron, Assigned: dbaron)

Tracking

Trunk
x86
Linux

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [patch])

Attachments

(3 attachments, 3 obsolete attachments)

Because of GDK, we send out extra single-click DOM events that we shouldn't be
for double-clicks and triple-clicks.

This is because the GDK event queue works in a weird way -- when it sees a
double or triple click, it adds an additional double or triple press event after
the event for the single press.  We need to ignore these events.  There are two
options:
 * look ahead in the event queue
 * ignore GDK's double/triple-press events and synthesize our own using the
times stored in the events and GDK's interval

blizzard prefers the first of these.  I'm not sure if I can assume that if
there's a corresponding double/triple press event it will always be the next
event in the queue.
Created attachment 136701 [details]
testcase

For each click of the mouse, you should see one of the following lines
(depending on whether than click has a clickCount of 1, 2, or 3):
down(1).  up.
down(2).  up.
down(3).  up.
Comment on attachment 136701 [details]
testcase

Steps to reproduce: triple-click in attachment	136701 [details]

Actual results:
down(1).  up.
down(1).  down(2).  up.
down(1).  down(3).  up.

Expected results:
down(1).  up.
down(2).  up.
down(3).  up.
We've been talking about this on irc.  I didn't realize that gdk was generating
an extra single click event for each double click.

Anyway, it should be pretty easy to look in the queue and see if there's either
a double or triple click event in the queue.  If there is, we can just assume
that this is the click associated with that event, and drop it.
Created attachment 136702 [details]
more thorough testcase

Just add "  click(n)" to the steps to reproduce for the previous testcase.
Attachment #136701 - Attachment is obsolete: true
Created attachment 136703 [details] [diff] [review]
patch

Patch that checks the next event in the queue to see if it's a 2BUTTON or
3BUTTON patch.	This seems pretty safe.  The code in gdk puts the 2BUTTON or
3BUTTON event onto the front of the queue using gdk_event_put() so it's pretty
safe to assume that the very next event will be the event we're looking for.

This works with the test case.
Comment on attachment 136703 [details] [diff] [review]
patch


Looks good to me, and works fine for me.  But, since I always like to simplify
code, I'll note that

>+        if (peekedEvent->any.type == GDK_2BUTTON_PRESS ||
>+            peekedEvent->any.type == GDK_3BUTTON_PRESS) {
>+            gdk_event_free(peekedEvent);
>+            return;
>+        }
>+
>+        gdk_event_free(peekedEvent);

Could be simplified to:

GdkEventType type = peekedEvent->any.type;
gdk_event_free(peekedEvent);
if (type == GDK_2BUTTON_PRESS || type == GDK_3BUTTON_PRESS)
    return;
Attachment #136703 - Flags: review+
Created attachment 136724 [details] [diff] [review]
final patch

Patch that includes a fix for dbaron's comment.
Attachment #136703 - Attachment is obsolete: true
Attachment #136724 - Flags: superreview?(dbaron)
Attachment #136724 - Flags: review?(dbaron)
Comment on attachment 136724 [details] [diff] [review]
final patch

I suspect this needs to be fixed in the GTK1 port as well (unless brendan was
using a GTK2 build).
Attachment #136724 - Flags: superreview?(dbaron)
Attachment #136724 - Flags: superreview+
Attachment #136724 - Flags: review?(dbaron)
Attachment #136724 - Flags: review+
It looks like the same patch should apply in the same function in
widget/src/gtk1/nsWidget.cpp (not nsWindow).  I'm still waiting for my build to
finish to test the result.  (I did test that it's needed for the GTK1 port.)
Attachment #136746 - Flags: superreview+
Attachment #136746 - Flags: review+
Fix checked in to trunk, 2003-12-21 10:54/55 -0800.
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.