1331601 - View source open an blank page when using responsive design view

Status: VERIFIED FIXED

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

Description

[u424271]

Steps to reproduce:
1) Ctrl+Shift+M to open responsive design view
2) Ctrl+Shift+M to exit responsive design view
3) Ctrl+U to open page source

Actual results
A blank page (completely white) opened in a new tab

Expected results
The page source should have opened in a new tab

Versions tested
53.0a1 (2017-01-16) (64-bit) [issue present]
51.0b14 (64-bit) [issue NOT present]
50.1.0 [issue NOT present]

Comment 1

[YF (Yang)]

This issue present if devtools.responsive.html.enabled=true.

Comment 2

[Loic]

It's related to the new RDM (available only with e10s).

There are 2 issues:

1) If you repeat STR from comment #0, Ctrl+U to view source doesn't work after the 1st time (no tab open).
It's due to:
J. Ryan Stinnett — Bug 1296498 - Enable new RDM UI for Nightly. r=ntim

2) The current issue, Ctrl+U opens a tab now but it's blank, no source displayed.
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=ccee8b226c3b318b901eecc916152cce9dda89df&tochange=3360b8d2f04bc8439c101c3fa892eecd7465e073
It's due to:
Michael Layzell — Bug 1318767 - Part 2: Correctly swap web progress listeners when swapping frameloaders, r=dao

Comment 3

[Jim Mathies [:jimm]]

(In reply to Loic from comment #2)
> It's related to the new RDM (available only with e10s).
> 
> There are 2 issues:
> 
> 1) If you repeat STR from comment #0, Ctrl+U to view source doesn't work
> after the 1st time (no tab open).
> It's due to:
> J. Ryan Stinnett — Bug 1296498 - Enable new RDM UI for Nightly. r=ntim
> 
> 2) The current issue, Ctrl+U opens a tab now but it's blank, no source
> displayed.
> https://hg.mozilla.org/integration/mozilla-inbound/
> pushloghtml?fromchange=ccee8b226c3b318b901eecc916152cce9dda89df&tochange=3360
> b8d2f04bc8439c101c3fa892eecd7465e073
> It's due to:
> Michael Layzell — Bug 1318767 - Part 2: Correctly swap web progress
> listeners when swapping frameloaders, r=dao

Can we break this up into two bugs?

Comment 4

[Jim Mathies [:jimm]]

jryans, have you looked at the first issue here? It's in 52 which is merging to beta next week.

Comment 5

[Loic]

(In reply to Jim Mathies [:jimm] from comment #3)
> (In reply to Loic from comment #2)
> > It's related to the new RDM (available only with e10s).
> > 
> > There are 2 issues:
> > 
> > 1) If you repeat STR from comment #0, Ctrl+U to view source doesn't work
> > after the 1st time (no tab open).
> > It's due to:
> > J. Ryan Stinnett — Bug 1296498 - Enable new RDM UI for Nightly. r=ntim
> > 
> > 2) The current issue, Ctrl+U opens a tab now but it's blank, no source
> > displayed.
> > https://hg.mozilla.org/integration/mozilla-inbound/
> > pushloghtml?fromchange=ccee8b226c3b318b901eecc916152cce9dda89df&tochange=3360
> > b8d2f04bc8439c101c3fa892eecd7465e073
> > It's due to:
> > Michael Layzell — Bug 1318767 - Part 2: Correctly swap web progress
> > listeners when swapping frameloaders, r=dao
> 
> Can we break this up into two bugs?

There is no need, my comment was just an "history" of the issue, behavior after #2 has "replaced" the behavior after #1, so there is only the 2nd issue to fix now (Ctrl+U opens a blank tab, even after cycling Ctrl+M then Ctrl+U)

Comment 6

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

