957490 - MouseEvent.buttons property has incorrect value in WinRT/Metro

Status: RESOLVED FIXED

(Core Graveyard :: Widget: WinRT, defect)

Description

[Matt Brubeck (:mbrubeck)]

Attachment: test case

The "buttons" property of the "mousedown" event is not initialized correctly in Metro Firefox.  You can see this in the attached test case.  When you press a mouse button, it should display 1 for the left mouse button, 2 for the right button, 4 for the middle button, or a sum if you press multiple buttons.  But on Metro it always displays zero, or only the previous button(s) if you press multiple buttons.

Comment 1

[Matt Brubeck (:mbrubeck)]

Attachment: patch

ModifierKeyState::InitMouseEvent is supposed to set the "buttons" property, but it uses ::GetKeyState which does not seem to be return up-to-date data in immersive mode.  With this patch we call equivalent WinRT APIs instead when running in Metro mode.

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 8356978 [details] [diff] [review]
patch

>+    props->get_IsRightButtonPressed(&buttonPressed);
>+    if (buttonPressed)
>+      buttons |= WidgetMouseEvent::eRightButtonFlag;
>+
>+    props->get_IsRightButtonPressed(&buttonPressed);
>+    if (buttonPressed)
>+      buttons |= WidgetMouseEvent::eRightButtonFlag;

Duplicated the code for right button :-)

And you should use {} for even if the content of a block is only one line.
https://developer.mozilla.org/en-US/docs/Developer_Guide/Coding_Style#Control_Structures

Comment 3

[Matt Brubeck (:mbrubeck)]

Attachment: patch v2

Fixed the problems pointed out by Masayuki above.  Thanks!

Comment 4

[Matt Brubeck (:mbrubeck)]

Attachment: automated test

This is a simple mochitest for this bug plus bug 945438.

Comment 5

[Matt Brubeck (:mbrubeck)]

Comment on attachment 8357308 [details] [diff] [review]
automated test

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

::: browser/metro/base/tests/mochitest/browser_mouse_events.js
@@ +6,5 @@
> +const leftButtonFlag = 1;
> +const rightButtonFlag = 2;
> +
> +gTests.push({
> +  desc: "native long tap works",

Oops, copy/paste leftovers.  Updated the description locally.

Comment 6

[Matt Brubeck (:mbrubeck)]

Folded the two patches together and landed on fx-team:
https://hg.mozilla.org/integration/fx-team/rev/d3ab07b86509

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d3ab07b86509