Skip to content
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

URL#searchParams#delete and "?" delimiter #332

Closed
stevenvachon opened this issue Jul 6, 2017 · 5 comments
Closed

URL#searchParams#delete and "?" delimiter #332

stevenvachon opened this issue Jul 6, 2017 · 5 comments

Comments

@stevenvachon
Copy link

stevenvachon commented Jul 6, 2017

In web-platform-tests/wpt#6445 (comment) we are suggesting that this should be true:

const url = new URL("http://host/?param");
url.searchParams.delete("param");
url.href;  //-> http://host/?

I cannot see a valid top-level reason to preserve the trailing delimiter when the user of URL has chosen to make a mutation.

@annevk
Copy link
Member

annevk commented Jul 12, 2017

@TimothyGu @achristensen07 I'm inclined to fix this given the surprise in both the Chrome and Firefox bugs. That would mean WebKit has to change its behavior. Though I think Firefox should probably still change to remove the question mark when you remove a non-existent parameter.

My proposal would be to change https://url.spec.whatwg.org/#concept-urlsearchparams-update so that if the serialization is the empty string we set query to null.

@TimothyGu
Copy link
Member

@annevk Alright. I'd still slightly prefer Firefox's behavior over Chrome's, but I'm fine with the path of least resistance, whatever that is.

And as one more data point, Node.js already follows Chrome's behavior after nodejs/node#10480.

@annevk
Copy link
Member

annevk commented Jul 13, 2017

PR up. Will need to file bugs against Firefox (likely just update https://bugzilla.mozilla.org/show_bug.cgi?id=1379113) and Safari. (Edge doesn't support URLSearchParams yet.)

https://bugs.chromium.org/p/chromium/issues/detail?id=740048 can be closed if this is approved.

@achristensen07
Copy link
Collaborator

I agree with this conceptually and have changed WebKit in https://bugs.webkit.org/show_bug.cgi?id=174465

hubot pushed a commit to WebKit/WebKit-http that referenced this issue Jul 13, 2017
…ted URL

https://bugs.webkit.org/show_bug.cgi?id=174465

Patch by Alex Christensen <[email protected]> on 2017-07-13
Reviewed by Chris Dumez.

Source/WebCore:

This makes us match the behavior of Chrome and Firefox, and the spec after whatwg/url#332 is approved.
This will be covered by an upcoming web platform test, and I updated fast/dom/DOMURL/searchparams.html to cover it now.

* platform/URLParser.cpp:
(WebCore::URLParser::serialize):
If there are no tuples, serialize to the null string instead of a non-null empty string.
This makes it so URL::setQuery removes the ?

LayoutTests:

* fast/dom/DOMURL/searchparams-expected.txt:
* fast/dom/DOMURL/searchparams.html:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@219458 268f45cc-cd09-0410-ab3c-d52691b4dbfc
@annevk
Copy link
Member

annevk commented Jul 14, 2017

Thanks @achristensen07 and @stevenvachon for pushing this!

@annevk annevk removed the needs implementer interest Moving the issue forward requires implementers to express interest label Jul 14, 2017
ryanhaddad pushed a commit to WebKit/WebKit that referenced this issue Dec 22, 2020
…ted URL

https://bugs.webkit.org/show_bug.cgi?id=174465

Patch by Alex Christensen <[email protected]> on 2017-07-13
Reviewed by Chris Dumez.

Source/WebCore:

This makes us match the behavior of Chrome and Firefox, and the spec after whatwg/url#332 is approved.
This will be covered by an upcoming web platform test, and I updated fast/dom/DOMURL/searchparams.html to cover it now.

* platform/URLParser.cpp:
(WebCore::URLParser::serialize):
If there are no tuples, serialize to the null string instead of a non-null empty string.
This makes it so URL::setQuery removes the ?

LayoutTests:

* fast/dom/DOMURL/searchparams-expected.txt:
* fast/dom/DOMURL/searchparams.html:

Canonical link: https://commits.webkit.org/191286@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@219458 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants