.
Created attachment 443719 [details] patch
Comment on attachment 443719 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=443719&action=review > LayoutTests/platform/wincairo/imported/w3c/web-platform-tests/cookies/ordering/ordering.sub-expected.txt:4 > +FAIL Cookies with longer path attribute values are ordered before shorter ones (2) assert_equals: The cookie was set as expected. expected "testG=2; testB=2; testF=2; testH=2; testC=2" but got "testB=2; testH=2; testC=2" Don't add FAIL result as a expected result. Mark the test as Failure.
Comment on attachment 443719 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=443719&action=review > LayoutTests/platform/wincairo/imported/w3c/web-platform-tests/cookies/navigated-away-expected.txt:1 > + Why do we have to add WinCairo specific navigated-away-expected.txt? Why can't WinCairo use the common navigated-away-expected.txt?
Comment on attachment 443719 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=443719&action=review > Source/WebCore/platform/network/curl/NetworkStorageSessionCurl.cpp:71 > cookies.append(cookie.value); Do you need to set the value even if the name is empty?
Comment on attachment 443719 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=443719&action=review > Source/WebCore/platform/network/curl/CookieUtil.cpp:137 > + result.path = emptyString(); Only path? What about other parameters? For example, if the second expires parameter has a invalid value, should it override the first one?
<rdar://problem/85310326>
Comment on attachment 443719 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=443719&action=review >> LayoutTests/platform/wincairo/imported/w3c/web-platform-tests/cookies/navigated-away-expected.txt:1 >> + > > Why do we have to add WinCairo specific navigated-away-expected.txt? Why can't WinCairo use the common navigated-away-expected.txt? Yeah I can clean it up, it was a bit tricky because I couldn't find a way to easily generate baseline files only for tests that don't match expected results. Also top level WPT baseline has FAIL results in the baseline so the expected results get a bit messy. I'll try to clean up the expected results some more. >> Source/WebCore/platform/network/curl/CookieUtil.cpp:137 >> + result.path = emptyString(); > > Only path? What about other parameters? For example, if the second expires parameter has a invalid value, should it override the first one? According to the spec for expires "If the attribute-value failed to parse as a cookie date, ignore the cookie-av." However checking both chrome and firefox, setting invalid or empty string expires/max-age will override previous attribute-values and set a session cookie. For domain we match chrome's behavior. Testing on trac.webkit.org with chrome: document.cookie = "foo=test; Domain=webkit.org; Domain=" => Sets Cookie with domain=.webkit.org document.cookie = "foo=test; Domain=webkit.org; Domain=google.com" => Doesn't set cookie I'll update the domain and max-age parsing to override previous attribute-values. >> Source/WebCore/platform/network/curl/NetworkStorageSessionCurl.cpp:71 >> cookies.append(cookie.value); > > Do you need to set the value even if the name is empty? Yeah, checking chrome you can set a cookie value to the empty string name. Doing `document.cookie = "=test"` would add `test` to the list of cookies with an empty name. Doing `document.cookie = "=test2"` would override the first cookie since they both are setting cookies on the empty string.
Created attachment 447060 [details] patch Updated patch, only adding expectations files for ones that should now all pass or have more passes than the w3c baseline.
Comment on attachment 447060 [details] patch LGTM
Committed r287000 (245212@main): <https://commits.webkit.org/245212@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 447060 [details].