1213267 - view source tab on some pages does not include view-source: prefix until reloaded

Status: VERIFIED FIXED

(Toolkit :: View Source, defect)

Description

[Nathan Froyd [:froydnj]]

STR:

1. Open a web page; I used https://taco-spolsky.github.io/
2. View its source.

Expected result:

New tab has view-source:https://taco-spolsky.github.io/ in the address bar.

Actual result:

New tab has https://taco-spolsky.github.io/ in the address bar.

Reloading (Ctrl-R) the new tab fixes the URL in the address bar.

This doesn't seem to work with all pages; visiting http://www.tedunangst.com/flak/post/hoarding-and-reuse and viewing its source correctly displays view-source:http://...  I'm guessing this has something to do with how view-source is interacting with the caching layer?

Comment 1

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

From mozregression's set of suspects:

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=c55a9670de0f69b5b964bba11e8fd0850ac017d9&tochange=b50322abc2863332513aa05e5e53ab3d8278672d

I am going to guess it's:

Dragana Damjanovic — Bug 1185256 - Save originURI to the history. r=bz

For added fun, that appears to be a security bug I can't access.

Dragana, can you investigate?

Comment 2

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

In my regression hunting, I always able to reproduce with these steps:

1. Click a link to some page you have never visited
2. Press Cmd / Ctrl-U to View Page Source
3. Address bar is missing view-source: prefix

Comment 3

[Dragana Damjanovic [:dragana]]

i will take a look

Comment 4

[Dragana Damjanovic [:dragana]]

Attachment: bug_1213267.patch

Comment 5

[Dragana Damjanovic [:dragana]]

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

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8672522 [details] [diff] [review]
bug_1213267.patch

r=me

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7e37f3445ffe

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7e37f3445ffe

Comment 9

[Dragana Damjanovic [:dragana]]

Comment on attachment 8672522 [details] [diff] [review]
bug_1213267.patch

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: This is a regression from  bug 1185256. The user impact is not high: view source tab will not include "view-source://" until page is reloaded, but it is only one-line patch.
Fix Landed on Version: 44
Risk to taking this patch (and alternatives if risky): very low
String or UUID changes made by this patch: none

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Approval Request Comment
[Feature/regressing bug #]: regression from bug 1185256
[User impact if declined]:This is a regression from  bug 1185256. The user impact is not high: view source tab will not include "view-source://" until page is reloaded, but it is only one-line patch.
[Describe test coverage new/current, TreeHerder]: There is simple way to reproduce it described in comment #0
[Risks and why]: very low
[String/UUID change made/needed]: none

Comment 10

[Liz Henry (:lizzard) (privacy/fingerprinting team)]

Comment on attachment 8672522 [details] [diff] [review]
bug_1213267.patch

Recent regression, reproducible, and we need this in beta to go with the uplift for bug 1185256.

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/d8c7509ddad0

Comment 12

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8672522 [details] [diff] [review]
bug_1213267.patch

Just like bug 1185256, not taking it.

Comment 13

[Cornel Ionce [:noni]]

Confirming the fix across platforms, using:
- Firefox 43 RC, buildID 20151208100201
- latest 44 DevEdition, build ID 20151213004008.

Comment 14

[Dragana Damjanovic [:dragana]]

Comment on attachment 8672522 [details] [diff] [review]
bug_1213267.patch


[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: This is a regrestin of bug 1185256 and needs to be uplifted too
User impact if declined: 
Fix Landed on Version:44
Risk to taking this patch (and alternatives if risky): low
String or UUID changes made by this patch: yes (the same once as bug 1185256)

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Comment 15

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8672522 [details] [diff] [review]
bug_1213267.patch

Need to uplift this regression fix to esr38.7 in order to uplift fix for bug 1185256 (sec-high issue)

Comment 16

[Dragana Damjanovic [:dragana]]

Attachment: bug_1213267_esr38.patch

Comment 17

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8724428 [details] [diff] [review]
bug_1213267_esr38.patch

r=me

Comment 18

[Dragana Damjanovic [:dragana]]

Comment on attachment 8724428 [details] [diff] [review]
bug_1213267_esr38.patch

Needed for 1246956.
depends on 1185256.

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: 
Fix Landed on Version:
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch: 

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Comment 19

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8724428 [details] [diff] [review]
bug_1213267_esr38.patch

This is also needed in esr38.7

Comment 20

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-esr38/rev/dfea8f0ec4b7

Comment 21

[Cornel Ionce [:noni]]

Also tested on Mac OS X 10.11, Windows 10 x86 and Ubuntu 12.04x86 with Firefox 38.7.0 ESR (buildID 20160302171452).
The view source is displayed in a new window and the title is "Source of: https://taco-spolsky.github.io/" (URL is not available in the opened window)

Since the view source in tab option was only implemented starting with Fx 42 (https://developer.mozilla.org/en-US/docs/Tools/View_source) and creating the "view_source.tab" boolean has no effect on ESR 38.7.0, I've also tested with the Source Viewer Tab addon (version 0.3.2014042801.1).
With the addon enabled, the view source is displayed in a new tab and the URL is "view-source-tab:https://taco-spolsky.github.io/".


Based on these results, I'm not sure how reliable the verification on ESR 38 is.
Am I missing something here?

Comment 22

[Dragana Damjanovic [:dragana]]

(In reply to Cornel Ionce [QA] from comment #21)
> Also tested on Mac OS X 10.11, Windows 10 x86 and Ubuntu 12.04x86 with
> Firefox 38.7.0 ESR (buildID 20160302171452).
> The view source is displayed in a new window and the title is "Source of:
> https://taco-spolsky.github.io/" (URL is not available in the opened window)
> 
> Since the view source in tab option was only implemented starting with Fx 42
> (https://developer.mozilla.org/en-US/docs/Tools/View_source) and creating
> the "view_source.tab" boolean has no effect on ESR 38.7.0, I've also tested
> with the Source Viewer Tab addon (version 0.3.2014042801.1).
> With the addon enabled, the view source is displayed in a new tab and the
> URL is "view-source-tab:https://taco-spolsky.github.io/".
> 
> 
> Based on these results, I'm not sure how reliable the verification on ESR 38
> is.
> Am I missing something here?

The test from comment #0 does not work for esr38 because the view source feature is not available in that version. But anyway this change needed to be uplifted together with bug 1185256, because the code path is used for other stuff as well. (it is just one line change)

Thanks for checking it.