Remove network.http.enablePerElementReferrer

RESOLVED FIXED in Firefox 55

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: francois, Assigned: swapneshks, Mentored)

Tracking

({good-first-bug})

unspecified
mozilla55
good-first-bug
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [necko-would-take])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

2 years ago
That pref was used to gate the release of per-element referrer policies until the spec was changed to enable this delivery mechanism.

The candidate recommendation of the Referrer Policy spec has it:

  https://www.w3.org/TR/referrer-policy/#referrer-policy-delivery-referrer-attribute

and it has made its way into the HTML spec too:

  https://html.spec.whatwg.org/multipage/infrastructure.html#referrer-policy-attribute

Given that we already have several prefs to trim the referrer (https://feeding.cloud.geek.nz/posts/tweaking-referrer-for-privacy-in-firefox/) and that we don't offer a toggle for document-level referrer policies, this per-element pref is not a meaningful user control mechanism.
Mentor: valentin.gosu
Keywords: good-first-bug
Whiteboard: [necko-would-take]
Hi.. I'm a beginner and would like to take up this bug.
I'm still catching up to things...
Flags: needinfo?(francois)
(Reporter)

Comment 2

2 years ago
(In reply to swapneshks from comment #1)
> Hi.. I'm a beginner and would like to take up this bug.
> I'm still catching up to things...

Feel free to give it a go.

You can start by searching for the name of the pref in https://searchfox.org/ to find out where it's defined and where it's used. From there, you'll get a feel for what needs to change in order to remove it entirely.
Flags: needinfo?(francois)
So I had a look at the usages and found that the pref is being declared at 

https://searchfox.org/mozilla-central/source/modules/libpref/init/all.js#1548

I tried commenting out the above line, but I am not sure how to check whether the action took any effect.
Also, regarding the pref usages, I have the following doubts:

1. Let's pick https://searchfox.org/mozilla-central/source/browser/base/content/browser.js#5821. Do I have to remove only the referrer condition or the entire block?
2. After removal of the pref usage, how do I test whether it took effect?
Flags: needinfo?(francois)
So these are all the instances of the pref:
https://searchfox.org/mozilla-central/search?q=network.http.enablePerElementReferrer&path=

(In reply to Swapnesh Kumar Sahoo [:swapneshks] from comment #3)
> 1. Let's pick
> https://searchfox.org/mozilla-central/source/browser/base/content/browser.
> js#5821. Do I have to remove only the referrer condition or the entire block?

All instances of the pref would become true. So

if (Services.prefs.getBoolPref("network.http.enablePerElementReferrer") &&
      linkNode) {

would become

if (linkNode) {

Also, in webidl files, [SetterThrows, Pref="network.http.enablePerElementReferrer"] becomes [SetterThrows]

> 2. After removal of the pref usage, how do I test whether it took effect?

I don't think there is much to check. Just make all the changes, submit the patch, and we'll make sure all the tests are still passing.
Assignee: nobody → swapneshks
Flags: needinfo?(francois)
Created attachment 8863260 [details] [diff] [review]
Remove network.http.enablePerElementReferrer usages

Do let me know if I went wrong anywhere/further changes to be made.
Attachment #8863260 - Flags: review?(valentin.gosu)
Comment on attachment 8863260 [details] [diff] [review]
Remove network.http.enablePerElementReferrer usages

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

You missed one instance at:
http://searchfox.org/mozilla-central/source/browser/base/content/content.js#530
Attachment #8863260 - Flags: review?(valentin.gosu)
Created attachment 8863261 [details] [diff] [review]
Remove network.http.enablePerElementReferrer usages

Patch with changes added for earlier missed instance.
Attachment #8863261 - Flags: review?(valentin.gosu)
(Assignee)

Updated

2 years ago
Attachment #8863260 - Attachment is obsolete: true
Comment on attachment 8863261 [details] [diff] [review]
Remove network.http.enablePerElementReferrer usages

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

Looks good. Thank you for your contribution!
Attachment #8863261 - Flags: review?(valentin.gosu) → review+
Keywords: checkin-needed
(In reply to Valentin Gosu [:valentin] (Vacation until May 1st) from comment #10)
> Comment on attachment 8863261 [details] [diff] [review]
> Remove network.http.enablePerElementReferrer usages
> 
> Review of attachment 8863261 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good. Thank you for your contribution!

My pleasure! 
This is my first accepted patch... Feels great!
WebIDL changes need DOM peer review before they can land.
Keywords: checkin-needed
Attachment #8863261 - Flags: review?(amarchesini)
Attachment #8863261 - Flags: review?(amarchesini) → review+
Keywords: checkin-needed

Comment 13

2 years ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0041706d6e43
Remove network.http.enablePerElementReferrer usages. r=valentin r=baku
Keywords: checkin-needed

Comment 14

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0041706d6e43
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.