1306975 - Add support for container tabs to RDM

Status: VERIFIED FIXED

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

Description

[Xavier ALT [:dex]]

Attachment: browser-console-error.log

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0
Build ID: 20161001030430

Steps to reproduce:

- Open a container tab (I choose "Work")
- Go to mozilla.org
- Switch to responsive design mode (Ctrl-Shift-M)


Actual results:

Page got blank


Expected results:

Responsive design mode UI should have appeared

Comment 1

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Thanks for the report!

Comment 2

[Kamil Jozwiak [:kjozwiak]]

I ran through mozregression three different times which pointed to bug#1305376 as the culprit. Cameron, mind taking a quick look?

13:14.29 INFO: Last good revision: f6c5daab390c0772cd67dc624359e50ba539c6b4
13:14.29 INFO: First bad revision: e4e13b10667c36eeb0d24ad09f37851504977208
13:14.29 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=f6c5daab390c0772cd67dc624359e50ba539c6b4&tochange=e4e13b10667c36eeb0d24ad09f37851504977208

13:16.50 INFO: Looks like the following bug has the changes which introduced the regression:
https://bugzilla.mozilla.org/show_bug.cgi?id=1305376

Comment 3

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to Kamil Jozwiak [:kjozwiak] from comment #2)
> I ran through mozregression three different times which pointed to
> bug#1305376 as the culprit. Cameron, mind taking a quick look?
> 
> 13:14.29 INFO: Last good revision: f6c5daab390c0772cd67dc624359e50ba539c6b4
> 13:14.29 INFO: First bad revision: e4e13b10667c36eeb0d24ad09f37851504977208
> 13:14.29 INFO: Pushlog:
> https://hg.mozilla.org/integration/autoland/
> pushloghtml?fromchange=f6c5daab390c0772cd67dc624359e50ba539c6b4&tochange=e4e1
> 3b10667c36eeb0d24ad09f37851504977208
> 
> 13:16.50 INFO: Looks like the following bug has the changes which introduced
> the regression:
> https://bugzilla.mozilla.org/show_bug.cgi?id=1305376

I don't think bug 1305376 is responsible for this.  It just so happens that the changeset just prior to that bug is for bug 1296498 where we first enabled the new RDM redesign in Nightly only.

If I test an earlier Nightly build like 20160915 and manually enable new RDM by flipping "devtools.responsive.html.enabled" to true first, the problem is still present.

My assumption is that this is a bug in new RDM that needs to account for user context somewhere.

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

ni? me to investigate complexity here.

Comment 5

[Kamil Jozwiak [:kjozwiak]]

Tanvi, as per our conversation on IRC,  I went through the STR mentioned in comment#0 and I couldn't reproduce the problem while using Private Browsing.

Platforms:
* macOS 10.12 x64 - PASSED
* Win 10 x64 VM - PASSED
* Ubuntu 16.04 x64 VM - PASSED

Comment 6

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

From my own testing, I agree with comment 5 that everything appears to behave correctly with RDM and Private Browsing.

As for container tabs, it's more complex, as I sort of suspected.  The main issue is that RDM uses swapFrameLoaders to move content between browsers.  Additionally, it also uses <iframe mozbrowser> to hold the content.  swapFrameLoaders fails when various properties of the source and destination frames don't match, and one of those is origin attributes (as I am sure you're well aware, since I've seen other bugs patching this code path for user context support).

So, to support container tabs properly, something like the following needs to be done:

1. Define what it means for <iframe mozbrowser> to have a user context and implement
  * Should it use the same attribute as <xul:browser> or something else?
2. Change RDM to set the proper attribute on the new frame before swapping content in

It's unclear to me whether all that will get done inside the 52 cycle that new RDM intends to ride the trains.  I'll convert this to a meta-bug to track the various pieces, and let's tentatively track this as blocking new RDM.  As a backup plan if this is too much to do in 52, we can add a temporary error message to RDM for container tabs to stop opening and avoid breaking the tab.

Comment 7

[Tanvi Vyas[:tanvi]]

Note that Containers will be enabled in Firefox 50+ with Test Pilot.  So if this crash issue isn't fixed, release users may experience it.

Comment 8

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Let's play it safe here.  I'll add an error notification for now in bug 1311358, so at least the tab doesn't break.  Then we can add real support here without the time pressure.

Comment 9

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Bug 1306975 - Add error notification for RDM with container tabs.

Review commit: https://reviewboard.mozilla.org/r/86902/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/86902/

Comment 10

[Firefox Bug Husbandry Bot]

Bulk change per https://bugzilla.mozilla.org/show_bug.cgi?id=1401020

Comment 12

[kud]

Hello. I'd love to see it work too.

My use case is this one: 

I have the website of my company which I develop. I would like to visit it in two ways:
- classic user
- dev user

For the dev user, I'd like to have a neutral environment, so I use containers for that. However RDM doesn't work with containers. So I had to switch like: I use "home" container for watching the website as user, and no container for dev this website. It's not the best way because as I don't have containers when I dev on it, I've got all my browser history with me when I visit it. But for the moment it works.

So I'll be really glad to be able to have all dev features when I use a container. :)

Comment 13

[github]

I'm a developer, and this would be very convenient.

Comment 14

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Bug 1306975 - Support containers in RDM.

This adds support for container tabs / contextual identity in Responsive Design
Mode.  Tabs in non-default contexts can now be opened in RDM just like regular
tabs.

Review commit: https://reviewboard.mozilla.org/r/250376/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/250376/

Comment 15

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f2e9e65adbcef9ed04508af5c46ff29649eeba42

Comment 16

[Gabriel Luong [:gl] (ΦωΦ)]

Comment on attachment 8984517 [details]
Bug 1306975 - Support containers in RDM.

https://reviewboard.mozilla.org/r/250376/#review256618

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e9a2d7e57f69
Support containers in RDM. r=gl

Comment 18

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/e9a2d7e57f69

Comment 19

[Timea Cernea [:tbabos][inactive]]

I can confirm that the issue no longer occurs on the latest Nightly 62.0a1 (2018-06-11) - 20180611220254 - across Windows 10 x64, Ubuntu 16.04 and MacOS High Sierra 10.13 platforms.
Responsive design mode works as expected in container tabs.

Comment 20

[Chris Mills [:cmills]]

Note added to Firefox 62 rel notes:
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/62#Developer_tools