Work - Support multiple mouse buttons being pressed/released simultaneously

RESOLVED FIXED

Status

Firefox for Metro
Input
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: TimAbraldes, Assigned: TimAbraldes)

Tracking

Trunk
All
Windows 8.1

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: feature=work [completed-elm])

Attachments

(1 attachment, 3 obsolete attachments)

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).

Updated

5 years ago
Depends on: 794621
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.
Assignee: nobody → tabraldes
Status: NEW → ASSIGNED
Summary: Don't use Pointer events for mouse input → Support multiple mouse buttons being pressed/released simultaneously
Whiteboard: [metro-mvp?][metro-it2]
Created attachment 691173 [details] [diff] [review]
Patch v1

This patch applies cleanly only after applying the patch in bug 820654
Attachment #691173 - Flags: review?(jmathies)

Updated

5 years ago
Whiteboard: [metro-mvp?][metro-it2] → [metro-mvp][metro-it2]
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.
Attachment #691173 - Flags: review?(jmathies)
Created attachment 693675 [details] [diff] [review]
Patch v2

Rebased to apply to current elm tip
Attachment #691173 - Attachment is obsolete: true
Attachment #693675 - Flags: review?(jmathies)
Created attachment 693689 [details] [diff] [review]
Patch v3

Oops, forgot to qref before posting.
Attachment #693675 - Attachment is obsolete: true
Attachment #693675 - Flags: review?(jmathies)
Attachment #693689 - Flags: review?(jmathies)

Comment 6

5 years ago
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.
Attachment #693689 - Flags: review?(jmathies) → review+
Created attachment 693963 [details] [diff] [review]
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+
Attachment #693689 - Attachment is obsolete: true
Attachment #693963 - Flags: review+
Landed on elm:
https://hg.mozilla.org/projects/elm/rev/e428a6d9a339

Build containing the change:
https://tbpl.mozilla.org/?tree=Elm&rev=e428a6d9a339
Whiteboard: [metro-mvp][metro-it2] → [metro-mvp][metro-it2][completed-elm]

Updated

5 years ago
Summary: Support multiple mouse buttons being pressed/released simultaneously → Work - Support multiple mouse buttons being pressed/released simultaneously
Whiteboard: [metro-mvp][metro-it2][completed-elm] → feature=work [completed-elm]
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.