Closed Bug 1213267 Opened 4 years ago Closed 4 years ago

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

Categories

(Toolkit :: View Source, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
mozilla44
Tracking Status
firefox43 --- verified
firefox44 --- verified
firefox-esr38 45+ fixed

People

(Reporter: froydnj, Assigned: dragana)

References

Details

(Keywords: regression)

Attachments

(2 files)

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?
Flags: needinfo?(jryans)
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?
Flags: needinfo?(jryans) → needinfo?(dd.mozilla)
Keywords: regression
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
i will take a look
Assignee: nobody → dd.mozilla
Flags: needinfo?(dd.mozilla)
Attachment #8672522 - Flags: review?(bzbarsky)
Comment on attachment 8672522 [details] [diff] [review]
bug_1213267.patch

r=me
Attachment #8672522 - Flags: review?(bzbarsky) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/7e37f3445ffe
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
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
Attachment #8672522 - Flags: approval-mozilla-esr38?
Attachment #8672522 - Flags: approval-mozilla-beta?
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.
Attachment #8672522 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment on attachment 8672522 [details] [diff] [review]
bug_1213267.patch

Just like bug 1185256, not taking it.
Attachment #8672522 - Flags: approval-mozilla-esr38? → approval-mozilla-esr38-
Confirming the fix across platforms, using:
- Firefox 43 RC, buildID 20151208100201
- latest 44 DevEdition, build ID 20151213004008.
Status: RESOLVED → VERIFIED
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.
Attachment #8672522 - Flags: approval-mozilla-esr38- → approval-mozilla-esr38?
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)
Attachment #8672522 - Flags: approval-mozilla-esr38? → approval-mozilla-esr38+
Attachment #8724428 - Flags: review?(bzbarsky)
Attachment #8672522 - Flags: approval-mozilla-esr38+
Comment on attachment 8724428 [details] [diff] [review]
bug_1213267_esr38.patch

r=me
Attachment #8724428 - Flags: review?(bzbarsky) → review+
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.
Attachment #8724428 - Flags: approval-mozilla-esr38?
Comment on attachment 8724428 [details] [diff] [review]
bug_1213267_esr38.patch

This is also needed in esr38.7
Attachment #8724428 - Flags: approval-mozilla-esr38? → approval-mozilla-esr38+
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?
Flags: needinfo?(dd.mozilla)
(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.
Flags: needinfo?(dd.mozilla)
You need to log in before you can comment on or make changes to this bug.