Open Bug 1386673 Opened 7 years ago Updated 2 years ago

Make Contextual Identity extensions be an optional permission

Categories

(WebExtensions :: General, enhancement, P3)

56 Branch
enhancement

Tracking

(Not tracked)

REOPENED

People

(Reporter: jkt, Unassigned)

References

(Blocks 3 open bugs)

Details

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.
Assignee: nobody → jkt
No longer depends on: 1354602
Priority: -- → P3
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
Flags: needinfo?(aswan)
Talked to Jonathan about this on IRC.  Please reset needinfo if I've cleared this prematurely.
Flags: needinfo?(aswan)
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.
Blocks: 1405887
Blocks bug 1191418
Product: Toolkit → WebExtensions
Blocks: 1458585
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → WONTFIX

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.

Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---

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

Flags: needinfo?(yuki)

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

Assignee: jonathan → nobody

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.

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.

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.

Flags: needinfo?(philipp)

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.
Flags: needinfo?(yuki)

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)

Depends on: 1549204

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)

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.

Flags: needinfo?(philipp)
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.