Closed Bug 1165501 Opened 5 years ago Closed 5 years ago

Re-setting referrer policy

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: franziskus, Assigned: franziskus)

References

Details

Attachments

(1 file, 1 obsolete file)

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
Assignee: nobody → franziskuskiefer
policy is updated if a new one is found; two spot tests to see that it is working
Attachment #8610888 - Flags: review?(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.
Attachment #8610888 - Flags: review?(sstamm) → review+
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
Attachment #8611431 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/50050ecd15d8
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.