View source open an blank page when using responsive design view

VERIFIED FIXED in Firefox 52

Status

()

Firefox
Developer Tools: Responsive Design Mode
P1
normal
VERIFIED FIXED
a year ago
a year ago

People

(Reporter: John O, Assigned: jryans)

Tracking

({regression})

52 Branch
Firefox 53
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox50 unaffected, firefox51 unaffected, firefox52+ verified, firefox53+ verified)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments)

(Reporter)

Description

a year ago
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

a year ago
This issue present if devtools.responsive.html.enabled=true.
Blocks: 1308297
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

Comment 2

a year ago
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
status-firefox50: --- → unaffected
status-firefox51: --- → unaffected
status-firefox52: --- → affected
status-firefox53: --- → affected
tracking-firefox52: --- → ?
tracking-firefox53: --- → ?
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)

Comment 5

a year ago
(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 hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 10

a year ago
mozreview-review
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 11

a year ago
mozreview-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+
(Assignee)

Comment 12

a year ago
mozreview-review-reply
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.
(Assignee)

Updated

a year ago
Blocks: 1330403
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 15

a year ago
mozreview-review
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+
(Assignee)

Updated

a year ago
Flags: qe-verify+
QA Contact: mihai.boldan

Comment 17

a year ago
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

Updated

a year ago
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?

Comment 19

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/48136ce8a357
https://hg.mozilla.org/mozilla-central/rev/0de2f57ed353
https://hg.mozilla.org/mozilla-central/rev/bdfebb742afe
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox53: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53

Comment 20

a year ago
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
tracking-firefox52: ? → +
tracking-firefox53: ? → +
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.
(Assignee)

Updated

a year ago
No longer blocks: 1308297

Comment 24

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/339271ebe1d7
status-firefox52: affected → fixed
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]
(Reporter)

Comment 26

a year ago
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: → bug 1333914

Comment 28

a year ago
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
status-firefox52: fixed → verified
status-firefox53: fixed → verified
Flags: qe-verify+
See Also: → bug 1341199
You need to log in before you can comment on or make changes to this bug.