regression: view-source is not shown in the address bar and links don't open

VERIFIED FIXED in Firefox 56

Status

()

defect
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: darkspirit, Assigned: mayhemer)

Tracking

({nightly-community, regression})

Trunk
mozilla57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox55 unaffected, firefox56+ verified, firefox57 verified)

Details

Attachments

(3 attachments)

Comment hidden (obsolete)
I can reproduce this bug in Nightly 57 x64 20170807113452 de_DE @ Windows 7 x64, too.
OS: Linux → All
Hardware: x86_64 → All
> Nightly 57 x64 20170807113452 @ Debian Testing (with Stylo enabled & disabled)
Same build, a day later: WFM (wtf)
> I can reproduce this bug in Nightly 57 x64 20170807113452 de_DE @ Windows 7 x64, too.
Still broken (in a new profile).

There is a ghost under the hood.
> > I can reproduce this bug in Nightly 57 x64 20170807113452 de_DE @ Windows 7 x64, too.
> Still broken (in a new profile).
On https://bugzilla.mozilla.org it works as it should. Maybe a history or HSTS/HSTS Preloading thing? I will dig further.
Reporter

Comment 4

2 years ago
str
Nightly 57 x64 20170808114032 @ Debian Testing

This only happens if you typed in a domain without a protocol, pressed enter and Ctrl+U.

STR (see attached video):
1. Type in fastaim.de, press enter (or confirm the first Awesomebar result "fastaim.de -- Visit") 
2. press Ctrl+U: view-source: is missing and links are unclickable
3. Close these tabs.
4. Type in "fastaim.de", but select the second Awesomebar result, e.g. "https://fastaim.de/", press enter and Ctrl+U: everything is fine
> This only happens if you typed in a domain without a protocol, pressed enter and Ctrl+U.

Also if you reload a website which was opened on this way and then press Ctrl+U you will get this bug.
(attachment 8894666 [details] from comment 0 was about that. Sorry for the unprecise STR there).
I had some fun with https://download-origin.cdn.mozilla.net/pub/firefox/nightly/2017/07/

I renamed the filenames after testing:

firefox-45.0a1.de.linux-x86_64_unaffected.tar.bz2
firefox-53.0a2.de.linux-x86_64_unaffected.tar.bz2
firefox-56.0a1.de.linux-x86_64_2017-06-30-10-02-34_unaffected.tar.bz2
firefox-56.0a1.de.linux-x86_64_2017-07-18-16-42-11_unaffected.tar.bz2
firefox-56.0a1.de.linux-x86_64_2017-07-24-10-03-04_unaffected.tar.bz2
firefox-56.0a1.de.linux-x86_64_2017-07-26-10-02-41_unaffected.tar.bz2
firefox-56.0a1.de.linux-x86_64_2017-07-27-16-25-31_unaffected.tar.bz2
firefox-56.0a1.de.linux-x86_64_2017-07-28-10-03-58_unaffected.tar.bz2
firefox-56.0a1.de.linux-x86_64_2017-07-29-10-02-5_unaffected.tar.bz2
firefox-56.0a1.de.linux-x86_64_2017-07-30-10-03-07_affected.tar.bz2 <-------- de (first affected build)
firefox-56.0a1.en-US.linux-x86_64_2017-07-30-10-03-07_affected.tar.bz2 <------- en-US

https://www.mozilla.org/de/firefox/beta/all/
firefox-55.0b13_unaffected.tar.bz2 (German Linux x64)
Beta 56 is not out yet.

So I went here: https://download-origin.cdn.mozilla.net/pub/firefox/candidates/56.0b1-candidates/build5/linux-x86_64/de/
* affected *
Has Regression Range: --- → yes
Has STR: --- → yes
Thanks for chasing it down this far!

Have you used mozregression[1] before?  That should give us a very precise window, down to just a few changesets most likely.

[1]: https://mozilla.github.io/mozregression/quickstart.html
Flags: needinfo?(bugzilla)
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #7)
> Thanks for chasing it down this far!
> 
> Have you used mozregression[1] before?  That should give us a very precise window, down to just a few changesets most likely.

Amazing tool!
mozregression --good 2017-07-28 --bad 2017-07-31
[...]
 9:07.06 INFO: Last good revision: dd3e2939065e8c6805b1994583d5a04f39c8ab7c
 9:07.06 INFO: First bad revision: 00167e9fe0c0fc573801eb8a905eb3822290c2da
 9:07.06 INFO: Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=dd3e2939065e8c6805b1994583d5a04f39c8ab7c&tochange=00167e9fe0c0fc573801eb8a905eb3822290c2da
Flags: needinfo?(bugzilla)
Thanks for the regression window!

:mayhemer, from the regression log above, it sounds like your patch might be involved here.

Can you please take a look?
Blocks: 1380012
Flags: needinfo?(honzab.moz)
Severity: major → normal
Assignee

Updated

2 years ago
Assignee: nobody → honzab.moz
Flags: needinfo?(honzab.moz)
Assignee

Comment 10

2 years ago
Posted patch v1Splinter Review
This looks like a proper fix.  nsDocShell::LoadPage is dedicated to showing the view-source: of the page.  It replaces the URI and drops the originalURI from the cloned session history entry (a session history entry clone from nsDocShell::GetCurrentDescriptor called from ViewSourceContent.loadSource).

Interesting is that nsDocShell::LoadPage has never been dropping loadReplace from the entry as well - probably a long standing bug.
Attachment #8895486 - Flags: review?(bzbarsky)
Comment on attachment 8895486 [details] [diff] [review]
v1

r=me
Attachment #8895486 - Flags: review?(bzbarsky) → review+
Assignee

Updated

2 years ago
Status: NEW → ASSIGNED
Keywords: checkin-needed
[Tracking Requested - why for this release]:
Regression in View Source functionality, let's be sure to uplift.
Assignee

Comment 13

2 years ago
Comment on attachment 8895486 [details] [diff] [review]
v1

Approval Request Comment
[Feature/Bug causing the regression]: 1380012
[User impact if declined]: as this bug title says
[Is this code covered by automated tests?]: apparently not :)
[Has the fix been verified in Nightly?]: only locally so far
[Needs manual test from QE? If yes, steps to reproduce]: yes, see comment 4 for str
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: the bug is isolated to view-source and the fix is also in a code that is dedicated to view-source only, while the cause is clear and well understood
[String changes made/needed]: none
Attachment #8895486 - Flags: approval-mozilla-beta?
Comment on attachment 8895486 [details] [diff] [review]
v1

We definitely want view source to work! Please uplift for 56 beta 2.
Attachment #8895486 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment on attachment 8895486 [details] [diff] [review]
v1

Correction, let's uplift this fix after we land it on m-c. It may still make it to beta 2.
Attachment #8895486 - Flags: approval-mozilla-beta+ → approval-mozilla-beta?

Comment 16

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/164240dd108d
Drop result principal URI from a cloned session history entry when opening view-source: page. r=bz
Keywords: checkin-needed

Comment 17

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/164240dd108d
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Comment on attachment 8895486 [details] [diff] [review]
v1

Fix a regression. Beta56+. Should be in 56.0b3.
Attachment #8895486 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
I've reproduced this issue on an affected Nightly build using STR from comment 4.

This is verified fixed on latest Nightly 57.0a1 (2017-08-22) and 56 Beta 6 (20170821193225) under Windows 10 x64, Mac OS X 10.11.6 and Ubuntu 16.04 x64 LTS.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.