1330167 - cookies.CookieStore.tabIds is an array of Tabs, should be an array of tab IDs

Status: RESOLVED FIXED

(WebExtensions :: General, defect)

Description

[Will Bamberg [:wbamberg]]

Attachment: cookies.zip

I've attached an example, which contains a background script like this:

function logStores(cookieStores) {
  for(store of cookieStores) {
    console.log(`Cookie store: ${store.id}\n Tab IDs:`, store.tabIds);
  }
}

function listCookieStores() {
  var getting = chrome.cookies.getAllCookieStores(logStores);
}

chrome.browserAction.onClicked.addListener(listCookieStores);

It gets all cookie stores, and logs each store's id and tabIds property. According to the docs (https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/Cookies/CookieStore), tabIds is an array of integers, where each integer is a Tab ID.

In Chrome, this is what you get. In Firefox, you get an array of Tab objects.

Comment 1

[Tomislav Jovanovic :zombie]

Attachment: Bug 1330167 - CookieStore.tabIds should be an array of tab IDs

Review commit: https://reviewboard.mozilla.org/r/104502/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/104502/

Comment 2

[Andrew Swan [:aswan]]

Comment on attachment 8826547 [details]
Bug 1330167 - CookieStore.tabIds should be an array of tab IDs

https://reviewboard.mozilla.org/r/104502/#review105206

::: toolkit/components/extensions/ext-cookies.js:425
(Diff revision 1)
>            let tabs = TabManager.for(extension).getTabs(window);
>            for (let tab of tabs) {
>              if (!(tab.cookieStoreId in data)) {
>                data[tab.cookieStoreId] = [];
>              }
> -            data[tab.cookieStoreId].push(tab);
> +            data[tab.cookieStoreId].push(tab.id);
>            }

This builds up a whole extension-facing Tab object for each tab only to discard everything except the id and the cookieStoreId.  But glancing through the code, I don't see any obvious way to address that without making much bigger changes than this really warrants...

Comment 3

[Tomislav Jovanovic :zombie]

Comment on attachment 8826547 [details]
Bug 1330167 - CookieStore.tabIds should be an array of tab IDs

https://reviewboard.mozilla.org/r/104502/#review105206

> This builds up a whole extension-facing Tab object for each tab only to discard everything except the id and the cookieStoreId.  But glancing through the code, I don't see any obvious way to address that without making much bigger changes than this really warrants...

*nod*

Comment 4

[(PTO June 19-July 7) Ryan VanderMeulen]

Autoland can't push this due to unresolved issues in MozReview.

Comment 5

[Tomislav Jovanovic :zombie]

My bad.  It's almost invisible in the new design.  :/

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b540a38f365f
CookieStore.tabIds should be an array of tab IDs r=aswan

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b540a38f365f