Closed Bug 1487297 Opened 6 years ago Closed 6 years ago

Add a Web Extension API for controlling the other content blocking options that we're exposing

Categories

(WebExtensions :: General, enhancement, P3)

enhancement

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 1493057

People

(Reporter: ehsan.akhgari, Assigned: baku)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Whiteboard: [privacy])

Attachments

(1 file, 1 obsolete file)

We currently allow tracking protection to be controlled by extensions.  It would be nice to provide the same controls for the other content blocking tools we are going to expose (especially the ones that we're going to turn on by default).
This is probably a good idea, but it could use more specifics. We already have APIs to control privacy settings that can be extended to handle more things:

https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/privacy
https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/browserSettings
This is in line with the other kinds of privacy features we provide to developers.  Marking it P3 for now, just to indicate it's something we'd want in our backlog, but like Kris said, we'd need more detail.  The other consideration is user awareness of when extensions are controlling these settings, because if privacy controls can be set, they can also be unset by malicious extensions trying to avoid detection and/or exfiltrate data.  We'll need some UX to make sure users are always aware and in control of these settings.
Severity: normal → enhancement
Component: Experiments → General
Priority: -- → P5
Priority: P5 → P3
Apologies for the lack of specifics.  :-)  I mainly filed this as a placeholder for now.

I am not remotely a WebExtensions API expert, but looking over what we have exposed currently, it seems that https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/privacy/websites would be a good place to expose the new controls.

We currently have trackingProtectionMode there, which is great and should stay exposed.  We also have thirdPartyCookiesAllowed, which is a bit limiting in our design here, since internally Gecko actually has several cookie policies that dictate how various types of cookies, including third-party cookies must be exposed, so a boolean is insufficient.

That boolean isn't supported yet, so maybe we have an opportunity to define how it works as part of this?  We can map the value false for it as the BEHAVIOR_REJECT_FOREIGN cookie policy.  But when it's true, we cannot infer the active cookie policy just from that.  (Since we may block *some* third-party cookies, e.g. only those from trackers, but not all).  In my ideal world, we would convert that API to an enumerated type value rather than a boolean with values such as "none", "all", "non-trackers-only", "visited-only" (and in the future once bug 1478427 is fixed, "non-trackers-and-visited-only") which would map to BEHAVIOR_REJECT_FOREIGN, BEHAVIOR_ACCEPT, BEHAVIOR_REJECT_TRACKER and BEHAVIOR_LIMIT_FOREIGN.  Not sure what the backwards-compat concerns are for these APIs.

We also need an API for exposing "fastblock".  Strawman: exposing an attribute under privacy.websites called slowLoadingTrackersAllowed (boolean) which maps to browser.fastblock.enabled.

We also need an API to control the global "content blocking" settings, so that extensions like adblockers and privacy extensions which want to take over the task of content block and override everything that Firefox does can have a one-stop shop to disable things globally rather than chasing an ever increasing list of APIs to call.  Strawman proposal: exposing an attribute under privacy.websites called contentBlockingEnabled (boolean) which maps to browser.contentblocking.enabled.

In terms of user awareness, I completely agree with you Mike that we need to carefully consider making the user aware of how extensions take over the various settings here.  We already have UI for privacy.websites.trackingProtectionMode which is integrated in the new Content Blocking UI in Preferences.  We would need similar UI for any new APIs being added here, at the very least, possibly more hooks into the UI code to modify things when an extension has taken over these settings.  I'd recommend the UX part to be done in close coordination with Bryan Bell who has been helping out the security engineering team in the design of the Content Blocking UI so far.
> We currently have trackingProtectionMode there, which is great and should
> stay exposed.  We also have thirdPartyCookiesAllowed, which is a bit

this is not supported yet.

About cookiePolicy I prefer to change: cookieConfig introducing a 'reject_tracker'. This should be enough for cookiePolicy.

> We also need an API for exposing "fastblock".  Strawman: exposing an
> attribute under privacy.websites called slowLoadingTrackersAllowed (boolean)
> which maps to browser.fastblock.enabled.

I'm going to do this as well.
Attached patch part 1 - reject tracker (obsolete) — Splinter Review
Assignee: nobody → amarchesini
Attachment #9006514 - Flags: review?(lgreco)
Attachment #9006515 - Flags: review?(lgreco)
Attachment #9006514 - Flags: review?(lgreco) → review+
Attachment 9006514 [details] [diff] looks good to me, it is unfortunate that Bug 1390160 is not fixed yet and so there is currently no UI to show in about:preferences that the cookie settings are controlled by an extension (and which is the extension that is controlling it), nevertheless the extension can already control that setting without the changes from attachment 9006514 [details] [diff] [review] (e.g. an extension with the privacy permission can already for it to "allow_all") and so I'm not adding Bug 1390160 as a blocker of this issue.

(But I've unassigned Bug 1390160, so that it is clear that is not being worked on yet, and I'm adding it as a "see also" on this issue).
See Also: → 1390160
Comment on attachment 9006515 [details] [diff] [review]
part 2 - fastblock

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

Thanks Andrea, this patch also look good, there are just some additional questions that I'd like to agreed on with you before proceeding to 
r+ this patch.

