ContextualIdentities API isn't reloaded when containers are enabled

RESOLVED FIXED in Firefox 55

Status

enhancement
P3
normal
RESOLVED FIXED
2 years ago
Last year

People

(Reporter: jkt, Assigned: baku)

Tracking

(Blocks 1 bug)

unspecified
mozilla55
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [triaged])

Attachments

(1 attachment, 1 obsolete attachment)

- Should browser.contextualIdentities even be a key of browser?
- When contextualIdentities is enabled the API should be enabled.

STR:

1. Set privacy.userContext.enabled false
2. Install extension
3. Set privacy.userContext.enabled true
4. Use browser.contextualIdentities


STR requires an extension that checks this API again in 4. which mine does not due to this issue.

I'm not sure if we want some form of onEnabled/onDisabled event as hopefully containers will become a default eventually.

---

Currently my extension pretty much requires containers to function correctly:

https://addons.mozilla.org/en-GB/firefox/addon/sea-containers/

I have added a link to install containers and when the user installs the browser.contextualIdentities API is still undefined.
Flags: needinfo?(amckay)
That would be up to baku, so pinging for an opinion.
Flags: needinfo?(amckay) → needinfo?(amarchesini)
Priority: -- → P3
Whiteboard: [triaged]
Posted patch extension.patch (obsolete) — Splinter Review
Assignee: nobody → amarchesini
Flags: needinfo?(amarchesini)
Attachment #8870787 - Flags: review?(kmaglione+bmo)
Attachment #8870787 - Flags: review?(kmaglione+bmo) → review?(aswan)
Comment on attachment 8870787 [details] [diff] [review]
extension.patch

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

Please use XPCOMUtils.defineLazyPreferenceGetter() instead of Preferences.get()
Attachment #8870787 - Flags: review?(aswan) → review-
This is exactly what I have done. I just expose an object called 'Preferences'.
Flags: needinfo?(aswan)
Attachment #8870787 - Flags: review- → review?(aswan)
Ah sorry! My fault. Here a new patch.
Attachment #8870787 - Attachment is obsolete: true
Attachment #8870787 - Flags: review?(aswan)
Attachment #8871327 - Flags: review?(aswan)
Comment on attachment 8871327 [details] [diff] [review]
extension.patch

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

The implementation looks good but is this really the desired behavior?  Will users of the API be frustrated by for instance, the inability to tell if query() returned an empty array since the query didn't match versus because containers are disabled?  Sorry for not raising that in the first pass, if this behavior is desired (eg we don't want to expose to extensions whether they are enabled?) then r=me.
Attachment #8871327 - Flags: review?(aswan) → review+
> The implementation looks good but is this really the desired behavior?  Will

The reason why I wrote this patch is because often, containers are enabled/disabled. If a webextension uses the contextualIdentity API, when containers are enabled again, browser.contextualIdentities is not available because we don't refresh the permission/manifest. Better to have this approach how implemented here.

jkt, feedback?
Flags: needinfo?(jkt)
Can we change the return if the user has containers disabled? That way the user can at least check and cope with this?
Options:
- return false
- throw
- return a uniqueId we could provide a constant for: browser.contextualIdentities.IS_DISABLED

Alternatively can we somehow expose something which indicates if containers is enabled?

browser.contextualIdentities.isDisabled(); // True or false


Personally I would suggest returning false is the most friendly and also gives a clear signal containers has been disabled, this would also be the minimal amount of code to remove if/when containers is always on.
Flags: needinfo?(jkt)
Good to me with returning 'false'.
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/be95ec17a58f
Expose contextualIdentities to webExtensions also if privacy.userContext.enabled is set to false, r=aswan
https://hg.mozilla.org/mozilla-central/rev/be95ec17a58f
Status: NEW → RESOLVED
Closed: 2 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.