Closed Bug 227328 Opened 21 years ago Closed 21 years ago

we get extra single-click events due to GDK


(Core Graveyard :: GFX: Gtk, defect)

Not set


(Not tracked)



(Reporter: dbaron, Assigned: dbaron)



(Whiteboard: [patch])


(3 files, 3 obsolete files)

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
 * 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.
Attached file testcase (obsolete) —
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]

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.
Attached file more thorough testcase
Just add "  click(n)" to the steps to reproduce for the previous testcase.
Attachment #136701 - Attachment is obsolete: true
Attached patch patch (obsolete) — Splinter Review
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]

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;
if (type == GDK_2BUTTON_PRESS || type == GDK_3BUTTON_PRESS)
Attachment #136703 - Flags: review+
Attached patch final patchSplinter Review
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.)
Attached patch GTK1 patch (obsolete) — Splinter Review
This is tested on a GTK1 build.
Attached patch GTK1 patchSplinter Review
It would help if I diffed the right file.
Attachment #136745 - Attachment is obsolete: true
Attachment #136746 - Flags: superreview+
Attachment #136746 - Flags: review+
Fix checked in to trunk, 2003-12-21 10:54/55 -0800.
Closed: 21 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.