Disable <meta referrer> handling on 36 pending an actual request to ship process

RESOLVED FIXED in Firefox 36

Status

()

defect
RESOLVED FIXED
4 years ago
a month ago

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Tracking

Trunk
mozilla36
x86
macOS
Points:
---

Firefox Tracking Flags

(firefox36+ fixed, firefox37 wontfix, firefox38 wontfix)

Details

Attachments

(1 attachment)

[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: nobody → bzbarsky
Status: NEW → ASSIGNED
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 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+
> 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.
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
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.
https://hg.mozilla.org/releases/mozilla-release/rev/521cf86d194b
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
See Also: → 1140638
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.