Closed
Bug 1134606
Opened 10 years ago
Closed 10 years ago
Disable <meta referrer> handling on 36 pending an actual request to ship process
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
Details
Attachments
(1 file)
3.20 KB,
patch
|
geekboy
:
review+
Sylvestre
:
approval-mozilla-release+
|
Details | Diff | Splinter Review |
[Tracking Requested - why for this release]: Because if we ship this, we can't remove it and it's not clear we want it. See discussion in bug 1113431. We need to a least disable it from being detected, but since people detect it via use....
![]() |
Assignee | |
Updated•10 years ago
|
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
![]() |
Assignee | |
Comment 2•10 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d4e90dbce88a
Comment 3•10 years ago
|
||
Comment on attachment 8566502 [details] [diff] [review] Disable <meta referrer> in Firefox 36 pending some loose ends being sorted out Review of attachment 8566502 [details] [diff] [review]: ----------------------------------------------------------------- ::: parser/html/nsHtml5TreeOpExecutor.cpp @@ +959,5 @@ > void > nsHtml5TreeOpExecutor::SetSpeculationReferrerPolicy(const nsAString& aReferrerPolicy) > { > + // Disabled for now. > + return; This won't cause fatal warnings (unreachable code) on some platforms where warnings are treated as errors, will it?
Attachment #8566502 -
Flags: review?(sstamm) → review+
Comment 4•10 years ago
|
||
Comment on attachment 8566502 [details] [diff] [review] Disable <meta referrer> in Firefox 36 pending some loose ends being sorted out [Triage Comment] I am using my super powers to fast track this patch to release. We do want to disable this in 36.
Attachment #8566502 -
Flags: approval-mozilla-release+
![]() |
Assignee | |
Comment 5•10 years ago
|
||
> This won't cause fatal warnings (unreachable code) on some platforms where warnings are > treated as errors, will it? https://treeherder.mozilla.org/#/jobs?repo=try&revision=fdc5c4ea198b is running to check, but so far it's green on Linux on try and on Mac locally.... if it becomes a problem, I'll deal with that.
Comment 6•10 years ago
|
||
So, if am understanding correctly we are *potentially* going to disable <meta referrer> in 36. I am not the expert in this code, but I browsed through the changeset a little. Unfortunately we don't have a boolean switch to just switch it off :-( Anyway, wouldn't we also need to disable it here: http://hg.mozilla.org/mozilla-central/rev/3511a31f2cd3#l10.222 Or potentially revisit all those callsites again: http://mxr.mozilla.org/mozilla-central/search?string=ReferrerPolicyFromString What should we do about referrer and CSP - disable the CSP referrer as well? Oh I guess it's not in 36 anyway: https://bugzilla.mozilla.org/show_bug.cgi?id=965727
Comment 7•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-beta/rev/521cf86d194b
status-firefox36:
--- → affected
status-firefox37:
--- → wontfix
status-firefox38:
--- → wontfix
Target Milestone: --- → mozilla36
![]() |
Assignee | |
Comment 8•10 years ago
|
||
Christoph, CSP is not an issue for 36 as you note. Good catch on ReferrerPolicyFromString being the right thing to look at. On beta this has three callsites: the two that I found patched and the image loader one. Looking at the image loader case, the only internal caller we have is nsAlertsIconListener::StartRequest which passes "default" as the policy. Extensions might be using it, of course, but I'm fine with leaving that bit in, since it's not web-exposed.
Comment 9•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-release/rev/521cf86d194b
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•