[webkit-reviews] review granted: [Bug 238202] Computation of Document siteForCookies is buggy in case document is created by window.open : [Attachment 455381] Patch

[bugzilla-daemon at webkit.org]

John Wilander <wilander at apple.com> has granted youenn fablet
<youennf at gmail.com>'s request for review:
Bug 238202: Computation of Document siteForCookies is buggy in case document is
created by window.open
https://bugs.webkit.org/show_bug.cgi?id=238202

Attachment 455381: Patch

https://bugs.webkit.org/attachment.cgi?id=455381&action=review




--- Comment #5 from John Wilander <wilander at apple.com> ---
Comment on attachment 455381
  --> https://bugs.webkit.org/attachment.cgi?id=455381
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=455381&action=review

r=me based on enhancing the tests (and getting green EWS bots).

> LayoutTests/http/tests/cookies/resources/testharness-helpers.js:38
> +    document.cookie = LAX_DOM + "=1; SameSite=Lax; Max-Age=100; path=/";

Please add a SameSite=Strict cookie too and make sure it works as expected.

> LayoutTests/http/tests/cookies/resources/testharness-helpers.js:68
> +    document.cookie = LAX_DOM + "=1; SameSite=Lax; Max-Age=100; path=/";

Ditto on a SameSite=Strict cookie.

>
LayoutTests/http/tests/cookies/same-site/popup-from-iframe-same-site-with-post-
form-expected.txt:2
> +PASS popup opened as 'about:blank', then post navigation to 127.0.0.1, so
samesite cookies are sent.

This output should be more specific and say whether SameSite Lax and SameSite
Strict cookies were sent.

>
LayoutTests/http/tests/cookies/same-site/popup-from-iframe-same-site-with-post-
form-expected.txt:3
> +PASS popup opened as '127.0.0.1', then post navigation to 127.0.0.1, so
samesite cookies are sent.

Ditto.

>
LayoutTests/http/tests/cookies/same-site/popup-from-iframe-same-site-with-post-
form-expected.txt:4
> +PASS popup loaded as '127.0.0.1', then post navigation to 127.0.0.1, so
samesite cookies are sent.

Ditto, plus I would like if this test output was distinct from the one above.
Could we add more details so that it's clear what's being tested?

>
LayoutTests/http/tests/cookies/same-site/popup-same-site-with-post-form-expecte
d.txt:2
> +PASS popup opened as 'about:blank', then post navigation to 127.0.0.1, so
samesite cookies are sent.

Ditto on cookie details.

>
LayoutTests/http/tests/cookies/same-site/popup-same-site-with-post-form-expecte
d.txt:3
> +PASS popup opened as '127.0.0.1', then post navigation to 127.0.0.1, so
samesite cookies are sent.

Ditto.

>
LayoutTests/http/tests/cookies/same-site/popup-same-site-with-post-form-expecte
d.txt:4
> +PASS popup loaded as '127.0.0.1', then post navigation to 127.0.0.1, so
samesite cookies are sent.

Ditto, plus the comment on making it distinct.