Bug 11138 – Incorrect mouse event generation on Windows

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Don Gibson]

Mouse events are not generated correctly in the Windows port:
* Clicks are only reported for the left button
* BUTTONUP events are reported on the wrong button
* Some events are reported with <garbage> as the button

Patch to fix these issues coming shortly.

[Don Gibson]

Created attachment 10883 [details]
patch v1

This passes the actual button message along to the PlatformMouseEvent constructor, which uses it to set the button state, click count, etc. correctly.

Clicks are now produced for all three buttons.  Click counting should match Windows' behavior (from Spy++): a MOUSEUP of one button between clicks of another does not reset the click count for the second button, but a MOUSEDOWN does.

[Don Gibson]

Created attachment 10884 [details]
patch v2

Updated to latest trunk... most of the "set the correct button" portion of this bug was fixed yesterday by aroben.  The remaining bit of that changed here is to set m_button to something sane for non-click messages (like WM_MOUSEMOVE).  Probably unnecessary, but it seems better than leaving m_button uninitialized.

That leaves getting click handling correct as the thing taking up most of this patch.

[Adam Roben (:aroben)]

Comment on attachment 10884 [details]
patch v2

r=me. Thanks!

[Adam Roben (:aroben)]

Comment on attachment 10884 [details]
patch v2

Oops, code is fine, but we need a ChangeLog entry.

[Hunter L. Williams]

>+        case WM_LBUTTONDBLCLK:  // For these messages, the OS ensures that the
>+        case WM_MBUTTONDBLCLK:  // previous BUTTONDOWN was for the same button.
>+        case WM_RBUTTONDBLCLK:
>+            {
>+                DWORD curTime = GetTickCount();
>+                if (curTime - globalPrevClickTime > GetDoubleClickTime() ||
>+                    m_position != globalPrevClickPosition)
>+                    globalClickCount = 0;
>+                globalPrevClickTime = curTime;
>+            }
>+            globalPrevClickButton = m_button;
>+            globalPrevClickPosition = m_position;
>+            m_clickCount = ++globalClickCount;
>+            return;

Isn't a DBLCLK 2 clicks, not one? I saw code further down in WebCore that had if (event.event().clickCount() == 3) that did triple click handling. It seems that with this, for a triple click you'll get one DBLCLK and one LBUTTONDOWN (say) and thus only get m_clickCount = 2... or is clickCount incremented somewhere else independently for WM_*DBLCLK?

[Don Gibson]

Created attachment 10885 [details]
patch v3

Now with 100% more ChangeLog

[Don Gibson]

(In reply to comment #5)
> Isn't a DBLCLK 2 clicks, not one? I saw code further down in WebCore that had
> if (event.event().clickCount() == 3) that did triple click handling. It seems
> that with this, for a triple click you'll get one DBLCLK and one LBUTTONDOWN
> (say) and thus only get m_clickCount = 2... or is clickCount incremented
> somewhere else independently for WM_*DBLCLK?

WM_LMOUSEDBLCLK is sent in place of the second WM_LBUTTONDOWN on a double click (Windows doesn't queue up clicks to decide whether you're single- or double-clicking before sending messages).  So the message sequence on a double click is:
WM_LBUTTONDOWN
WM_LBUTTONUP
WM_LBUTTONDBLCLK
WM_LBUTTONUP
Thus, a double click message is still just one click.  We could avoid click counting altogether and just handle the messages as "double" and "single" clicks more easily if we didn't need to worry about dispatching triple-clicks, which Windows doesn't understand.

[Hunter L. Williams]

OK, it's one click on Windows, but in Frame::handleMousePressEvent, there's this test:

if (event.event().clickCount() >= 3) {
  handleMousePressEventTripleClick(event);
  ...

which means that on Mac at least the event is initialized with clickCount = 3 for the triple click case, which means we probably need to do that here too. 

When you enumerate the messages that get sent, do you mean that Windows counts the first DOWN/UP action as the first component of the double click? i.e. are they coalesced? Do DOWN, UP, DBLCLK get sent for all doubleclick actions? Is the following UP the only message sent for the third click? (What happened to the DOWN for that one?)

[Hunter L. Williams]

Apologies, I was confused. I was thinking about another issue, which I have filed in bug 11144.

[Don Gibson]

Comment on attachment 10885 [details]
patch v3

Forgot to ask for review, oops

[Adam Roben (:aroben)]

Comment on attachment 10885 [details]
patch v3

r=me

[Alexey Proskuryakov]

Committed revision 16811.