1165501 - Re-setting referrer policy

Status: RESOLVED FIXED

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

Description

[Franziskus Kiefer [:franziskus]]

Referrer policy spec (http://w3c.github.io/webappsec/specs/referrer-policy/#set-referrer-policy) says "If a policy has previously been set, then we overwrite it with the new value.". The code currently implements an old version of the spec that sets the policy to No-Referrer on any attempt of changing it.

* https://dxr.mozilla.org/mozilla-central/source/dom/base/nsDocument.cpp#2977
* https://dxr.mozilla.org/mozilla-central/source/parser/html/nsHtml5TreeOpExecutor.cpp#1013

Comment 1

[Franziskus Kiefer [:franziskus]]

https://dxr.mozilla.org/mozilla-central/source/dom/base/nsDocument.cpp#3787

Comment 2

[Franziskus Kiefer [:franziskus]]

Attachment: using most recent referrer policy found in the document (r=sstamm)

policy is updated if a new one is found; two spot tests to see that it is working

Comment 3

[Franziskus Kiefer [:franziskus]]

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

Comment 4

[Sid Stamm [:geekboy or :sstamm]]

Comment on attachment 8610888 [details] [diff] [review]
using most recent referrer policy found in the document (r=sstamm)

Review of attachment 8610888 [details] [diff] [review]:
-----------------------------------------------------------------

Just some minor comments.  r=me with removal of that unused field in nsHtml5TreeOpExecutor and a minor tweak to the test.

::: dom/base/test/bug704320.sjs
@@ +43,5 @@
>  // script that clicks a link after all resources are (hopefully)
>  // loaded. The click triggers a redirection to file_bug704320_redirect.html,
>  // which in turn notifies the main window that it's time to check the test
>  // results.
> +function createTest(schemeFrom, schemeTo, policy, wrongPolicy) {

Please rename the new argument to be "optionalEarlierPolicy" or "overriddenPolicy" to make it clearer what this new argument does.

::: parser/html/nsHtml5TreeOpExecutor.cpp
@@ +1012,5 @@
>  {
> +  // Record "speculated" referrer policy locally and thread through the
> +  // speculation phase.  The actual referrer policy will be set by
> +  // HTMLMetaElement::BindToTree().
> +  mSpeculationReferrerPolicyWasSet = true;

Please also remove mSpeculationReferrerPolicyWasSet from nsHtml5TreeOpExecutor, since it is no longer necessary.  Keep nsDocument::mReferrerPolicySet, though, since it's needed for the CSP->speculation parser connection.

Comment 5

[Franziskus Kiefer [:franziskus]]

Attachment: using most recent referrer policy found in the document (r=sstamm)

Comment 6

[Franziskus Kiefer [:franziskus]]

Comment on attachment 8611431 [details] [diff] [review]
using most recent referrer policy found in the document (r=sstamm)

Review of attachment 8611431 [details] [diff] [review]:
-----------------------------------------------------------------

carries over

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/50050ecd15d8

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/50050ecd15d8

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/50050ecd15d8