::: toolkit/components/extensions/parent/ext-privacy.js
@@ +159,5 @@
> +    "browser.fastblock.enabled",
> +  ],
> +
> +  setCallback(value) {
> +    return {[this.prefNames[0]]: value};

is this feature already enabled on all channels?

If it is not, should we reject the `set` when the related preference is not turned on?

From another point of view, we could also opt to lock this setting behind a preference on its own, while we add the UI to let the user know that an extension is controlling this setting.

:baku (and :Ehsan) what do you think?

In the meantime I filed Bug 1489498, as the follow up issue to ensure that we "show that a WebExtension is managing content blocking settings".

::: toolkit/components/extensions/schemas/privacy.json
@@ +117,5 @@
>        "firstPartyIsolate": {
>          "$ref": "types.Setting",
>          "description": "If enabled, the browser will associate all data (including cookies, HSTS data, cached images, and more) for any third party domains with the domain in the address bar. This prevents third party trackers from using directly stored information to identify you across different websites, but may break websites where you login with a third party account (such as a Facebook or Google login.) The value of this preference is of type boolean, and the default value is <code>false</code>."
>        },
> +      "fastBlock": {

How about using "slowLoadingTrackersAllowed", as suggested by Ehsan in comment 3, for this one?

Also, it seems reasonable to me to also add the additional "contentBlockingEnabled" setting (also suggested by Ehsan in comment 3) to control the "global" content blocking setting as part of this issue.
Flags: needinfo?(ehsan)
Comment on attachment 9006515 [details] [diff] [review]
part 2 - fastblock

Hi :baku,
I just noticed that I missed to clear the previous review request on this patch.

See comment 8 for some questions I have about this part of the change.
Attachment #9006515 - Flags: review?(lgreco)
(In reply to Luca Greco [:rpl] from comment #8)
> ::: toolkit/components/extensions/parent/ext-privacy.js
> @@ +159,5 @@
> > +    "browser.fastblock.enabled",
> > +  ],
> > +
> > +  setCallback(value) {
> > +    return {[this.prefNames[0]]: value};
> 
> is this feature already enabled on all channels?

Not yet.

> If it is not, should we reject the `set` when the related preference is not
> turned on?

Good point.

Thinking about what you said, I think this patch actually implements the wrong logic.  The browser.fastblock.enabled controls *the feature* and *whether it should be turned on right now*.  We should probably separate these two into two separate prefs.  For example, we can keep browser.fastblock.enabled to mean whether the feature should be enabled, and introduce a new pref, let's call it browser.fastblock.active (with a default value of true) to mean whether the fastblock feature is active at any given point in time.  Then, for the feature to kick in, both prefs need to be true.

And here in set(), we can check whether browser.fastblock.enabled is true.  If it's false, we can reject the call due to the feature not being available, and if it's true we can adjust browser.fastblock.active.

(baku, it's probably worth bringing up this change with Gary before changing things on them!)

Also, a minor random nit, I wouldn't expose an API called websites.fastBlock...  "fastblock" is just an internal code name we're using, but it's not supposed to be a developer facing name.  Perhaps "websites.blockSlowLoadingTrackers" or some such instead?

> From another point of view, we could also opt to lock this setting behind a
> preference on its own, while we add the UI to let the user know that an
> extension is controlling this setting.

I'm not completely following this part.  Did I answer this above?  If not, do you mind clarifying a bit please?
Flags: needinfo?(ehsan)
No longer blocks: 1489498
Depends on: 1489498
(In reply to :Ehsan Akhgari from comment #10)
> Also, a minor random nit, I wouldn't expose an API called
> websites.fastBlock...  "fastblock" is just an internal code name we're
> using, but it's not supposed to be a developer facing name.  Perhaps
> "websites.blockSlowLoadingTrackers" or some such instead?

Totally agree, I also mentioned it near the end of comment 8.

We have one of the settings in the privacy API that is called `thirdPartyCookiesAllowed` 
and so I was proposing to name this `slowLoadingTrackersAllowed` (as you suggested in comment 3).
 
> > From another point of view, we could also opt to lock this setting behind a
> > preference on its own, while we add the UI to let the user know that an
> > extension is controlling this setting.
> 
> I'm not completely following this part.  Did I answer this above?  If not,
> do you mind clarifying a bit please?

Sure, my apologies, let me clarify it:

We would like to do not allow extensions to use new APIs like this (which basically take control
of a feature that the user can control from the Firefox UI, e.g. about:preferences or the doorhangers),
until we also have some UI that allow the user to:

- know that an extension is controlling that feature/setting
- know which is the extension that is controlling it
- allow the user to disable the extension to opt-out the controlled settings

Like we do for most of the other similar APIs/features (e.g. when an extension is controlling the newTab url, or the tracking protection).

One way of doing it is to land the WebExtensions API still locked behind a pref and reject the API calls to set the `slowLoadingTrackersAllowed` privacy setting if that pref is not yet enabled (and then turn it on officially on all the channels once the related UI is in place).

(As a side note, during our last triaging we reversed the relationship between this bug and Bug 1489498 for the same reason).
See Also: → 1493057
Depends on: 1493058
No longer depends on: 1493058
Comment on attachment 9006514 [details] [diff] [review]
part 1 - reject tracker

My patch in bug 1493057 does part of what this patch did...
Attachment #9006514 - Attachment is obsolete: true
Whiteboard: [privacy]
ehsan and baku, is this patch complete?  We are moving forward with dependent bug 1489498, but I want to make sure everything is settled with the API and its structure before proceeding.  Also, please take a look at this short doc and comment, if needed:

https://docs.google.com/document/d/16-mfoqh91HGthU2Ib9uhsoFaqFpPLg7ACGaHcs2Z-XM/edit?usp=sharing

Thank you.
Flags: needinfo?(ehsan)
Flags: needinfo?(amarchesini)
No, This patch is just about fastblock. We need something different to covert the prefs behind the content blocking panel.
Flags: needinfo?(amarchesini)
Flags: needinfo?(ehsan)
Blocks: 1462372
No longer blocks: antitracking
FastBlock has been removed. There is nothing else to do here. Currently, webextensions can set cookieBehavior values and enable/disable tracking protection in normal and private browsing mode.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: