1958875 - Re-enable CookieStore API

Status: RESOLVED FIXED

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

Description

[edgul]

The CookieStore API was disabled temporarily.
This bug will re-enable it.

Comment 1

[Andrea Marchesini [:baku]]

Attachment: Bug 1958875 - Re-enable CookieStore API, r?edgul

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5da3002a62fc
Re-enable CookieStore API, r=edgul

Comment 3

[Atila Butkovits]

Backed out for causing failures at test_serviceworker_interfaces.html.

Backout link: https://hg-edge.mozilla.org/integration/autoland/rev/514fd2961000a68d328d1e75d6946e20167556f7

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=5da3002a62fceab68bb4b42ac956f20f81df6858&selectedTaskRun=LwtTaQQPQ0KMm6aBSxxgXA.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=504507159&repo=autoland&lineNumber=2430

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7aeb1bd11e48
Re-enable CookieStore API, r=edgul,webidl,smaug,valentin

Comment 6

[Cristian Tuns]

Backed out for causing mochitest failures in /test_interfaces_secureContext.html

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | dom/tests/mochitest/general/test_interfaces_secureContext.html | If this is failing: DANGER, are you sure you want to expose the new interface CookieStoreManager to all webpages as a property on 'window'? Do not make a change to this file without a review from a DOM peer for that specific change!!! (or a JS peer for changes to ecmaGlobals)

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f781ab633afe
Re-enable CookieStore API, r=edgul,webidl,smaug,valentin

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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

Comment 9

[Donal Meehan [:dmeehan]]

:baku want to add a release note back for this?

This was the wording used when it was added to the Fx136 release notes before it was later disabled.

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

Are any changes needed to the wording?

Comment 10

[Sebastian Zartner [:sebo]]

I'd say the wording should now be something like "The CookieStore API got re-enabled. It provides methods for getting and setting cookies asynchronously from either a page or a service worker." (reflecting the wording on MDN).

I am also wondering whether it is worth mentioning that circumstance in the browser compat data, as well, meaning it was enabled in 136.0, then disabled in 136.0.2 and got re-enabled now for 140. Hamish, what do you think?

Sebastian

Comment 11

[Andrea Marchesini [:baku]]

The wording is fine. Thanks.

Comment 12

[Hamish Willee]

FWIW Donal wording is great. There is no need to mention the re-enabling because this history is not visible/relevant to external developers (exposed and undone in same release)

Comment 13

[Donal Meehan [:dmeehan]]

Thanks, added to the Fx140 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 Fx140 release notes.

Comment 14

[Hamish Willee]

FF140 MDN docs work for this can be tracked in https://github.com/mdn/content/issues/39612

Comment 15

[Hamish Willee]

OK, just found out from the compat data that this also supports:

• CookieStoreManager
• ExtendableCookieChangeEvent
• ServiceWorkerGlobalScope cookiechange event
• ServiceWorkerRegistration.cookies

Added in https://bugzilla.mozilla.org/show_bug.cgi?id=1947894 which was not tagged as needing docs (though they seem OK anyway)

1. Did anything else major change in the API from when it was implemented in https://bugzilla.mozilla.org/show_bug.cgi?id=1475599

   For example, when we did docs for that issue, it was stated that Firefox has chosen to only return name and value from get and in change events. Is this still the case?

2. Docs for ExtendableCookieChangeEvent indicate that this event is fired with type = changed for new cookies that are not immediately deleted, and with type = deleted for cookies that are deleted. There is also a note:

   A cookie that is replaced due to the insertion of another cookie with the same name, domain, and path, is ignored and does not trigger a change event.

   Is that all correct? I.e. a new cookie is referred to as "changed" but you can't actually change a cookie - if you do so you are replacing it, and so the event is not fired?

Comment 16

[Andrea Marchesini [:baku]]

• ExtendableCookieChangeEvent

All looks right except the 'partitioned' property. That is not part of the spec.

1. Did anything else major change in the API from when it was implemented in https://bugzilla.mozilla.org/show_bug.cgi?id=1475599

No. We just included the SW support (bug 1947894). The documentation is correct.

For example, when we did docs for that issue, it was stated that Firefox has chosen to only return name and value from get and in change events. Is this still the case?

Yes it is.

A cookie that is replaced due to the insertion of another cookie with the same name, domain, and path, is ignored and does not trigger a change event.

No. This is false. Any cookie change generates an event. This line needs to be updated.

Comment 17

[Hamish Willee]

I've marked this as complete because I've finished most of the work (waiting for reviews).

ExtendableCookieChangeEvent
All looks right except the 'partitioned' property. That is not part of the spec.

According to the linked spec the thing that has the properties is a CookieListItem, and in that partitioned does appear.
It is according to the linked spec: https://wicg.github.io/cookie-store/#dictdef-cookielistitem