we get extra single-click events due to GDK

RESOLVED FIXED

Status

Core Graveyard
GFX: Gtk
RESOLVED FIXED
14 years ago
9 years ago

People

(Reporter: dbaron, Assigned: dbaron)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [patch])

Attachments

(3 attachments, 3 obsolete attachments)

(Assignee)

Description

14 years ago
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.
(Assignee)

Comment 1

14 years ago
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.
(Assignee)

Comment 2

14 years ago
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.
(Assignee)

Comment 4

14 years ago
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.
(Assignee)

Comment 6

14 years ago
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+
(Assignee)

Updated

14 years ago
Whiteboard: [patch]
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)
(Assignee)

Comment 8

14 years ago
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+
(Assignee)

Comment 9

14 years ago
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.)
(Assignee)

Comment 10

14 years ago
Created attachment 136745 [details] [diff] [review]
GTK1 patch

This is tested on a GTK1 build.
(Assignee)

Comment 11

14 years ago
Created attachment 136746 [details] [diff] [review]
GTK1 patch

It would help if I diffed the right file.
Attachment #136745 - Attachment is obsolete: true
Attachment #136746 - Flags: superreview+
Attachment #136746 - Flags: review+
Blocks: 92033
(Assignee)

Comment 12

14 years ago
Fix checked in to trunk, 2003-12-21 10:54/55 -0800.
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.