1937477 - Enable Cookie-Store API

Status: RESOLVED FIXED

(Core :: Networking: Cookies, enhancement, P2)

Description

[Andrea Marchesini [:baku]]

The CookieStore API implementation is stable enough. We implemented the same subset as Safari; it's time to enable it for release.

Comment 1

[Andrea Marchesini [:baku]]

Attachment: Bug 1937477 - Enable CookieStore API, r?valentin

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6d55ff513cf8
Enable CookieStore API, r=valentin

Comment 3

[Norisz Fay [:noriszfay]]

Backed out as requested by developer

Backout link

Comment 4

[Andrea Marchesini [:baku]]

Attachment: Bug 1937477 - Enable CookieStore API, r?valentin

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a6718102d13e
Enable CookieStore API, r=valentin,webidl,emilio

Comment 6

[Serban Stanca [:SerbanS]]

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

Comment 7

[Donal Meehan [:dmeehan]]

:baku could you consider nominating this for a release note? (Process info)

Comment 8

[Andrea Marchesini [:baku]]

Release Note Request (optional, but appreciated)
[Why is this notable]: The lack of the CookieStore API introduced web-compact issues. Exposing this API offers an easier-to-use, async API to play with cookies.
[Affects Firefox for Android]: Yes
[Suggested wording]: The CookieStore API is an asynchronous cookie API to scripts running in HTML documents and service workers.
[Links (documentation, blog post, etc)]:

• MDN: https://developer.mozilla.org/en-US/docs/Web/API/CookieStore
• Spec: https://wicg.github.io/cookie-store/

Comment 9

[Donal Meehan [:dmeehan]]

Thanks, added to the Fx136 nightly release notes, please allow 30 minutes for the site to update.
Keeping the relnote-firefox flag as ? to keep it on the radar for inclusion in the final Fx136 release notes.

Comment 10

[Hamish Willee]

FF136 MDN docs for this can be tracked in https://github.com/mdn/content/issues/37929

From the compatibility data changes, would it be correct to say that the implementation of the Cookie Store API is partial?

Specifically, the implementation allows getting CookieStore in the main thread (window.cookieStore
) or in a service worker (ServiceWorkerGlobalScope.cookieStore) and getting cookiechange events in either context.

The "not implemented" part of this is

• Getting methods do not return partitioned information. Why is this? FF supports CHIPS now.
• CookieStoreManager and ServiceWorkerRegistration: cookies not implemented.
• ExtendableCookieChangeEvent not implemented.

Is that right?

Comment 11

[Hamish Willee]

@Donal, the https://www.mozilla.org/firefox/136.0a1/releasenotes/ don't seem quite right grammatically:

Added support for the CookieStore API, an asynchronous cookie API to scripts running in HTML documents and service workers.

Comment 12

[Andrea Marchesini [:baku]]

From the compatibility data changes, would it be correct to say that the implementation of the Cookie Store API is partial?

Yes, it's partially because we decided not to implement the CookieStoreManager and the ExtendableCookieChangeEvent. We also expose only names and cookie values and no other attributes (domain, path, sameSite, etc.).

Specifically, the implementation allows getting CookieStore in the main thread (window.cookieStore
) or in a service worker (ServiceWorkerGlobalScope.cookieStore) and getting cookiechange events in either context.

Cookiechange event is only a main-thread thing.

The "not implemented" part of this is

• Getting methods do not return partitioned information. Why is this? FF supports CHIPS now.

Because we do not expose more attributes than name and value, it's not about supporting CHIPS. It's about being compatible with the document.cookie. See this: https://searchfox.org/mozilla-central/source/dom/webidl/CookieStore.webidl#71-83

Note that, in the setter (CookieStore.set), we do support all the attributes (partitioned too): https://searchfox.org/mozilla-central/source/dom/webidl/CookieStore.webidl#54-62

• CookieStoreManager and ServiceWorkerRegistration: cookies not implemented.
• ExtendableCookieChangeEvent not implemented.

Is that right?

Correct.

Comment 13

[Donal Meehan [:dmeehan]]