(In reply to Jim Mathies [:jimm] from comment #4)
> jryans, have you looked at the first issue here? It's in 52 which is merging
> to beta next week.

I am investigating now.  Not sure if they will be a fix before 52 is in beta, but we'll try our best.

Comment 7

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

To clarify, 52 has behavior 1 from comment 2 (only first time works) while 53 has behavior 2 (tabs keep opening, but always blank).

Comment 8

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

Attachment: Bug 1331601 - Restore remoteType on RDM close.

In bug 1318767, `updateBrowserRemoteness` was changed to take the `remoteType`
as part of an options argument, but the RDM call site wasn't updated.

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

Comment 9

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

Attachment: Bug 1331601 - Copy tab listener state flags in RDM.

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

Comment 10

[Alexandre Poirot [:ochameau]]

Comment on attachment 8828451 [details]
Bug 1331601 - Restore remoteType on RDM close.

https://reviewboard.mozilla.org/r/105852/#review106836

Comment 11

[Alexandre Poirot [:ochameau]]

Comment on attachment 8828452 [details]
Bug 1331601 - Copy tab listener state flags in RDM.

https://reviewboard.mozilla.org/r/105854/#review106834

Could you put in commit extra message the reason why you did that?
What did it fixed?

::: devtools/client/responsive.html/browser/swap.js:160
(Diff revision 1)
>        innerBrowser = null;
>  
> +      // Copy tab listener state flags to content tab.  Each tab gets its own tab
> +      // listener and state flags which cache document loading progress.  The state flags
> +      // are checked when switching tabs to update the browser UI.  The later step of
> +      // `swapBrowsersAndCloseOther` will fold the state back into the main tab.

It may be better to refer to the first doc rather than copy/pasting it here. And just keep the first sentence here.

Comment 12

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

Comment on attachment 8828452 [details]
Bug 1331601 - Copy tab listener state flags in RDM.

https://reviewboard.mozilla.org/r/105854/#review106834

Yep, I'll add more notes.

The core issue was that, without this, the tab would have mStateFlags == 0.  tabbrowser.xml only triggers the browser UI to updated on switching tabs if this field has some non-zero value.  I am not exactly sure _why_ this check is present...

But for this bug about view source, the tab switch handler is where the disabled / enabled state of the view source command is updated.  Not updating it appropriately was what prevented view source from being opened.

> It may be better to refer to the first doc rather than copy/pasting it here. And just keep the first sentence here.

Makes sense, I added a short note to refer to the previous copy above for more details.

Comment 13

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

Comment on attachment 8828452 [details]
Bug 1331601 - Copy tab listener state flags in RDM.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/105854/diff/1-2/

Comment 14

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

Attachment: Bug 1331601 - RDM browser_device_change.js fix for subpixel size.

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

Comment 15

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

Comment on attachment 8828552 [details]
Bug 1331601 - RDM browser_device_change.js fix for subpixel size.

https://reviewboard.mozilla.org/r/105904/#review106844

Comment 16

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

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

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/48136ce8a357
Restore remoteType on RDM close. r=ochameau
https://hg.mozilla.org/integration/autoland/rev/0de2f57ed353
Copy tab listener state flags in RDM. r=ochameau
https://hg.mozilla.org/integration/autoland/rev/bdfebb742afe
RDM browser_device_change.js fix for subpixel size. r=jryans

Comment 18

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

Comment on attachment 8828452 [details]
Bug 1331601 - Copy tab listener state flags in RDM.

Approval Request Comment
[Feature/Bug causing the regression]: Bug exists since new RDM enabled in 52
[User impact if declined]: If declined, view source will only open one time after opening and closing RDM in a tab.
[Is this code covered by automated tests?]: No, manually tested.
[Has the fix been verified in Nightly?]: Not yet, waiting to land
[Needs manual test from QE? If yes, steps to reproduce]: Doesn't need to block uplift, but QE is welcome to double check.
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Only affects tab state when using RDM
[String changes made/needed]: None

Comment 19

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/48136ce8a357
https://hg.mozilla.org/mozilla-central/rev/0de2f57ed353
https://hg.mozilla.org/mozilla-central/rev/bdfebb742afe

Comment 20

[Sajedul Islam]

I have reproduced this bug with Nightly 53.0a1 (2017-01-17) on Windows 8.1 , 64 Bit ! 

Build   ID    20170117030218
User Agent    Mozilla/5.0 (Windows NT 6.3; WOW64; rv:53.0) Gecko/20100101 Firefox/53.0

This bug's fix is Verified with latest Nightly !

Build   ID    20170122030212
User Agent    Mozilla/5.0 (Windows NT 6.3; WOW64; rv:53.0) Gecko/20100101 Firefox/53.0

[bugday-20170118]

Comment 21

[Julien Cristau [:jcristau] (back April 22)]

Tracking new RDM issue for 52/53

Comment 22

[Julien Cristau [:jcristau] (back April 22)]

Comment on attachment 8828452 [details]
Bug 1331601 - Copy tab listener state flags in RDM.

devtools RDM fix for beta52.

We only need this one patch on 52, not the other two, right?

Comment 23

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

(In reply to Julien Cristau [:jcristau] from comment #22)
> Comment on attachment 8828452 [details]
> Bug 1331601 - Copy tab listener state flags in RDM.
> 
> devtools RDM fix for beta52.
> 
> We only need this one patch on 52, not the other two, right?

Correct, only this patch is needed in 52.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/339271ebe1d7

Comment 25

[Mohammad Maruf Rahman]

I have reproduced this bug with Nightly 53.0a1 (2017-01-17) (64-bit) on Windows 7 , 64 Bit !

This bug's fix is verified with latest Beta!

Build   ID   20170124094647
User Agent   Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:52.0) Gecko/20100101 Firefox/52.0

[bugday-20170125]

Comment 26

[u424271]

The initial issue as reported is fixed. However related issues still remain in the latest Nightly build.
[54.0a1 (2017-01-24) (64-bit)]

Steps to reproduce:
1) Open 2 different webpages in 2 tabs
2) Put each tab into RDM
3) View source on Tab1
4) View source on Tab2

Expected behaviour:
3) Tab1 page source displayed
4) Tab2 page source displayed

Actual behaviour:
3) Tab1 page source IS displayed
4) Blank page is displayed

Other notes
After exiting RDM in Tab2, view source does work.
Going back to Tab1 (still in RDM). View source then does not work (blank page displayed)

Comment 27

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

(In reply to John O from comment #26)
> The initial issue as reported is fixed. However related issues still remain
> in the latest Nightly build.
> [54.0a1 (2017-01-24) (64-bit)]
> 
> Steps to reproduce:
> 1) Open 2 different webpages in 2 tabs
> 2) Put each tab into RDM
> 3) View source on Tab1
> 4) View source on Tab2
> 
> Expected behaviour:
> 3) Tab1 page source displayed
> 4) Tab2 page source displayed
> 
> Actual behaviour:
> 3) Tab1 page source IS displayed
> 4) Blank page is displayed
> 
> Other notes
> After exiting RDM in Tab2, view source does work.
> Going back to Tab1 (still in RDM). View source then does not work (blank
> page displayed)

This is a separate issue, so I've filed bug 1333914 to track this.

Comment 28

[Timea Zsoldos [:zstimi/tzsoldos], Desktop QA]

I have reproduced this issue using Firefox 53.0a1 (2017.01.17) on Win 8.1 x64.
I can confirm this issue is fixed, I verified using Firefox 52.0b2 and Firefox 53.0a2 (2017.01.30) on Win 8.1 x64, Ubuntu 16.04 x64 and Mac OS X 10.11.