Anne (:annevk) Reporter
	Description • 7 years ago

Test: http://w3c-test.org/url/urlsearchparams-constructor.html, landed at https://github.com/w3c/web-platform-tests/pull/4523.

Standard: https://github.com/whatwg/url/issues/27, landed at https://github.com/whatwg/url/pull/175.

	Andrea Marchesini [:baku] Assignee
	Comment 1 • 7 years ago

record<> is not supported yet.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 2 • 7 years ago

Comment on attachment 8827079 [details] [diff] [review]
url.patch

> 
>-/* static */ already_AddRefed<URLSearchParams>
>-URLSearchParams::Constructor(const GlobalObject& aGlobal,
>-                             URLSearchParams& aInit,
>-                             ErrorResult& aRv)
>-{
>-  RefPtr<URLSearchParams> sp =
>-    new URLSearchParams(aGlobal.GetAsSupports(), aInit);
>-
>-  return sp.forget();
>-}

Why removing this is ok? Why do we have this non-standard ctor? Was it at some point in the spec?
Blink at least seems to allow URLSearchParams as a param to ctor, so it is very much not clear to me why this change is ok.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 3 • 7 years ago

Or is removing that ok since URLSearchParams has stringifier?

	Anne (:annevk) Reporter
	Comment 4 • 7 years ago

It's okay because URLSearchParams has iterable<>. The only observable difference is a custom iterator and I added a test for that.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 5 • 7 years ago

Comment on attachment 8827079 [details] [diff] [review]
url.patch

Ok, thanks Anne.


couldn't you in the test check that the right exception is thrown.

	Anne (:annevk) Reporter
	Comment 6 • 7 years ago

It would be better if we added those tests to web-platform-tests.

	Andrea Marchesini [:baku] Assignee
	Comment 8 • 7 years ago

Comment on attachment 8827371 [details] [diff] [review]
test for WPT

This patch has been sent as PR on github.

	Pulsebot
	Comment 9 • 7 years ago

Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/293533db8cc7
Construct URLSearchParams from sequence or from string, r=smaug

	Ryan VanderMeulen [:RyanVM]
	Comment 10 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/293533db8cc7

	Boris Zbarsky [:bzbarsky]
	Comment 11 • 7 years ago

Do you mind sending an intent to ship for this?

	Chris Mills [:cmills]
	Comment 12 • 7 years ago

I've had a go at updating the docs on this feature:

https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams
https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams/URLSearchParams
https://developer.mozilla.org/en-US/Firefox/Releases/53#DOM_HTML_DOM

And what I have there at the moment seems to work fine. However, I'm a little confused - the most up to date WebIDL says this:

[Constructor(optional (sequence<sequence<USVString>> or record<USVString, USVString> or USVString) init = ""),

Now, I might be wrong, but the way I understand it a sequence is basically an array of objects. And I'm not sure what a record is...

I've tried experimenting with this:

1. when I try to pass an array of parameter USVStrings into URLSearchParams(), it throws a typeerror in Fx and Chrome
2. when I try to pass an object containing parameter USVStrings into URLSearchParams(), it works, but then any call to the object instance (e.g. paramObj.get('foo')) just returns null, even if I know the param is present in one of the USVStrings.

Can you give me some quick examples of using a sequence or record in this context? Feel free to hit me up over mail/IRC to avoid polluting the bug too much.

	Anne (:annevk) Reporter
	Comment 13 • 7 years ago

Nested sequence of strings (note that the inner array has to be two items exactly): new URLSearchParams([["a", "1"], ["b", "2"]]).

Record: new URLSearchParams({a: "1", b: "2"}).

Thanks for documenting!

	Kohei Yoshino [:kohei]
	Comment 14 • 7 years ago

And the record (object) support has been added to Firefox 54 in Bug 1331580, not Firefox 53 in this bug.

	Chris Mills [:cmills]
	Comment 15 • 7 years ago

Cool, thanks both; I get it now. I've updated the ctor page to mention the sequence, but I still don't see the record syntax working in the latest nightly. Too new? I any case, I'll leave it for now, and fix it up when I come to write the Fx54 release notes.

	Anne (:annevk) Reporter
	Comment 16 • 7 years ago

It works fine here, 55.0a1 (2017-03-06) (64-bit).