Closed Bug 1331601 Opened 3 years ago Closed 3 years ago

View source open an blank page when using responsive design view

Categories

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

52 Branch
defect

Tracking

(Not tracked)

VERIFIED FIXED
Firefox 53

People

(Reporter: mozillabugs, Assigned: jryans)

References

Details

(Keywords: regression)

Attachments

(3 files)

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]
This issue present if devtools.responsive.html.enabled=true.
Blocks: rdm-top
Severity: normal → minor
Status: UNCONFIRMED → NEW
Has STR: --- → yes
Component: Untriaged → Developer Tools: Responsive Design Mode
Ever confirmed: true
Keywords: regression
Summary: View source opens blank page after using responsive design view → View source open an blank page when using responsive design view
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
Blocks: 1318767, 1296498
OS: Unspecified → All
Hardware: Unspecified → All
Version: 53 Branch → 52 Branch
(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?
Flags: needinfo?(epinal99-bugzilla2)
jryans, have you looked at the first issue here? It's in 52 which is merging to beta next week.
Flags: needinfo?(jryans)
(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)
Flags: needinfo?(epinal99-bugzilla2)
(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.
Assignee: nobody → jryans
Severity: minor → normal
Status: NEW → ASSIGNED
Flags: needinfo?(jryans)
Priority: -- → P1
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 on attachment 8828451 [details]
Bug 1331601 - Restore remoteType on RDM close.

https://reviewboard.mozilla.org/r/105852/#review106836
Attachment #8828451 - Flags: review?(poirot.alex) → review+
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.
Attachment #8828452 - Flags: review?(poirot.alex) → review+
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.
Blocks: 1330403
Comment on attachment 8828552 [details]
Bug 1331601 - RDM browser_device_change.js fix for subpixel size.

https://reviewboard.mozilla.org/r/105904/#review106844
Attachment #8828552 - Flags: review+
Flags: qe-verify+
QA Contact: mihai.boldan
Pushed by jryans@gmail.com:
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
Blocks: 1317293
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
Attachment #8828452 - Flags: approval-mozilla-aurora?
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]
Tracking new RDM issue for 52/53
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?
Attachment #8828452 - Flags: approval-mozilla-aurora? → approval-mozilla-beta+
(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.
No longer blocks: rdm-top
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]
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)
(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.
See Also: → 1333914
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.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
See Also: → 1341199
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.