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


Attachments
testcase (673 bytes, text/html; charset=UTF-8)
2003-12-02 18:39 PST, David Baron :dbaron: ⌚️UTC-8
no flags Details
more thorough testcase (740 bytes, text/html; charset=UTF-8)
2003-12-02 18:56 PST, David Baron :dbaron: ⌚️UTC-8
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-8
no flags Details | Diff | Splinter Review
GTK1 patch (1.49 KB, patch)
2003-12-03 12:45 PST, David Baron :dbaron: ⌚️UTC-8
blizzard: review+
blizzard: superreview+
Details | Diff | Splinter Review

Description User image David Baron :dbaron: ⌚️UTC-8 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
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.
Comment 1 User image David Baron :dbaron: ⌚️UTC-8 2003-12-02 18:39:00 PST
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 2 User image David Baron :dbaron: ⌚️UTC-8 2003-12-02 18:39:57 PST
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.
Comment 3 User image 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 User image David Baron :dbaron: ⌚️UTC-8 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 User image Christopher Blizzard (:blizzard) 2003-12-02 19:02:34 PST
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 6 User image David Baron :dbaron: ⌚️UTC-8 2003-12-02 19:07:44 PST
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;
Comment 7 User image 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 User image David Baron :dbaron: ⌚️UTC-8 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 User image David Baron :dbaron: ⌚️UTC-8 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 User image David Baron :dbaron: ⌚️UTC-8 2003-12-03 12:43:46 PST
Created attachment 136745 [details] [diff] [review]
GTK1 patch

This is tested on a GTK1 build.
Comment 11 User image David Baron :dbaron: ⌚️UTC-8 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 User image David Baron :dbaron: ⌚️UTC-8 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.