Closed Bug 1686361 Opened 3 years ago Closed 3 years ago

Sync event.synthesizeMouseAtPoint from EventUtils.js

Categories

(Remote Protocol :: Marionette, defect, P3)

Default
defect

Tracking

(firefox88 fixed)

RESOLVED FIXED
88 Branch
Tracking Status
firefox88 --- fixed

People

(Reporter: andrew, Assigned: andrew)

References

Details

Attachments

(1 file, 2 obsolete files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/87.0.4280.88 Safari/537.36

Steps to reproduce:

When calling synthesizeMouseAtPoint with anything but a mouseup or mousedown event, the events generated lead to buttons being pushed:

event.synthesizeMouseAtPoint(58, 93, {type: 'mousemove'}, win);

Actual results:

Produces an event with properties:

{
  'altKey': False,
  'button': 0,
  'buttons': 1,
  'ctrlKey': False,
  'metaKey': False,
  'pageX': 58,
  'pageY': 93,
  'shiftKey': False,
  'target': 'outer',
  'type': 'mousemove',
}

Expected results:

The event should not have a value of 1 for buttons.

This seems to be because ContentUtils.SendMouseEvent sets the value of buttons from the value of button if buttons is not specified.

The default value of buttons set by event.synthesizeMouseAtPoint() is domutils.MOUSE_BUTTONS_NOT_SPECIFIED (0), but the default value of button is 0, which is the PRIMARY button.

When the buttons value is domutils.MOUSE_BUTTONS_NOT_SPECIFIED, then ContentUtils.SendMouseEvent() sets the value of the buttons flag based on the value of button:

  event.mButtons = aButtons != nsIDOMWindowUtils::MOUSE_BUTTONS_NOT_SPECIFIED
                       ? aButtons
                   : msg == eMouseUp ? 0
                                     : GetButtonsFlagForButton(aButton);

In this situation, aButtons is NOT_SPECIFIED and the msg != mouseUp, so we call GetButtonsFlagForButton(0)
When button is 0 (MouseButton::ePrimary), then the button flag is (correctly) the MouseButtonsFlag::ePrimaryFlag - 1.

Ideally the default value of button should be -1 (not specified), and then this will all just work, but when opts.type is empty, the default value is assumed to be a button click on the primary button.

When synthesizeMouseAtPoint() is called with a mouse event type, then
the default should be not to press any buttons.

Depends on D100435

Assignee: nobody → andrew

This is great work Andrew! As already asked on Matrix, would you mind just fixing that in EventUtils.js? So we can move this bug to the other product / component. Once that has been fixed we can then do a sync for the whole module.

If that would be too much work (necessary time) for you, I'm happy to get this just landed for our implementation, and sync with EventUtils later. Please let me know. Thanks.

Severity: -- → S3
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: needinfo?(andrew)
Priority: -- → P3

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

Yup sure - I've pushed that as D101690.

I've also found a new bug in nsContentUtils reported as https://bugzilla.mozilla.org/show_bug.cgi?id=1686625 but that should not affect this issue at all.

Flags: needinfo?(andrew)

Comment on attachment 9196989 [details]
Bug 1686361: Set the value of buttons based on provided event

Revision D101690 was moved to bug 1686663. Setting attachment 9196989 [details] to obsolete.

Attachment #9196989 - Attachment is obsolete: true
Attachment #9196692 - Attachment is obsolete: true

Once bug 1686663 has been fixed, we can sync the code from EventUtils.js, to be up-to-date also with other additions. For the remaining sync task I filed bug 1686666, which can be done at a later time.

Depends on: 1686663
Summary: event.synthesizeMouseAtPoint defaults to pressing buttons → Sync event.synthesizeMouseAtPoint from EventUtils.js

Hi Andrew, will you be able to create that small patch? If not please let me know and I can cover it. Thanks!

Flags: needinfo?(andrew)

Yup - I’ll try and do that over the next 24-48 hours. I’ll leave the needinfo flag to remind me.

Hi @whimboo,

Can I just check what you have in mind for this change. Do you want me to just copy over the fix so that we just correctly calculate mouse buttons, or look at a full sync of that function? It looks like it’s been slowly diverging for several years. Ironically the code in marionette uses more recent JS features (specifically let) than the code it is based on in mochitest.

Thanks

Flags: needinfo?(andrew) → needinfo?(hskupin)

Just pick the necessary parts for that particular change. The full sync will happen afterward via bug 1686666 (which is marked as being blocked by this bug). Thanks.

Flags: needinfo?(hskupin)

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
Attachment #9204426 - Attachment description: Bug 1686361: Calculate buttons based on provided event → Bug 1686361 - [marionette] Sync synthesizeMouseAtPoint from EventUtils.js.
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9c6887577e85
[marionette] Sync synthesizeMouseAtPoint from EventUtils.js. r=marionette-reviewers,whimboo
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 88 Branch
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: