Last Comment Bug 227328 - we get extra single-click events due to GDK
: we get extra single-click events due to GDK
Product: Core Graveyard
Classification: Graveyard
Component: GFX: Gtk (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: ---
Assigned To: David Baron :dbaron: ⌚️UTC-10
: Hixie (not reading bugmail)
Depends on:
Blocks: gtk2
  Show dependency treegraph
Reported: 2003-12-02 18:37 PST by David Baron :dbaron: ⌚️UTC-10
Modified: 2009-01-22 10:17 PST (History)
2 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---

testcase (673 bytes, text/html; charset=UTF-8)
2003-12-02 18:39 PST, David Baron :dbaron: ⌚️UTC-10
no flags Details
more thorough testcase (740 bytes, text/html; charset=UTF-8)
2003-12-02 18:56 PST, David Baron :dbaron: ⌚️UTC-10
no flags Details
patch (1.11 KB, patch)
2003-12-02 19:02 PST, Christopher Blizzard (:blizzard)
dbaron: review+
Details | Diff | Splinter Review
final patch (1.06 KB, patch)
2003-12-03 08:28 PST, Christopher Blizzard (:blizzard)
dbaron: review+
dbaron: superreview+
Details | Diff | Splinter Review
GTK1 patch (6.43 KB, patch)
2003-12-03 12:43 PST, David Baron :dbaron: ⌚️UTC-10
no flags Details | Diff | Splinter Review
GTK1 patch (1.49 KB, patch)
2003-12-03 12:45 PST, David Baron :dbaron: ⌚️UTC-10
blizzard: review+
blizzard: superreview+
Details | Diff | Splinter Review

Description David Baron :dbaron: ⌚️UTC-10 2003-12-02 18:37:31 PST
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.
Comment 1 David Baron :dbaron: ⌚️UTC-10 2003-12-02 18:39:00 PST
Created attachment 136701 [details]

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 2 David Baron :dbaron: ⌚️UTC-10 2003-12-02 18:39:57 PST
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.
Comment 3 Christopher Blizzard (:blizzard) 2003-12-02 18:44:43 PST
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.
Comment 4 David Baron :dbaron: ⌚️UTC-10 2003-12-02 18:56:33 PST
Created attachment 136702 [details]
more thorough testcase

Just add "  click(n)" to the steps to reproduce for the previous testcase.
Comment 5 Christopher Blizzard (:blizzard) 2003-12-02 19:02:34 PST
Created attachment 136703 [details] [diff] [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 6 David Baron :dbaron: ⌚️UTC-10 2003-12-02 19:07:44 PST
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)
Comment 7 Christopher Blizzard (:blizzard) 2003-12-03 08:28:50 PST
Created attachment 136724 [details] [diff] [review]
final patch

Patch that includes a fix for dbaron's comment.
Comment 8 David Baron :dbaron: ⌚️UTC-10 2003-12-03 10:15:52 PST
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).
Comment 9 David Baron :dbaron: ⌚️UTC-10 2003-12-03 10:42:29 PST
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.)
Comment 10 David Baron :dbaron: ⌚️UTC-10 2003-12-03 12:43:46 PST
Created attachment 136745 [details] [diff] [review]
GTK1 patch

This is tested on a GTK1 build.
Comment 11 David Baron :dbaron: ⌚️UTC-10 2003-12-03 12:45:01 PST
Created attachment 136746 [details] [diff] [review]
GTK1 patch

It would help if I diffed the right file.
Comment 12 David Baron :dbaron: ⌚️UTC-10 2003-12-21 10:56:14 PST
Fix checked in to trunk, 2003-12-21 10:54/55 -0800.

Note You need to log in before you can comment on or make changes to this bug.