-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Cookie Store API: empty string path defaulting #54264
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
There are no reviewers for this pull request. Please reach out on the chat room to get help with this. Thank you! |
https://bugs.webkit.org/show_bug.cgi?id=297268 Reviewed by NOBODY (OOPS!). Implement whatwg/cookiestore#283 with tests upstreamed at web-platform-tests/wpt#54264
https://bugs.webkit.org/show_bug.cgi?id=297268 Reviewed by NOBODY (OOPS!). Implement whatwg/cookiestore#283 with tests upstreamed at web-platform-tests/wpt#54264
https://bugs.webkit.org/show_bug.cgi?id=297268 Reviewed by NOBODY (OOPS!). Implement whatwg/cookiestore#283 with tests upstreamed at web-platform-tests/wpt#54264
https://bugs.webkit.org/show_bug.cgi?id=297268 Reviewed by NOBODY (OOPS!). Implement whatwg/cookiestore#283 with tests upstreamed at web-platform-tests/wpt#54264
| assert_equals(internalCookie.path, currentDirectory); | ||
| }, 'CookieListItem - cookieStore.set with empty string path defaults to current URL'); | ||
|
|
||
| promise_test(async testCase => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't get this one, why is the cookie rejected? Doesn't this pose some back-compat risk?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It rejects in Chrome today. The reason is that the path becomes /cookiestore and it has to be / for a __host--prefixed cookie.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Optionally, it might be worthwhile to add a comment to that effect.
yoavweiss
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Also correct the algorithm as it incorrectly assumed that path could be null when in fact it's always a string. Tests: web-platform-tests/wpt#54264. Closes #242.
https://bugs.webkit.org/show_bug.cgi?id=297268 Reviewed by Rupin Mittal. Implement whatwg/cookiestore#283 with tests upstreamed at web-platform-tests/wpt#54264 Canonical link: https://commits.webkit.org/298681@main
For whatwg/cookiestore#283.