Touch simulation isn't turned on when navigating to a different origin
Categories
(DevTools :: Responsive Design Mode, defect, P2)
Tracking
(Not tracked)
People
(Reporter: nchevobbe, Assigned: nchevobbe)
References
(Blocks 2 open bugs)
Details
Attachments
(1 obsolete file)
Steps to reproduce
- Go to https://www.mozilla.org/en-US/
- Open DevTools WebConsole
- Evaluate
document.addEventListener("touchstart", console.log)
- Turn on RDM and touch simulation
- Click in the content page (a
touchstart
log is printed in the console) - In the same tab, navigate to https://twitter.com (with DevTools and RDM still opened)
- Evaluate
document.addEventListener("touchstart", console.log)
in the console again - 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.
Assignee | ||
Comment 1•4 years ago
|
||
there's no need for a new browsing context to be created to trigger this issue, a simple target switch is enough
Comment 2•4 years ago
|
||
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•4 years ago
|
||
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•4 years ago
•
|
||
(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•4 years ago
•
|
||
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•4 years ago
|
||
(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•4 years ago
|
||
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•4 years ago
|
||
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•4 years ago
|
||
Yep, I can see the problem using the steps in comment #8 (m-c build, Win10)
Honza
Updated•4 years ago
|
Updated•4 years ago
|
Comment 10•4 years ago
|
||
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•4 years ago
|
||
Assignee | ||
Updated•4 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Comment 12•3 years ago
|
||
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?
Assignee | ||
Comment 13•3 years ago
|
||
So here that's the opposite, this only works with server side target switching :)
I'm fixing blocking/depending on bugs accordingly
Description
•