1276533 - Tests: Synthesizing mouse click may fail on smaller screens

Status: RESOLVED FIXED

(DevTools :: General, defect)

Description

[Steve Melia]

Happen to be using a laptop 1366x768 and encountered a test failure in devtools/client/inspector/test/browser_inspector_initialization.js when trying to simulate "right-click; inspect element" as the #closing tag was not in the visible area.

Comment 1

[Steve Melia]

Attachment: 1276533-synthesizeMouse.patch

Use scrollIntoView to make sure the element is in the visible area before simulating the mouse click.

Comment 2

[Patrick Brosset <:pbro>]

Comment on attachment 8757729 [details] [diff] [review]
1276533-synthesizeMouse.patch

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

Looks ok to me, but I think this is something EventUtils should (and might) handle.
I don't remember, off the top of my head, cases where we had to scroll nodes into view before using EventUtils to click on them.
Alex might know more than me on this.

Comment 3

[Alexandre Poirot [:ochameau]]

Comment on attachment 8757729 [details] [diff] [review]
1276533-synthesizeMouse.patch

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

In theory, you may try to fire a click event on a hidden element.
A webpage, for example, may do element.click() or use dispatchEvent on some dom nodes that are out of the viewport.
Given that EventUtils are low lever helpers, it doesn't make sense to automagically scoll to the element.
I'm also wondering if we should do that in synthesizeMouseEvent.
We may keep that for clickOnInspectMenuItem function.

I'm wondering if any test break with your current patch:
  https://treeherder.mozilla.org/#/jobs?repo=try&revision=73ce88dfcfc5

Comment 4

[Alexandre Poirot [:ochameau]]

Comment on attachment 8757729 [details] [diff] [review]
1276533-synthesizeMouse.patch

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

Looks like it doesn't break any test and it seems like a good assumption to ensure nodes are in the viewport for inspector tests.

Could you just add a comment in synthesizeMouse comment about the automatic scroll if not in viewport?

Thanks.

Comment 5

[Steve Melia]

Attachment: 1276533-synthesizeMouse-comment.patch

Cool! Updated the comment, thanks Alex.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/cb0b25c9093f
Update test actor to scroll element into view before simulating mouse click;r=ochameau

Comment 7

[Carsten Book [:Tomcat]]

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