Summary: A JavaScript developer can clear out the search portion of a URL object by using the search setter with the value ''. In Safari TP, doing so results in a URL object that includes a trailing '?' character, which does not appear to match the behavior in other browsers that support the URL object. Other browsers stringify that URL object without a trailing '?'. Steps to Reproduce: In any JavaScript execution context, e.g. the Safari Developer Tools' console, run the following: var url = new URL('https://example.com/?testing'); url.search = ''; console.log(url.toString()); Expected Results: The expected logged value is 'https://example.com/' Actual Results: The actual logged value is 'https://example.com/?' Version: Release 13 (Safari 10.0, WebKit 11603.1.5) Notes: The relevant specification is at https://url.spec.whatwg.org/?a=b#concept-urlsearchparams-update All other modern browsers (Chrome, Firefox, and Edge) appear to properly stringify that example without a trailing '?'. Configuration: This happens all the time.
This is also an issue when using URLSearchParams: const url = new URL('https://foo.com/?bar=baz'); url.searchParams.delete('bar'); console.log(url.toString()); This will include an extra ?
<rdar://problem/31800441>
Created attachment 308026 [details] Patch
Comment on attachment 308026 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=308026&action=review > LayoutTests/imported/w3c/web-platform-tests/url/url-setters-expected.txt:545 > +FAIL URL: Setting <https://example.net?lang=en-US#nav>.search = '?' assert_equals: expected "https://example.net/?#nav" but got "https://example.net/#nav" > +FAIL <a>: Setting <https://example.net?lang=en-US#nav>.search = '?' assert_equals: expected "https://example.net/?#nav" but got "https://example.net/#nav" > +FAIL <area>: Setting <https://example.net?lang=en-US#nav>.search = '?' assert_equals: expected "https://example.net/?#nav" but got "https://example.net/#nav" Anne, these tests fail in Chrome and will fail in WebKit if we make this change. They pass in Firefox, which is against my interpretation of https://url.spec.whatwg.org/#dom-url-search setter step 3. What do you think?
Comment on attachment 308026 [details] Patch Attachment 308026 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/3598005 New failing tests: fast/forms/get-forms-to-about-blank.html fast/forms/submit-nil-value-field-assert.html http/tests/navigation/onload-navigation-iframe.html http/tests/navigation/onload-navigation-iframe-timeout.html fast/forms/empty-get.html http/tests/navigation/back-send-referrer.html http/tests/history/redirect-js-form-submit-2-seconds.html http/tests/download/inherited-encoding-form-submission-result.html fast/loader/form-submission-before-load-get.html fast/history/gesture-before-onload-form-submit.html http/tests/navigation/onload-navigation-iframe-2.html fast/forms/missing-action.html fast/dom/HTMLAnchorElement/set-href-attribute-search.html http/tests/history/redirect-js-form-submit-0-seconds.html http/tests/history/redirect-js-form-submit-before-load.html
Created attachment 308037 [details] Archive of layout-test-results from ews113 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews113 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 308026 [details] Patch Attachment 308026 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/3598104 New failing tests: fast/forms/submit-nil-value-field-assert.html http/tests/navigation/onload-navigation-iframe.html http/tests/history/redirect-js-form-submit-2-seconds.html http/tests/navigation/onload-navigation-iframe-timeout.html fast/forms/empty-get.html http/tests/navigation/back-send-referrer.html http/tests/download/inherited-encoding-form-submission-result.html fast/dom/HTMLAnchorElement/set-href-attribute-search.html fast/forms/missing-action.html fast/history/gesture-before-onload-form-submit.html http/tests/navigation/onload-navigation-iframe-2.html fast/loader/form-submission-before-load-get.html http/tests/history/redirect-js-form-submit-before-load.html http/tests/history/redirect-js-form-submit-0-seconds.html fast/forms/get-forms-to-about-blank.html
Created attachment 308038 [details] Archive of layout-test-results from ews100 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 308026 [details] Patch Attachment 308026 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/3598102 New failing tests: fast/forms/submit-nil-value-field-assert.html fast/forms/get-forms-to-about-blank.html http/tests/navigation/onload-navigation-iframe-timeout.html fast/forms/empty-get.html http/tests/navigation/back-send-referrer.html http/tests/history/redirect-js-form-submit-2-seconds.html fast/dom/HTMLAnchorElement/set-href-attribute-search.html fast/forms/missing-action.html fast/history/gesture-before-onload-form-submit.html http/tests/navigation/onload-navigation-iframe-2.html fast/loader/form-submission-before-load-get.html http/tests/history/redirect-js-form-submit-before-load.html http/tests/history/redirect-js-form-submit-0-seconds.html http/tests/navigation/onload-navigation-iframe.html
Created attachment 308040 [details] Archive of layout-test-results from ews106 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 308026 [details] Patch Attachment 308026 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/3598178 New failing tests: fast/forms/submit-nil-value-field-assert.html fast/forms/get-forms-to-about-blank.html http/tests/navigation/onload-navigation-iframe-timeout.html fast/forms/empty-get.html http/tests/navigation/back-send-referrer.html http/tests/history/redirect-js-form-submit-2-seconds.html fast/dom/HTMLAnchorElement/set-href-attribute-search.html http/tests/navigation/onload-navigation-iframe.html http/tests/navigation/onload-navigation-iframe-2.html fast/forms/missing-action.html http/tests/history/redirect-js-form-submit-before-load.html http/tests/history/redirect-js-form-submit-0-seconds.html
Created attachment 308047 [details] Archive of layout-test-results from ews125 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews125 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.6
(In reply to Alex Christensen from comment #4) > Anne, these tests fail in Chrome and will fail in WebKit if we make this > change. They pass in Firefox, which is against my interpretation of > https://url.spec.whatwg.org/#dom-url-search setter step 3. What do you > think? I think you're reading it wrong or WebKit has perhaps the wrong internal model for a URL query. There's a distinction between query being null and it being the empty string. There's also a distinction between setting url.search to "" (makes query null) and setting it to "?" (makes query the empty string). Does that help?
Comment on attachment 308026 [details] Patch Yep, thanks Anne. Looks like I'll need to do some more serious URL surgery.
Created attachment 310343 [details] Patch
Created attachment 310344 [details] Patch
(In reply to Chris Dumez from comment #16) > Created attachment 310344 [details] > Patch I'd like Alex to review this.
Comment on attachment 310344 [details] Patch Are there no tests in LayoutTests/imported/w3c/web-platform-tests/url/url-setters.html that start passing because of this change? If not, then I think something is wrong. Maybe it's skipped or marked as pass/fail from the days before URLParser when it used to assert.
(In reply to Alex Christensen from comment #18) > Comment on attachment 310344 [details] > Patch > > Are there no tests in > LayoutTests/imported/w3c/web-platform-tests/url/url-setters.html that start > passing because of this change? If not, then I think something is wrong. > Maybe it's skipped or marked as pass/fail from the days before URLParser > when it used to assert. Test seems to be skipped on wk1 but not wk2. I can check if the test is somehow outdated. I don't know if this test is supposed to cover the issue in the bug. You can see however that the tests I updated / rebaselined covers *exactly* the case covered in this bug and that we now get the correct result. Given this, saying if no WPT tests started passing, then the patch is wrong, seems odd. Anyway, I'll take a look at url-setters.html to see if it covers this case and if it is up-to-date.
(In reply to Chris Dumez from comment #19) > (In reply to Alex Christensen from comment #18) > > Comment on attachment 310344 [details] > > Patch > > > > Are there no tests in > > LayoutTests/imported/w3c/web-platform-tests/url/url-setters.html that start > > passing because of this change? If not, then I think something is wrong. > > Maybe it's skipped or marked as pass/fail from the days before URLParser > > when it used to assert. > > Test seems to be skipped on wk1 but not wk2. I can check if the test is > somehow outdated. I don't know if this test is supposed to cover the issue > in the bug. > > You can see however that the tests I updated / rebaselined covers *exactly* > the case covered in this bug and that we now get the correct result. Given > this, saying if no WPT tests started passing, then the patch is wrong, seems > odd. > > Anyway, I'll take a look at url-setters.html to see if it covers this case > and if it is up-to-date. I looked at url-setters.html and it is up-to-date. It also seems like it should cover this case (or similar cases at least). I'll investigate why the bots do not see new passes.
(In reply to Chris Dumez from comment #20) > (In reply to Chris Dumez from comment #19) > > (In reply to Alex Christensen from comment #18) > > > Comment on attachment 310344 [details] > > > Patch > > > > > > Are there no tests in > > > LayoutTests/imported/w3c/web-platform-tests/url/url-setters.html that start > > > passing because of this change? If not, then I think something is wrong. > > > Maybe it's skipped or marked as pass/fail from the days before URLParser > > > when it used to assert. > > > > Test seems to be skipped on wk1 but not wk2. I can check if the test is > > somehow outdated. I don't know if this test is supposed to cover the issue > > in the bug. > > > > You can see however that the tests I updated / rebaselined covers *exactly* > > the case covered in this bug and that we now get the correct result. Given > > this, saying if no WPT tests started passing, then the patch is wrong, seems > > odd. > > > > Anyway, I'll take a look at url-setters.html to see if it covers this case > > and if it is up-to-date. > > I looked at url-setters.html and it is up-to-date. It also seems like it > should cover this case (or similar cases at least). I'll investigate why the > bots do not see new passes. The test must be skipped as I see this when rebaselining it locally: -FAIL URL: Setting <https://example.net?lang=en-US#nav>.search = '' assert_equals: expected "https://example.net/#nav" but got "https://example.net/?#nav" -FAIL <a>: Setting <https://example.net?lang=en-US#nav>.search = '' assert_equals: expected "https://example.net/#nav" but got "https://example.net/?#nav" -FAIL <area>: Setting <https://example.net?lang=en-US#nav>.search = '' assert_equals: expected "https://example.net/#nav" but got "https://example.net/?#nav" -FAIL URL: Setting <https://example.net?lang=en-US>.search = '' assert_equals: expected "https://example.net/" but got "https://example.net/?" -FAIL <a>: Setting <https://example.net?lang=en-US>.search = '' assert_equals: expected "https://example.net/" but got "https://example.net/?" -FAIL <area>: Setting <https://example.net?lang=en-US>.search = '' assert_equals: expected "https://example.net/" but got "https://example.net/?" -FAIL URL: Setting <https://example.net>.search = '' assert_equals: expected "https://example.net/" but got "https://example.net/?" -FAIL <a>: Setting <https://example.net>.search = '' assert_equals: expected "https://example.net/" but got "https://example.net/?" -FAIL <area>: Setting <https://example.net>.search = '' assert_equals: expected "https://example.net/" but got "https://example.net/?" +PASS URL: Setting <https://example.net?lang=en-US#nav>.search = '' +PASS <a>: Setting <https://example.net?lang=en-US#nav>.search = '' +PASS <area>: Setting <https://example.net?lang=en-US#nav>.search = '' +PASS URL: Setting <https://example.net?lang=en-US>.search = '' +PASS <a>: Setting <https://example.net?lang=en-US>.search = '' +PASS <area>: Setting <https://example.net?lang=en-US>.search = '' +PASS URL: Setting <https://example.net>.search = '' +PASS <a>: Setting <https://example.net>.search = '' +PASS <area>: Setting <https://example.net>.search = ''
Created attachment 310425 [details] Patch
(In reply to Chris Dumez from comment #22) > Created attachment 310425 [details] > Patch Rebaselined the WPT and unskipped it for WK1. I cannot find any reason why it would have been skipped for WK2.
Comment on attachment 310425 [details] Patch Clearing flags on attachment: 310425 Committed r217004: <http://trac.webkit.org/changeset/217004>
All reviewed patches have been landed. Closing bug.