817061 - Work - Support multiple mouse buttons being pressed/released simultaneously

Status: RESOLVED FIXED

(Firefox for Metro Graveyard :: Input, defect)

Description

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

It turns out that PointerPressed/PointerReleased events are not fired for every button press/release.

We receive a PointerPressed event for the first button that is pressed, but then we do not receive any PointerPressed/PointerReleased events until ALL buttons have been released, regardless of what happens to the buttons in between.

As an example, if a user presses down the left mouse button, clicks the middle mouse button, presses down the right mouse button, releases the left mouse button, then releases the right mouse button, we will receive only one PointerPressed event (for the initial push of the left mouse button) and one PointerReleased event (for the final release of the right mouse button).

This likely won't affect normal browsing, but may be an issue for games and other input-intensive applications.  We should probably switch away from Pointer events for mouse input (but keep Pointer events for pen and touch).

Comment 1

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Button presses/releases that occur in between a PointerPressed and PointerReleased event arrive as PointerMoved events.  We just have to check the PointerUpdateKind member of our PointerPointProperties.  I've got a patch that utilizes this strategy that I'll attach momentarily.  Morphing this bug title to be about supporting multiple mouse buttons simultaneously.

Comment 2

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Attachment: Patch v1

This patch applies cleanly only after applying the patch in bug 820654

Comment 3

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Comment on attachment 691173 [details] [diff] [review]
Patch v1

I'm rebasing this patch.  I'll mark for review when I upload the rebased patch.

Comment 4

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Attachment: Patch v2

Rebased to apply to current elm tip

Comment 5

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Attachment: Patch v3

Oops, forgot to qref before posting.

Comment 6

[Jim Mathies [:jimm]]

Comment on attachment 693689 [details] [diff] [review]
Patch v3

Review of attachment 693689 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/windows/winrt/MetroInput.cpp
@@ +616,5 @@
>  
> +void
> +MetroInput::InitGeckoMouseEventFromPointerPoint(
> +                                  nsMouseEvent& aEvent,
> +                                  UI::Input::IPointerPoint* aPointerPoint) {

assert or fail here on a pad pointer.

Comment 7

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Attachment: Patch v4

(In reply to Jim Mathies [:jimm] from comment #6)
> Comment on attachment 693689 [details] [diff] [review]
> Patch v3
> 
> Review of attachment 693689 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: widget/windows/winrt/MetroInput.cpp
> @@ +616,5 @@
> >  
> > +void
> > +MetroInput::InitGeckoMouseEventFromPointerPoint(
> > +                                  nsMouseEvent& aEvent,
> > +                                  UI::Input::IPointerPoint* aPointerPoint) {
> 
> assert or fail here on a pad pointer.

Done!

Carrying forward r+

Comment 8

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Landed on elm:
https://hg.mozilla.org/projects/elm/rev/e428a6d9a339

Build containing the change:
https://tbpl.mozilla.org/?tree=Elm&rev=e428a6d9a339

Comment 9

[Matt Brubeck (:mbrubeck)]

Resolving bugs in the Firefox for Metro product that are fixed on the elm branch.  Sorry for the bugspam.  Search your email for "bugspam-elm" if you want to find and delete all of these messages at once.