Starting RDM from the toolbox might use the wrong tab target
Categories
(DevTools :: Responsive Design Mode, defect, P3)
Tracking
(firefox87 fixed)
| Tracking | Status | |
|---|---|---|
| firefox87 | --- | fixed |
People
(Reporter: jdescottes, Assigned: jdescottes)
References
Details
Attachments
(4 files)
RDM creates its own tab target in ui.js: https://searchfox.org/mozilla-central/rev/2a24205479519e70c0574929f45730d285141584/devtools/client/responsive/ui.js#398
const descriptor = await this.client.mainRoot.getTab();
const targetFront = await descriptor.getTarget();
We don't pass anything to getTab, so in the end we will use the currently selected tab to create the target. This is usually fine, but can fail in some edge case scenarios. STRs:
- open a first tab on google.com
- open a second tab on mozilla.org
- open devtools for the second tab
- move devtools to a separate window
- select the google.com tab
- focus the devtools window again, without selecting the mozilla.org tab
- enable RDM
At this point, the UI of the mozilla.org tab will be modified to include the RDM ui. But most of the RDM features will actually impact the google.com tab. Eg. taking a screenshot will take a screenshot of the google tab. Changing the user agent will impact the google tab. Etc...
This is probably unlikely to be triggered by a end user, but it would be nice to fix it.
|
Assignee | |
Comment 1•4 years ago
|
||
Will try to fix this and cleanup a bit tab target usage from RDM. This will help for Bug 1644397
|
|
Assignee | |
Updated•4 years ago
|
|
|
Assignee | |
Comment 2•4 years ago
|
||
RDM uses a dedicated client, target-list etc. Which means that we can use a local tab descriptor and remove the target switching logic from RDM.
|
|
Assignee | |
Comment 3•4 years ago
|
||
Depends on D103325
|
|
Assignee | |
Comment 4•4 years ago
|
||
Depends on D103326
Spotted while writing the patch, makes the code unnecessarily hard to navigate.
|
|
Assignee | |
Comment 5•4 years ago
|
||
Depends on D103687
Pushed by jdescottes@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b96b289caa90 [devtools] Update RDM tab target creation to rely on a local tab target descriptor r=nchevobbe https://hg.mozilla.org/integration/autoland/rev/2101550772b1 [devtools] RDM should stop watching resources and targets after performing cleanup r=nchevobbe https://hg.mozilla.org/integration/autoland/rev/b80ef0302895 [devtools] Remove string concatenation to toggle RDM from toolbox r=nchevobbe https://hg.mozilla.org/integration/autoland/rev/822bc5a120c7 [devtools] Add test for opening RDM on non-selected tab r=nchevobbe
|
||
Comment 7•4 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/b96b289caa90
https://hg.mozilla.org/mozilla-central/rev/2101550772b1
https://hg.mozilla.org/mozilla-central/rev/b80ef0302895
https://hg.mozilla.org/mozilla-central/rev/822bc5a120c7
Description
•