Closed Bug 1330678 Opened 3 years ago Closed 3 years ago

Construct URLSearchParams from array or object

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: annevk, Assigned: baku)

References

Details

(Keywords: dev-doc-complete)

Attachments

(2 files)

Depends on: 1330698
Depends on: 1330699
Attached patch url.patchSplinter Review
record<> is not supported yet.
Attachment #8827079 - Flags: review?(bugs)
Blocks: 1331333
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.
Attachment #8827079 - Flags: review?(bugs) → review-
Or is removing that ok since URLSearchParams has stringifier?
It's okay because URLSearchParams has iterable<>. The only observable difference is a custom iterator and I added a test for that.
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.
Attachment #8827079 - Flags: review- → review+
It would be better if we added those tests to web-platform-tests.
Attached patch test for WPTSplinter Review
Attachment #8827371 - Flags: review?(annevk)
Comment on attachment 8827371 [details] [diff] [review]
test for WPT

This patch has been sent as PR on github.
Attachment #8827371 - Flags: review?(annevk)
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/293533db8cc7
Construct URLSearchParams from sequence or from string, r=smaug
Blocks: 1331580
https://hg.mozilla.org/mozilla-central/rev/293533db8cc7
Status: NEW → RESOLVED
Closed: 3 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Do you mind sending an intent to ship for this?
Flags: needinfo?(amarchesini)
Flags: needinfo?(amarchesini)
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.
Flags: needinfo?(amarchesini)
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!
Flags: needinfo?(amarchesini)
And the record (object) support has been added to Firefox 54 in Bug 1331580, not Firefox 53 in this bug.
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.
It works fine here, 55.0a1 (2017-03-06) (64-bit).
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.