1386673 - Make Contextual Identity extensions be an optional permission

Status: REOPENED

(WebExtensions :: General, enhancement, P3)

Description

[Jonathan Kingston [:jkt] he/him]

In Bug 1354602 we are enabling containers when a user installs a container extension.

We would like to make this work when the extension has an optional permission for containers also.

This bug is about ensuring we tracking the right state for extensions and correctly track the permission changes.

Comment 2

[Jonathan Kingston [:jkt] he/him]

When using a optional preference, we should only enable the permissions then.

Should we auto deny permissions.request() if the user doesn't have containers enabled? Is there more work we should be doing to make this a cleaner experience?

See Bug 1391184

Comment 3

[Andrew Swan [:aswan]]

Talked to Jonathan about this on IRC.  Please reset needinfo if I've cleared this prematurely.

Comment 4

[Jonathan Kingston [:jkt] he/him]

The discussed solution was to treat optional permissions the same, when the user accepts them it will flip the prefs. We also should update web extensions to inform the user when they are accepting the permission, this should make it clear that it controls containers.

Comment 6

[ExE Boss]

Blocks bug 1191418

Comment 8

[Shane Caraveo (:mixedpuppy)]

We already do not prompt for contextual identities, so there is no sense in it being optional.

https://searchfox.org/mozilla-central/rev/97cb0a90bd053de87cd1ab7646d5565809166bb1/toolkit/components/extensions/test/xpcshell/test_ext_permissions.js#562-586

Comment 9

[Luca Greco [:rpl] [:luca] [:lgreco]]

We discussed about this with Shane because I think even if we don't prompt for the contextual identities API there is still a reason (and more importantly a use case) for making it optional:

When an extension requires that permission Firefox does enable the contextualIdentities feature inside Firefox itself, which is something that is disabled by default.

Some extensions (e.g. the ones that manage tabs in a sidebar, like TreeStyleTabs, Tab Center Redux, etc.) may want to require that permission as optional and only enable the feature is the user wants to, or if another extension has already enabled the feature (e.g. when an extension like multi-accounts-container is installed, or if they detect through a new property added to the browserSettings API).

And so I'm reopening this bug to evaluate this bugzilla issue based on this use case.

Comment 10

[Luca Greco [:rpl] [:luca] [:lgreco]]

Let's also needinfo-ing Piro to hear an opinion from an extension developer perspective on the use case described in Comment 9.

Comment 11

[Luca Greco [:rpl] [:luca] [:lgreco]]

... and also un-assign it until someone is going to actually work on it.

Comment 12

[Shane Caraveo (:mixedpuppy)]

If we were to support this for the use case described, we would need to add a browserSettings.contextualIdentityEnabled API. That would allow an extension to determine whether the feature is enabled in firefox, then request the optional permission.

Comment 13

[kkapsner]

Good idea with the browserSettings.contextualIdentityEnabled. Another idea that would solve my use case (https://github.com/kkapsner/CanvasBlocker/issues/381) is to have a read only permission that does not enable the feature.

Comment 14

[Shane Caraveo (:mixedpuppy)]

Hrm, I suppose that the issue would be:

Where A is a container addon, and B is a tab manager addon:

addon A requires contextual identies
addon B wants to support the use, but doesn't want to require it
addon B requests the optional permission
addon A is uninstalled

In that case we'd want to disable contextual identities because no addon is driving a primary feature with it, but because addon B has permission now, it is not disabled.

So ideally, any read operation would not require permission, but any write operation would.

This could probably be achieved by only requiring the permission to use create/update/remove functionality, allow the rest of the namespace to be used without any permission.

Philipp, what do you think? I see no reason that we couldn't do this.

Comment 15

[YUKI "Piro" Hiroshi]

Basically Tree Style Tab addon aims to provide a tab management UI compatible to Firefox's tab bar, and it is out of purpose that activating the contextual identities feature. It will make me happy if the permission contextualIdentities just provides "read" access but doesn't activate the contextual identities feature.

So an optional contextualIdentities permission won't match to my case. The value contextualIdentities will be listed as a permanent permission forever, because it is required to read the color information of each container. In other words, to provide tabbar compatible UI including colored container indicators, we always need to request the permission despite unused containers feature doesn't need to be activated.

Thus a future like following looks best for my case:

• The contextualIdentities permission don't activate the containers feature if it is not activated yet. It just provides "read" permission for containers information including contextualIdentities.onUpdated and other listeners.
• The browserSettings API exposes an property like contextualIdentitiesEnabled corresponding to the activation status of the containers feature.
• The browserSettings API provides ability to listen changes of contextualIdentitiesEnabled. When it becomes true my addon will activates its containers selector UI dynamically.

Comment 16

[Luca Greco [:rpl] [:luca] [:lgreco]]

That is a very good point, thanks Piro and kkapsner!

Given that both comment 13 and comment 15 are explicitly mentioning reasonable use cases for a read-only contextualIdentities permission, I think that (to avoid making a breaking change related to how the existing contextualIdentities use to work now) we could evaluate to also introduce a new contextualIdentities.read-only pref.

(it would not be the first "sub-permission" that we do support, e.g. we do have a "downloads.open" permission along with the "downloads" one)

Comment 17

[Shane Caraveo (:mixedpuppy)]

The plan we've come up with thus far is:

1. address bug 1549204 first
2. add new permission (without prompt) for contextualIdentities.readOnly
3. support both permissions as optional

The readonly permission will give access to the contextualIdentities namespace, but only those APIs that cannot change data.
The full permission will give access to the entire namespace.

An extension that currently uses "contextualIdentities" for read only access would have to change to the new permission and move "contextualIdentities" to the optional_permissions.

"permissions": ["contextualIdentities.readOnly"],
"optional_permissions": ["contextualIdentities"]

This would result in the permission being maintained for current users, avoiding bug 1549204 if it cannot be addressed first.

We have to change the api implementation itself to:

• dont set preferences for readonly access
• all readonly APIs will check the pref, and either reject or return an empty dataset (resolved promise) if the feature is not enabled (I lean towards empty dataset)

Comment 18

[JulianHofstadter]

Chiming it, I too am all in favor of readOnly and optional. Users who don't want the container feature an addon optionally provides may also not want to have Containers enabled and see options appear in menus they aren't interested in.