1300103 - Context menu is wrongly displayed if using the left click in the viewport while Touch simulation is enabled

Status: VERIFIED FIXED

(DevTools :: Responsive Design Mode, defect, P1)

Description

[Mihai Boldan, Desktop QA [:mboldan]]

[Affected versions]:
- Firefox 51.0a1 (2016-09-02)

[Affected platforms]:
- Windows 10x64, Mac OS X 10.11.1, Ubuntu 16.04x64

[Steps to reproduce]:
1. Launch Firefox.
2. From about:config, enable the devtools.responsive.html.enabled pref.
3. Open RDM.
4. Enable Toggle tools.
5. Hold the left click pressed in the viewport.

[Expected result]:
- If the click is pressed while the cursor is over an element, that element is correctly selected and if is pressed while the cursor is not over any element from the page, nothing happens.

[Actual result]:
- The context menu is displayed. Here is a screencast of the issue: https://drive.google.com/open?id=0B-cUbHVNThEKQWpucHNKMVd4YWs

[Regression range]:
- This is a recent regression.
Last good revision: 26e22af660e543ebb69930f082188b69ec756185
First bad revision: 987bda408af336b975d3d9108396356a8e0d28b3
Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=26e22af660e543ebb69930f082188b69ec756185&tochange=987bda408af336b975d3d9108396356a8e0d28b3

[Additional notes]:
- Note that if the left click is used in the viewport in an area where is no element to be selected. the nearest element from the cursor is selected(from my point of view is not en expected result).

Comment 1

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

I don't see this as a regression exactly, and also the feature is disabled for now.  It's related to bug 1292133 which wires up contextmenu messages.  The right-click case is improved (inspect element works), but this long tap left click issue now appears.

To clarify the STR, you need to enable touch simulation like the summary says:

1. Launch Firefox.
2. From about:config, enable the devtools.responsive.html.enabled pref.
3. Open RDM.
4. Enable touch simulation by clicking the hand icon in the viewport toolbar.
5. Hold the left click pressed in the viewport.

It appears to reproduce on at least macOS 10.11 and Windows 10 for me.

Comment 2

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Bug 1300103 - Use correct coordinates in simulated contextmenu event.

Review commit: https://reviewboard.mozilla.org/r/89908/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/89908/

Comment 3

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=81770271f6828c3f888b5c15234a5ba6a61dec3f

Comment 4

[Alexandre Poirot [:ochameau]]

Comment on attachment 8806513 [details]
Bug 1300103 - Use correct coordinates in simulated contextmenu event.

https://reviewboard.mozilla.org/r/89908/#review89586

::: devtools/shared/touch/simulator-core.js:239
(Diff revision 1)
>                           Ci.nsIDOMMouseEvent.MOZ_SOURCE_TOUCH);
>    },
>  
> -  sendContextMenu(target, x, y) {
> +  sendContextMenu({ target, clientX, clientY, screenX, screenY }) {
>      let doc = target.ownerDocument;
> -    let evt = doc.createEvent("MouseEvent");
> +    let MouseEvent = doc.defaultView.MouseEvent;

nit:
let { MouseEvent } = target.ownerDocument.defaultView;
?
(looks like doc is not used anywhere else)

Comment 5

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8806513 [details]
Bug 1300103 - Use correct coordinates in simulated contextmenu event.

https://reviewboard.mozilla.org/r/89908/#review89586

> nit:
> let { MouseEvent } = target.ownerDocument.defaultView;
> ?
> (looks like doc is not used anywhere else)

`doc.defaultView` gets used again for the `view` event option.

I cleaned it up slightly using `let view = target.ownerDocument.defaultView;` instead of `doc`.

Comment 6

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8806513 [details]
Bug 1300103 - Use correct coordinates in simulated contextmenu event.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/89908/diff/1-2/

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/22d480ab8d7e
Use correct coordinates in simulated contextmenu event. r=ochameau

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/22d480ab8d7e

Comment 9

[Mihai Boldan, Desktop QA [:mboldan]]

The issue is no longer reproducible on Firefox 52.0a1 (2016-11-09).
The tests were performed under Windows 10x64, Mac OS X 10.11.6, Ubuntu 16.04x64.

Comment 10

[Mihai Boldan, Desktop QA [:mboldan]]

Hello Ryan,

After more investigation I noticed that this issue is still reproducible on the latest Nightly build. 
Also, I've retested it on Firefox 52.0a1 (2016-11-09) and it seems that it was not fixed from the beginning. I've probably used the STR from Comment 0 instead of the STR from Comment 1 while verifying this bug. 
Should I reopen this one, or it is better to log a new bug?

Comment 11

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to Mihai Boldan, QA [:mboldan] from comment #10)
> Hello Ryan,
> 
> After more investigation I noticed that this issue is still reproducible on
> the latest Nightly build. 
> Also, I've retested it on Firefox 52.0a1 (2016-11-09) and it seems that it
> was not fixed from the beginning. I've probably used the STR from Comment 0
> instead of the STR from Comment 1 while verifying this bug. 
> Should I reopen this one, or it is better to log a new bug?

Hmm, I am not seeing the same thing here.  I am testing with Firefox 53 (2016-11-22) on macOS 10.12.

Using steps from comment 1, the context menu is positioned correctly after long tapping in the viewport.

Looking at comment 0, I don't think steps there actually matched your screencast.  Touch simulation is clearly on in your video, so it seems like the steps in comment 1 are closely to what was used even then.

Can you provide more details about your setup?  Perhaps an updated video?

Comment 12

[Mihai Boldan, Desktop QA [:mboldan]]

It seems that it was a misunderstanding from my side. After discussing with Ryan, we jump to the conclusion that the actual result from the Latest Nightly and Latest Aurora builds is also the expected one. 
It's expected for the context menu to be displayed if holding the left click while the Touch simulation is enabled.