(In reply to Hamish Willee from comment #11)

@Donal, the https://www.mozilla.org/firefox/136.0a1/releasenotes/ don't seem quite right grammatically:

Added support for the CookieStore API, an asynchronous cookie API to scripts running in HTML documents and service workers.

Tweaked it slightly

Comment 14

[Hamish Willee]

Thank you. I'm encountering some problems with this API, which I am hoping you can explain.

1. Cookies created using document.cookie cannot be deleted using the new API when served on MDN or github pages, but they can be locally.

   • Using the API locally (i.e. via npx http-server . -o -p 9999) I can use CookieStore to set, delete, list cookies as you would expect, whether they are created using the CookieStore or using document.cookie. However when served from MDN or github pages, delete code indicates that all cookies are deleted, but those created with document.cookie are still present when you list the cookies.
   • I would expect the delete to raise an exception in this case.
   • This behaviour true on FF and Chrome.
   • Test source code and github page - the current cookies are listed in the console after you press the list button.
   • Should this work/am I doing something wrong?

2. Setting a cookie in MDN environment silently fails.

   • In this PR, I create an example for setting a cookies with options (note, doesn't matter how you create the cookie, this is just one I have instrumented).
   • On FF NIghtly no error is raised - the set succeeds. However when I read the cookies created there aren't any.
   • On Chrome this gives and error Error setting cookie2: UnknownError: Failed to execute 'set' on 'CookieStore': An unknown error occurred while writing the cookie.
   • So firstly, am I doing something wrong/can you tell me what MDN might be doing wrong here.
   • and secondly, shouldn't FF know if it has failed here and raise an error?

3. The MDN "PLay" just fails with an HTTP 404. Something about these examples it does not like. Completely possible it is nothing to do with the CookieStore API, but I'm hopeful you might see something obvious.

Comment 15

[Andrea Marchesini [:baku]]

1. The issue with cookie deletion is in the path. If you do not pass the path, '/' is used by default. That does not match the cookie path set via document.cookie when the test runs on github. The path there is '/cookie-test'.
2. can you share the error you see in chrome? And can you provide a test case? Thanks!

Comment 16

[Hamish Willee]

Thanks Andrea,

1. Thanks very much. Would I be correct in thinking that delete() doesn't find a matching cookie it should just resolve (i.e. not report an error)? That's a bit annoying since you know you want to delete something, and you expected it to succeed.
2. Re the error in chrome, see the second example in https://pr38095.content.dev.mdn.mozit.cloud/en-US/docs/Web/API/CookieStore/set#setting_a_cookie_with_options - scroll down and select Show cookies.
   • This catches errors in setting the cookies, which shows: UnknownError: Failed to execute 'set' on 'CookieStore': An unknown error occurred while writing the cookie.
   • It then logs all cookies - so we see cookie2: Cookie not found since the cookie creation failed.
   • Note that the FIRST example in this doc also fails. However this doesn't show any exception. We still get the cookie2: Cookie not found since the cookie creation failed on both Chrome and firefox.
   • Sorry I can't give you something more predictable.

Comment 17

[Andrea Marchesini [:baku]]

1. Thanks very much. Would I be correct in thinking that delete() doesn't find a matching cookie it should just resolve (i.e. not report an error)? That's a bit annoying since you know you want to delete something, and you expected it to succeed.

Correct. Delete does not inform about what has been deleted.

2. Re the error in chrome, see the second example in https://pr38095.content.dev.mdn.mozit.cloud/en-US/docs/Web/API/CookieStore/set#setting_a_cookie_with_options - scroll down and select Show cookies.

Both Chrome and Firefox have similar behavior here. This is good. What is missing is a good error message. The issue is related to the 3rd party domain. I suspect you could have similar issues using document.cookie.

Comment 18

[Hamish Willee]

G'day, one last issue.

The following code creates two cookies with partitioned: true and then attempts to delete both of them, one using default options (just the name) and the other using partitioned: true. Deletion should succeed only on the second cookie that matches partitioned: true in its options.

If you run the code below you get different results on Chrome and Firefox:

• FF: Cookies remaining after attempted deletions (cookie2 should be deleted): cookie2
• Chrome: Cookies remaining after attempted deletions (cookie2 should be deleted): cookie1

I believe it is Firefox in error - it is matching the opposite of what it should. Can you confirm?

async function setTestCookies() {
  // Set two cookies
  try {
    await cookieStore.set({
      name: "cookie1",
      value: `cookie1-value`,
      partitioned: true,
    });
  } catch (error) {
    console.log(`Error setting cookie1: ${error}`);
  }

  try {
    await cookieStore.set({
      name: "cookie2",
      value: `cookie2-value`,
      partitioned: true,
    });
  } catch (error) {
    console.log(`Error setting cookie2: ${error}`);
  }

  // Log cookie names
  const cookieNames = (await cookieStore.getAll())
    .map((cookie) => cookie.name)
    .join(" ");
  console.log(`Initial cookies: ${cookieNames}`);
}

async function cookieTest() {
  //Create our test cookies
  await setTestCookies();

  // Delete cookie1 specifying just the name
  try {
    await cookieStore.delete("cookie1");
  } catch (error) {
    console.log(`Error deleting cookie1: ${error}`);
  }

  // Delete cookie2, setting partitioned to true
  try {
    await cookieStore.delete({
      name: "cookie2",
      partitioned: true,
    });
  } catch (error) {
    console.log(`Error deleting cookie2: ${error}`);
  }

  // Log cookie names again (to show cookie1 deleted)
  cookieNames = (await cookieStore.getAll())
    .map((cookie) => cookie.name)
    .join(" ");
  console.log(
    `Cookies remaining after attempted deletions (cookie2 should be deleted): ${cookieNames}`,
  );
}

cookieTest();

Comment 19

[Andrea Marchesini [:baku]]

Our implementation is changing. Can we revisit these issues as soon as we re-enable this API? Thanks!

Comment 20

[Hamish Willee]

Of course!