Closed Bug 1757458 Opened 3 years ago Closed 3 years ago

[fission] Undo-close-tab strips off `view-source:` from view-source URLs, and loads the actual page (instead of showing the source), if there's a redirect involved

Categories

(Core :: DOM: Navigation, defect, P3)

defect

Tracking

()

RESOLVED FIXED
100 Branch
Tracking Status
firefox-esr91 --- disabled
firefox97 --- wontfix
firefox98 --- wontfix
firefox99 --- wontfix
firefox100 --- fixed

People

(Reporter: dholbert, Assigned: smaug)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(2 files)

STR:

  1. Start Firefox with a fresh profile (for good measure)
  2. Open two tabs in your Firefox window: one at https://www.example.org/ and one at view-source:https://support.mozilla.org
  3. Close the view-source tab.
  4. Ctrl+Shift+T to undo that close-tab operation.

EXPECTED RESULTS:
The restored tab should be restored with the view-source: part of the URL intact, and it should show me the source just as it did before I closed it.

ACTUAL RESULTS:
The tab is restored with the view-source: prefix stripped off of the URL, so that I end up looking at the actual page itself rather than its source.

(Note: I ran into this when investigating a testcase that crashes Firefox when loaded directly. I was viewing the source, and then closed the tab, and then did undo-close-tab, and this made Firefox load the testcase and crash.)

mozregression clued me in that this is a fission-specific issue, it started happening by default in Nightly when we preffed on fission by default.

If I explicitly turn on fission for my mozregression run (fission.autostart:true), then I get this regression range:

5:37.90 INFO: Last good revision: d95aab08e480fc45205453c02d9941b7b9cc80e9 (2020-10-13)
5:37.90 INFO: First bad revision: 3a9fcbf00f37714e083b447a059ad543e50eee71 (2020-10-14)
5:37.90 INFO: Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d95aab08e480fc45205453c02d9941b7b9cc80e9&tochange=3a9fcbf00f37714e083b447a059ad543e50eee71

Interestingly: from my testing, this only seems to repro if I explicitly enter the view-source URL into the URL bar (including view-source:).

If I instead perform a View-Source action on an actual tab (e.g. Ctrl U) and then I attempt to reproduce with that view-source tab, then I get expected results.

I think maybe (?) this might specifically reproduce when the URL in question redirects to some other site. This leads me to suspect that maaaaybe this is a regression from (or related to) bug 1668083.

I can reliably reproduce this if the URL that I enter into my URL bar is
view-source:https://support.mozilla.org/
(...and importantly: after I hit Enter, this redirects to view-source:https://support.mozilla.org/en-US/)

If I instead type in the full URL (including the /en-US/ suffix), view-source:https://support.mozilla.org/en-US/, and then I hit enter, then I get EXPECTED RESULTS -- undo-close-tab works properly.

As further confirmation of this theory: I can reproduce this bug using these URLs (which both redirect to their www. subdomain):
view-source:https://yahoo.com
view-source:https://google.com

...but if I include www (avoiding the need for a redirect), then I cannot reproduce anymore.

Summary: [fission] Undo-close-tab strips off `view-source:` from the URL, loading the actual page → [fission] Undo-close-tab strips off `view-source:` from view-source URLs, and loads the actual page (instead of showing the source), if there's a redirect involved

(In reply to Daniel Holbert [:dholbert] from comment #2)

This leads me to suspect that maaaaybe this is a regression from (or related to) bug 1668083.

Marking as such for now (this seems like the most-likely lead from skimming the regression range); changing component as well. Smaug, could you take a look?

Component: Session Restore → DOM: Navigation
Flags: needinfo?(bugs)
Product: Firefox → Core
Regressed by: 1668083

Here's a perhaps-more-scientifically-reliable way to trigger this (less manual typing / room for user-error).

STR:
(1) set about:config pref browser.startup.homepage to this string:

view-source:https://www.yahoo.com|view-source:https://google.com

(2) Open a new window (Ctrl+N)
(3) In the new window: close and then restore the first tab (Yahoo) , by doing Ctrl+W and then Ctrl+Shift+T
(4) Switch to the second tab (Google) and close and restore that one by again doing Ctrl+W, Ctrl+Shift+T

(Note: step 3 is optional, and is just there for comparison)

ACTUAL RESULTS:
Yahoo's source restores successfully (presumably because our view-source URL included the www up-front and didn't require a redirect)
Google's source does not restore successfully; it loads the actual live website.

You can flip the results (e.g. make Yahoo fail and Google succeed) if you adjust the initial URLs (the about:config pref in step 1) to switch which one has www and hence which one needs vs. doesn't-need a redirect.

Attached video screencast of bug, using comment 4 STR (deleted) —
Attachment #9265795 - Attachment description: screencast of bug → screencast of bug, using comment 4 STR

Set release status flags based on info from the regressing bug 1668083

Flags: needinfo?(bugs)
Has Regression Range: --- → yes

@smaug: could you be so kind to set priority and severity of this ticket? :-}

Flags: needinfo?(bugs)
Severity: -- → S3
Flags: needinfo?(bugs)
Priority: -- → P3

Somehow here https://searchfox.org/mozilla-central/rev/15f12b0c6c56b449304f6cb1f84ac4df84dc936a/netwerk/ipc/DocumentLoadListener.cpp#2546-2551 the uri of the mLoadingSessionHistoryInfo->mInfo drops view-source: prefix.

Assignee: nobody → bugs
Attachment #9266893 - Attachment description: WIP: Bug 1757458, replace loading session history entry only right before redirecting load to real channel → Bug 1757458, replace loading session history entry only right before redirecting load to real channel
Pushed by opettay@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/794afc641923 replace loading session history entry only right before redirecting load to real channel r=peterv
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 100 Branch
Flags: in-testsuite+

The patch landed in nightly and beta is affected.
:smaug, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(bugs)

This is super edge case. No need to land to beta.

Flags: needinfo?(bugs)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: