1686663 - SynthesizeMouseAtPoint sends incorrect value for buttons

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[Andrew Nicols]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 11_1_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/87.0.4280.141 Safari/537.36

Steps to reproduce:

Currently the default value of buttons is set to
MOUSE_BUTTONS_NOT_SPECIFIED, which defers calculation of the value to
the DOMWindowUtils GetButtonsFlagForButton function. This calculates a
default value based upon the value of the button key.

By specifying a default button value of 0, which has a meaning of
ePrimary, the buttons value is calculated as the
ePrimaryFlag (1), suggesting that a button was pressed.

synthesizeMouseAtPoint(x, y, {type: 'mousemove'}, window)

Actual results:

An event was generated with properties including:

{
  type: 'mousemove',
  button: 0,
  buttons: 1,
}

Expected results:

An event was generated with properties including:

{
  type: 'mousemove',
  button: 0,
  buttons: 0,
}

Comment 1

[Andrew Nicols]

Attachment: Bug 1686663: Set the value of buttons based on provided event

Currently the default value of buttons is set to
MOUSE_BUTTONS_NOT_SPECIFIED, which defers calculation of the value to
the DOMWindowUtils GetButtonsFlagForButton function. This calculates a
default value based upon the value of the button key.

By specifying a default button value of 0, which has a meaning of
ePrimary, the buttons value is calculated as the
ePrimaryFlag (1), suggesting that a button was pressed.

This patch changes the behaviour to set the value of buttons based on
the original value of button before the default was applied. The value
of buttons also considers the event type to ensure that a mousedown
event has a default value calculated by DOMWindowUtils.

With the new behaviour:

• if a value was explicitly set for buttons, this is used
• if a value was explicitly set for button, then the not-specified
  constant is used to defer calculation to DOMWindowUtils
• if an event type was specified and that event type was not the
  'mousedown' event, then the no-button constant is used
• if an event type was not specified or it was for the 'mousedown'
  event, then the not-specified constant is used to defer calculation to
  DOMWindowUtils

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4a9483083a4d
Set the value of buttons based on provided event r=smaug

Comment 3

[Alexandru Michis [:malexandru]]

Backed out changeset 4a9483083a4d (Bug 1686663) for causing bc failures in browser_selectpopup.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/6d2246c011f5f2f6961750353850b68beb3e812c

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&fromchange=1ddecf8b957105b7987c719ad90057bcc0e4f4ea&searchStr=windows%2C7%2Copt%2Cmochitests%2Ctest-windows7-32%2Fopt-mochitest-browser-chrome-e10s&tochange=c7f363ff3c4fdf8134d24ccc5db0042a4baf949d&group_state=expanded&selectedTaskRun=IaC9N6AsTcSDbwPx99k12Q.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=327065548&repo=autoland&lineNumber=1980
(Update) Looks to have also caused failures in browser_anti_clickjacking.js:
https://treeherder.mozilla.org/logviewer?job_id=327051449&repo=autoland&lineNumber=2100

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Hi Andrew, I saw that the patch got backed out. Have you had a chance yet to check why browser/base/content/test/forms/browser_selectpopup.js is failing here?

Comment 5

[Andrew Nicols]

Hi @whimboo,

I started having a look but have been busy the past week. I have a fix for the browser/base/content/test/forms/browser_selectpopup.js but I haven’t had a chance to look at browser/extensions/formautofill/test/browser/creditCard/browser_anti_clickjacking.js yet.

I’ve not had a chance to work out why that ones failing as I can’t see any direct use of mousemove in there.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Thanks for the update. I had a look at the Treeherder link for the backout and I can only see the failures for browser_selectpopup.js, but none for browser_anti_clickjacking.js. Can you remember where that latter failure actually happened?

Comment 7

[Andrew Nicols]

Hi @whimboo,

I can replicate those issues locally but I don’t see how they relate to the changes here and I’m not quite sure where to fix them. I see them at the second tree herder link: https://treeherder.mozilla.org/logviewer?job_id=327051449&repo=autoland&lineNumber=2100

I’ve updated the commit in Phabricator to fix the issues in browser_selectpopup.js but I’m unable to run any try jobs to confirm the fix.

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Thanks for the link. I had a quick look and what looks strange is that getDisplayedPopupItems() should be run only once, but given the log it runs twice:

https://treeherder.mozilla.org/logviewer?job_id=327093066&repo=autoland&lineNumber=2091-2092

You could add some more info(msg) calls to the test or the used head.js file so we can figure out what's wrong with the first popup element.

If browser_selectpopup.js works fine locally with your patch applied, then it's fine and we can test on try once the credit card test works.

Comment 9

[Andrew Nicols]

Any chance you could kick off the try jobs again please. I’m unable to replicate this issue any further after rebasing. Thanks

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Done: https://treeherder.mozilla.org/jobs?repo=try&revision=680c8b780d03a48a36da37006a44ae911a2c887c

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c50c4d146af1
Set the value of buttons based on provided event r=smaug

Comment 12

[Andreea Pavel [:apavel]]

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