1704029 - Touch simulation isn't turned on when navigating to a different origin

Status: ASSIGNED

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

Description

[Nicolas Chevobbe [:nchevobbe]]

Steps to reproduce

1. Go to https://www.mozilla.org/en-US/
2. Open DevTools WebConsole
3. Evaluate document.addEventListener("touchstart", console.log)
4. Turn on RDM and touch simulation
5. Click in the content page (a touchstart log is printed in the console)
6. In the same tab, navigate to https://twitter.com (with DevTools and RDM still opened)
7. Evaluate document.addEventListener("touchstart", console.log) in the console again
8. Click in the content page

Expected results

A touchstart log is printed in the console again

Actual results

We don't get the log displayed, which indicates that the event isn't fired.

---

Note that the touchEventsOverride flag is set properly on the page what's missing here is starting the touch simulator again when a new browsing context is created.

A test will be added in Bug 1703178 with disabled assertions that should be re-enabled when this bug is fixed.

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

there's no need for a new browsing context to be created to trigger this issue, a simple target switch is enough

Comment 2

[Jan Honza Odvarko [:Honza] (always need-info? me)]

I was testing this on Win 10 + m-c local build and I can't reproduce it

I tried

• No fission
• Fission enabled but devtools.testing.enableServerWatcherSupport => false
• Fission enabled + all DevTools prefs => true

What am I doing wrong?

Comment 3

[Julian Descottes [:jdescottes]]

I was quickly looking at this and onTargetAvailable attempts to restore the actorState (which should restart touch simulator, among other things)

  async onTargetAvailable({ targetFront }) {
    if (this.destroying) {
      return;
    }

    if (targetFront.isTopLevel) {
      this.responsiveFront = await targetFront.getFront("responsive");
      await this.restoreActorState();
    }
  }

But in restoreActorState, we bail out early at

    const hasDeviceState = await this.hasDeviceState();
    if (hasDeviceState) {
      // Return if there is a device state to restore, this will be done when the
      // device list is loaded after the post-init.
      return;
    }

But the post-init event this refers to is only emitted for the initial load of the UI. So when we are really target switching (and not initializing the UI), we should maybe bypass this if (hasDeviceState). Although the initialization sequence doesn't seem completely clear to me at the moment.

Comment 4

[Julian Descottes [:jdescottes]]

(For the record, I can reproduce on current m-c, and reading the code, I don't see how this could work, even on a different platform)

Comment 5

[Julian Descottes [:jdescottes]]

Following WIP fixes it for me: https://hg.mozilla.org/try/rev/87fb87d43a6add58ca6cadfacce0563dfed3e67f

EDIT: See my last comments on https://phabricator.services.mozilla.com/D111262 We might not want to fix that in RDM ui and instead fix it in the command directly. Might lead to remove some code currently performed on RDM ui target-switching

Comment 6

[Julian Descottes [:jdescottes]]

(In reply to Jan Honza Odvarko [:Honza] (always need-info? me) from comment #2)

I was testing this on Win 10 + m-c local build and I can't reproduce it

What am I doing wrong?

Oh I think you have to interact with the device list first! Otherwise the if (hasDeviceState) bail doesn't come into play here.

Comment 7

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Ok, I was able to reproduce that now (adding interacting with the device list into STRs)
But, note that I had to disable Fission to see the issue.

Comment 8

[Julian Descottes [:jdescottes]]

Interesting, I can reproduce with Fission enabled, clean profile.
Note that without fission, navigating to twitter still triggers a target switch, because of the headers used by twitter.

My exact STRs (on central, with a clean profile)

• enable fission
• start firefox
• Go to https://www.mozilla.org/en-US/
• open devtools
• enable RDM
• select iPad in device list
• (touch simulation should get enabled)
• dismiss the RDM banner telling you to reload
• reload the page
• evaluate document.addEventListener("touchstart", console.log)
• click -> event is visible in the console
• navigate to https://twitter.com
• evaluate document.addEventListener("touchstart", console.log)
• click

ER: event should be logged
AR: nothing is logged

Honza: do you mind testing with those steps?

Comment 9

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Yep, I can see the problem using the steps in comment #8 (m-c build, Win10)

Honza

Comment 10

[Alexandre Poirot [:ochameau]]

It is much easier to reproduce once you apply bug 1686748's patch.
This break on simple reload and browser_target_configuration_command_touch_events.js starts failing.

I'll attach a patch to just make try green, but it would be best to actually fix this issue.
Having such patch helps me see the other tests that are failing... and undercover all necessary changes.

Comment 11

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1704029 - Ignore test failure due to missing early touch events when having server side target switching

Comment 12

[Alexandre Poirot [:ochameau]]

Sounds like this has been fixed if I look at:
https://searchfox.org/mozilla-central/source/devtools/shared/commands/target-configuration/tests/browser_target_configuration_command_touch_events.js#20-22

If not, may be that's something to address before enabling server targets?

Comment 13

[Nicolas Chevobbe [:nchevobbe]]

So here that's the opposite, this only works with server side target switching :)
I'm fixing blocking/depending on bugs accordingly