Responsive design mode triggers touch events when inspecting

VERIFIED FIXED in Firefox 68

Status

defect
P1
normal
VERIFIED FIXED
2 years ago
12 days ago

People

(Reporter: dev, Assigned: pbro)

Tracking

(Blocks 1 bug, {regression})

56 Branch
Firefox 68
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox-esr60 wontfix, firefox56 wontfix, firefox57 wontfix, firefox58 wontfix, firefox59 wontfix, firefox66 wontfix, firefox67 wontfix, firefox68 verified, firefox69 verified)

Details

(Whiteboard: [rdm-mvp] [dt-q])

Attachments

(1 attachment)

Reporter

Description

2 years ago
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:56.0) Gecko/20100101 Firefox/56.0
Build ID: 20171002220106

Steps to reproduce:

1. Create a div with touch listeners. (Like in this fiddle: https://jsfiddle.net/qyx5qccm/)
2. Start the responsive design mode and turn on touch simulation.
3. Click on the "select element" button and then select the div we created in step 1.

(I am using e10s so this is related to the "new" responsive design mode. I cannot tell if it happened in the old one as well.)


Actual results:

The touch listeners get fired.


Expected results:

The touch listeners shouldn't get fired. If the touch listener fires a function modifying the element, this element becomes hard to debug. 
This doesn't seem to happen when inspecting elements with click listeners (and touch simulation disabled).
Component: Untriaged → Developer Tools: Responsive Design Mode
Thanks for filing and sharing the fiddle!

This seems similar but not identical to bug 1386796.
Blocks: rdm-touch
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P2
See Also: → 1386796

Comment 2

2 years ago
Hi Ryan,
I'm interested in working on this issue. 
Can you provide me some pointers on how I can approach this?
Flags: needinfo?(jryans)
Sure, I'll try to sketch the issue here.

Effectively, we have two components conflicting with each other:

* The touch simulator[1] (which is only active when touch is enabled in RDM), which listens for mouseup, mousedown, etc. and creates fake touch events
* The inspector's "pick" mode[2] for selecting elements, which listens for click, etc. to set the node

At a high level, I think most users would agree (like the reporter of this bug) it's confusing for the touch simulator to do things when pick mode is enabled.

The code about picking sets `_isPicking`[3] in the highlighter actor to true when active.  I think one approach would be for the touch simulator to check if there is a highlighter actor, and if so, see if it is picking.  If it is, it would ignore events.

So, as a rough sketch:

1. The emulation actor should provide[4] the `tabActor` to the `TouchSimulator` so it can find other actors.
2. The touch simulator can use code such as:

```
  get _highlighterActor() {
    if (this.tabActor.exited) {
      return Promise.resolve();
    }
    let form = this.tabActor.form();
    let inspector = this.conn.getActor(form.inspectorActor);
    if (!inspector) {
      return Promise.resolve();
    }
    return inspector.getHighlighter();
  },
```

This returns a promise, so you can use yield or await to wait for the highlighter.  We'll probably want to cache the highlighter actor if we do get one, so we don't have to repeat this check for every event.

3. When the touch simulator handles event, it can use the highlighter actor from above to check the `_isPicking` state.

We'll also want to add a test.  Hopefully that helps!

[1]: http://searchfox.org/mozilla-central/source/devtools/server/actors/emulation/touch-simulator.js
[2]: http://searchfox.org/mozilla-central/rev/1285ae3e810e2dbf2652c49ee539e3199fcfe820/devtools/server/actors/highlighters.js#386
[3]: http://searchfox.org/mozilla-central/rev/1285ae3e810e2dbf2652c49ee539e3199fcfe820/devtools/server/actors/highlighters.js#230
[4]: http://searchfox.org/mozilla-central/rev/1285ae3e810e2dbf2652c49ee539e3199fcfe820/devtools/server/actors/emulation.js#31
Flags: needinfo?(jryans)
Reporter

Comment 5

Last year
Does the status change indicate that this bug probably won't be fixed? (Sorry - I am new to Firefox Bugzilla) If so: Is it possible to stop the responsive design mode from re-loading when clicking on the button for touch enabling/disabling? In this way, you could simply click the button every time you want to inspect an element.
(In reply to Martin from comment #5)
> Does the status change indicate that this bug probably won't be fixed?

No, it just means it won't be fixed for version 58.  We'd still like to fix it.

> (Sorry - I am new to Firefox Bugzilla) If so: Is it possible to stop the
> responsive design mode from re-loading when clicking on the button for touch
> enabling/disabling? In this way, you could simply click the button every
> time you want to inspect an element.

I'd like introduce explicit reload control for RDM, which should help with what you're describing here.  I hope to work on this in bug 1428816.
Reporter

Comment 7

Last year
Sounds good - thanks for the explanation!

Updated

Last year
Product: Firefox → DevTools
Whiteboard: [dt-q]
Whiteboard: [dt-q] → [rdm-mvp] [dt-q]
See Also: → 1539636
Assignee

Updated

3 months ago
Duplicate of this bug: 1539636
Assignee

Comment 9

3 months ago

Comment 3 sounds like the general way to go, but there is a complication that might make this harder:
RDM has its own DebuggerServer (that's because it can run without DevTools being open).

So when you open both RDM and DevTools, you really end up with 2 different servers.
When you try to access the inspector actor from RDM's target, you do get one, because the inspector actor is a top-level actor that always exists. But it's just not the same instance as the one the DevTools toolbox also uses.
So there's no way for RDM to know whether we are currently picking an element or not.

I suggest another way:
in DevTools, when the picker is started, get the RDM UI instance via the RDM manager (this is a singleton that's guaranteed to be the right one).
Once we have the object, send a message to its server saying that the picker is started for that particular tab.
So that means introducing new actor methods, but I don't see any other way.

I'll give it a try.

Duplicate of this bug: 1386796
Assignee

Comment 11

3 months ago

My suggestion in comment 9 should work, but it requires accessing the RDM manager from the node picker, and that might be made easier with bug 1539764.

Assignee: nobody → pbrosset
Status: NEW → ASSIGNED
Priority: P2 → P1

(transferring regression info from dupe bug 1539636 comment 6)

Blocks: 1285566
Keywords: regression

Comment 14

3 months ago
Pushed by pbrosset@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5b860d8c3379
Tell RDM when picking to stop simulating touch; r=mtigley,gl

Comment 15

3 months ago
bugherder
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 68
Flags: qe-verify+

Comment 16

12 days ago

This issue is verified as Fixed in Nightly 69.0a1 (2019-06-03).

Updated

12 days ago
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.