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

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Boris Zbarsky [:bzbarsky]]

[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....

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Disable <meta referrer> in Firefox 36 pending some loose ends being sorted out

Comment 2

[Boris Zbarsky [:bzbarsky]]

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

Comment 3

[Sid Stamm [:geekboy or :sstamm]]

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?

Comment 4

[Sylvestre Ledru [:Sylvestre]]

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.

Comment 5

[Boris Zbarsky [:bzbarsky]]

> 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

[Christoph Kerschbaumer [:ckerschb}]

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

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/521cf86d194b

Comment 8

[Boris Zbarsky [:bzbarsky]]

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

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/521cf86d194b