Open Bug 1704029 Opened 4 years ago Updated 2 years ago

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

Categories

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

defect

Tracking

(Not tracked)

ASSIGNED

People

(Reporter: nchevobbe, Assigned: nchevobbe)

References

(Blocks 2 open bugs)

Details

Attachments

(1 obsolete file)

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.

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

Summary: Touch simulation isn't turned on when navigating to a page that forces a new browsing context → Touch simulation isn't turned on when navigating to a different origin

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?

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.

(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)

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

(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.

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.

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?

Flags: needinfo?(odvarko)

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

Honza

Flags: needinfo?(odvarko)
Fission Milestone: --- → M8
Whiteboard: dt-fission-m3-triage → dt-fission-m3-mvp
Severity: -- → S3
Priority: -- → P2

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.

Blocks: 1698891
Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED
Attachment #9220548 - Attachment is obsolete: true
Fission Milestone: M8 → ---
Whiteboard: dt-fission-m3-mvp

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?

Flags: needinfo?(nchevobbe)

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

No longer blocks: 1698891
Depends on: 1698891
Flags: needinfo?(nchevobbe)
Blocks: rdm-touch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: