Closed Bug 1398097 Opened 7 years ago Closed 4 years ago

tabs.create API on Android does not support cookieStoreId

Categories

(WebExtensions :: Android, enhancement, P5)

57 Branch
enhancement

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 1622500

People

(Reporter: robwu, Unassigned)

References

Details

(Keywords: dev-doc-needed)

Attachments

(1 file, 2 obsolete files)

The tabs.create API on Android does not support cookieStoreId (even though "cookieStoreId" is added to the tabs.json schema).

Firefox Desktop:
https://searchfox.org/mozilla-central/rev/44c693914255638d74bcf1ec3b0bcd448dfab2fd/browser/components/extensions/ext-tabs.js#307-363

Firefox for Android:
https://searchfox.org/mozilla-central/rev/44c693914255638d74bcf1ec3b0bcd448dfab2fd/mobile/android/components/extensions/ext-tabs.js#241

At the very least I'd like to see support for cookieStoreId:'firefox-private' to allow private tabs to be opened in Fennec (this is currently not possible - bug 1372178) (to open a private tab, add the isPrivate flag to the options for BrowserApp.addTab).
Severity: normal → enhancement
Priority: -- → P5
Attached patch patch to resolve the bug (obsolete) — Splinter Review
Hi!
Kindly review it and let me know if any changes are required.
Flags: needinfo?(amckay)
Hi Aastha,
I took a look at the attached patch and here is an initial feedback:

There is no isPrivate property in tabs.create (and we don't want to add one on Firefox for Android), but we have a cookieStoreId in Firefox for Desktop, which can be used to put a single tab into an isolated container (like the related Test Pilot Experiments https://testpilot.firefox.com/experiments/containers/).

In Firefox Desktop using cookieStoreId 'firefox-private' to create a tab in a non private window raises an error: 

- http://searchfox.org/mozilla-central/rev/f54c1723befe6bcc7229f005217d5c681128fcad/browser/components/extensions/test/browser/browser_ext_tabs_cookieStoreId.js#22,27 

But in Firefox for Android there aren't windows, but the users is allowed to create private tabs.

So, in my opinion, in comment 0 Rob is suggesting is that we could allow (only on Firefox for Android) an API call like:

tabs.create({cookieStoreId: 'firefox-private, ...});

and in the API implementation we have to check that the cookieStoreId is a valid one (only the default and the private ones for now, using global.isPrivateCookieStore and global.isDefaultCookieStore, http://searchfox.org/mozilla-central/rev/f54c1723befe6bcc7229f005217d5c681128fcad/toolkit/components/extensions/ext-toolkit.js#49-55),
then we can create a proper Firefox for Android private tab by using the isPrivate property supported by the internal BrowserApp.addTab method (which is an internal privileged method that is used by the Firefox for Android internals and the WE APIs implemented on Firefox for Android, e.g.: 

- https://searchfox.org/mozilla-central/rev/44c693914255638d74bcf1ec3b0bcd448dfab2fd/mobile/android/components/extensions/ext-tabs.js#276

The following are some Firefox for Android internals that are using "isPrivate: true" on BrowserApp.addTab (which confirms that it is how Firefox for Android opens a private tab internally):

- http://searchfox.org/mozilla-central/source/mobile/android/chrome/content/aboutPrivateBrowsing.js#29
- http://searchfox.org/mozilla-central/source/mobile/android/chrome/content/browser.js#617
Flags: needinfo?(amckay)
Attached patch v2.patch (obsolete) — Splinter Review
I have made the changes as said in comment 2. But I am not able to run the test_ext_tabs_create.html.
Attachment #8913239 - Attachment is obsolete: true
Flags: needinfo?(lgreco)
Attachment #8914316 - Flags: review?(lgreco)
Hi Aastha,

In the attached patch there is an if statement at line 269 that is redundant and it is not closed and it introduces a syntax error in the ext-tabs.js module, which is very likely to be also the reason why you were not able to run test_ext_tabs_create.html successfully (when a Firefox for Android test is failing, you should also take a look in the messages logged on android using adb logcat, because the mochitest test suite is not currently able to catch and log these errors in the test output).
Flags: needinfo?(lgreco)
I am still not able to run test_ext_tabs_create.html. This is the link to adb logcat messages for the process (https://pastebin.mozilla.org/9069980).

How can I resolve this?
Flags: needinfo?(lgreco)
I resolved that error with running the test.
Attachment #8914316 - Attachment is obsolete: true
Attachment #8914316 - Flags: review?(lgreco)
Flags: needinfo?(lgreco)
Attachment #8920827 - Flags: review?(lgreco)
I'm not sure using PB mode for this is a good idea personally. Given that this would mean container 1 and container 2 wouldn't ne isolated.
Hi Jonathan,
The idea proposed by Rob Wu (as briefly described in Comment 0) is not to allow the creation of the "contextual containers" on Firefox for Android, but only to allow a subset of the behavior of the cookieStoreId property in the tabs.create API call, restricted to just the 'firefox-private' (and eventually 'firefox-default') cookieStoreId values (and reject for any other value).

By looking at the following test cases:

- http://searchfox.org/mozilla-central/rev/1285ae3e810e2dbf2652c49ee539e3199fcfe820/browser/components/extensions/test/browser/browser_ext_tabs_cookieStoreId.js#26-27

it seems that both:

- creating a new tab in a private window without any cookieStoreId
- creating a new tab in a private window with cookieStoreId 'firefox-private'

have the same outcomes (and given that we do not allow any other cookieStoreId to be used in a private window, I guess that two tabs created in a private window with and without the cookieStoreId are not actually isolated from each other).

If that is true as it seems, the following would be the only behaviors that we would like to allow on Firefox for Android: 

- no cookieStoreId -> create a new regular tab 
  (this is the only behavior allowed by the Firefox for Android WebExtensions tabs API right now)

- cookieStoreId: firefox-default -> create a new regular tab 
  (not really needed by our use case, but it could be nice to keep it consistent with the behavior 
  on Firefox Desktop)

- cookieStoreId: firefox-private -> create a private tab

- any other cookieStoreId value -> rejection 

From an implementation point of view, the Firefox for Android "tabs" API implementation would not import or use any of the contextualidentity internals, nor allow any of the additional behaviors that can be supported with the full contextualidentify feature enabled (create two non-private tabs isolated from each other). 

Nevertheless I want to be sure that the behaviors that we want to allow are consistent with how it behaves on Firefox Desktop (minus the differences with Firefox for Android, e.g. there is no private window, just private tabs), otherwise I would prefer a different strategy to allow this use case (creating private tabs from a Firefox for Android WebExtension).
Flags: needinfo?(jkt)
I've developed an WebExtension Add-on for "Temporary Containers" and noticed that it's possible to use browser.contextualIdentities.create() on Android since FF53 but it's not possible to create tabs with a cookieStoreId. This might be the wrong place to ask - but is there a reason to support creating contextualIdentities but not support creating tabs in them? Personally I'd love to see that feature on Android too.
Luca and I spoke at the time but never wrote this down. Creating tabs with firefox-private seems completely fine my only concern that I voiced was that code might not understand about the tab/window separation. I don't think there are any security/privacy concerns here that we need worry about.

The limitation on container tabs within Android is purely that the interface isn't present.
Flags: needinfo?(jkt)
Product: Toolkit → WebExtensions
Our compat data correctly reports this as not being supported on Android; we should update it once this is resolved.
Keywords: dev-doc-needed

tabs.create + cookieStoreId has been "implemented" on GeckoView (bug 1622500).

In my view, the implementation does not meet the requirements / description of the API, and I have voiced that concern in bug 1643688, which is scheduled for later.

Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: