Intent to implement and ship: change argument type in URLSearchParams' constructor

66 views
Skip to first unread message

Raphael Kubo da Costa

unread,
Jun 23, 2017, 11:21:34 AM6/23/17
to blin...@chromium.org
Intent to Implement/Ship
------------------------
Change URLSearchparams' constructor from
Constructor(optional (sequence<sequence<USVString>> or USVString or URLSearchParams) init = "")
to
Constructor(optional (sequence<sequence<USVString>> or record<USVString, USVString> or USVString) init = ""),

Contact emails
--------------
raphael.ku...@intel.com

Spec
----
https://url.spec.whatwg.org/#interface-urlsearchparams

Summary
-------
In January 2017, https://github.com/whatwg/url/pull/175 was merged into
the URL spec. It changed the union in URLSearchParams' constructor from
(USVString or URLSearchParams)
to the long one mentioned above.

Blink's implementation gained support for sequence<sequence<USVString>>
in March, but we still didn't support records and explicitly accepted
URLSearchParams in the union.

By supporting records and sequences in the union we also accept any
object; per WebIDL, if the object has a @@iterator (URLSearchParams
does), it is converted to a sequence, otherwise it's treated as a
record.

Motivation
----------
Align Blink's implementation with the spec, which also makes us pass
more tests from web-platform-tests and reduce interoperability issues.

Interoperability risk
---------------------
Firefox:
- Dropped URLSearchParams from the constructor in 53
https://bugzilla.mozilla.org/show_bug.cgi?id=1330678
- Added record<USVString, USVString> to the union in 54
https://bugzilla.mozilla.org/show_bug.cgi?id=1331580

Edge:
- Does not seem to implement URLSearchParams at all
https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/8993198/

WebKit:
- Implemented a similar change in January 2017
https://bugs.webkit.org/show_bug.cgi?id=166973

Compatibility risk
------------------
Passing a URLSearchParams to the constructor is still supported, but it
is processed differently. While we used to have URLSearchParams as a
union member in the past and just copy the parameters to the new object,
we now initialize the new object by converting the existing
URLSearchParams to a sequence<sequence<USVString, USVString>> (I don't
see a big difference in practice).

Passing an object without an @@iterator symbol (e.g. `new
URLSearchParams({})`) now causes the object to be processed as a record
instead of as a plain string. In other words,
new URLSearchParams({}).toString()
used to return "%5Bobject+Object%5D=", and it returns an empty string
now.

Is this feature supported on all six Blink platforms?
-----------------------------------------------------
Yes.

Is this feature fully tested by web-platform-tests?
---------------------------------------------------
Yes.

Related bug report
------------------
https://bugs.chromium.org/p/chromium/issues/detail?id=697378

Requesting approval to ship the behavior change
-----------------------------------------------
Yes, in master (M61).

Rick Byers

unread,
Jun 23, 2017, 4:11:05 PM6/23/17
to Raphael Kubo da Costa, blink-dev
If I'm understanding correctly, the web exposed difference here is quite subtle and extremely unlikely for anyone to notice.  Firefox successfully shipping this change backs that up.  Given that I don't think there's any value in adding a chromestatus entry or mentioning this change in the blog post etc. (arguably it's just a tiny bugfix and not really a web exposed change worthy of an intent at all).

LGTM1

On Fri, Jun 23, 2017 at 11:21 AM, Raphael Kubo da Costa <raphael.ku...@intel.com> wrote:
Intent to Implement/Ship
------------------------
Change URLSearchparams' constructor from
    Constructor(optional (sequence<sequence<USVString>> or USVString or URLSearchParams) init = "")
to
    Constructor(optional (sequence<sequence<USVString>> or record<USVString, USVString> or USVString) init = ""),

Contact emails
--------------

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/87vanmybke.fsf%40rkubodac-desk.ger.corp.intel.com.


Domenic Denicola

unread,
Jun 23, 2017, 4:37:08 PM6/23/17
to Rick Byers, Raphael Kubo da Costa, blink-dev
From: Rick Byers [mailto:rby...@chromium.org]

> If I'm understanding correctly, the web exposed difference here is quite subtle and extremely unlikely for anyone to notice.  Firefox successfully shipping this change backs that up.  Given that I don't think there's any value in adding a chromestatus entry or mentioning this change in the blog post etc. (arguably it's just a tiny bugfix and not really a web exposed change worthy of an intent at all).

While I agree with your compat assessment, I do think developers might enjoy hearing about this via chromestatus/the blog post. It allows them to go from code like

new URLSearchParams([
['k1', 'v1'],
['k2', 'v2]
]);

to

new URLSearchParams({
k1: 'v1',
k2: 'v2'
});

which was a big developer ask on the whatwg/url spec repo.

Rick Byers

unread,
Jun 23, 2017, 5:45:19 PM6/23/17
to Domenic Denicola, Raphael Kubo da Costa, blink-dev
Ah, that's a good point - I was thinking only of the breaking change side, not the shipping something new side.  Sorry!

Yes please add a chromestatus entry with either a link or just an example like the above in the comments.

Frank Cuthbert

unread,
Jun 23, 2017, 11:09:25 PM6/23/17
to blink-dev

mk...@chromium.org

unread,
Jun 24, 2017, 2:39:45 AM6/24/17
to blink-dev
LGTM2, but I agree that we should have a chromestatus entry so we mention this in the blog post.


On Friday, June 23, 2017 at 5:21:34 PM UTC+2, Raphael Kubo Da Costa wrote:
Intent to Implement/Ship
------------------------
Change URLSearchparams' constructor from
    Constructor(optional (sequence<sequence<USVString>> or USVString or URLSearchParams) init = "")
to
    Constructor(optional (sequence<sequence<USVString>> or record<USVString, USVString> or USVString) init = ""),

Contact emails
--------------

Raphael Kubo da Costa

unread,
Jun 26, 2017, 5:03:42 AM6/26/17
to Rick Byers, Domenic Denicola, blink-dev
Rick Byers <rby...@chromium.org> writes:
> Yes please add a chromestatus entry with either a link or just an example
> like the above in the comments.

I've created https://www.chromestatus.com/feature/5737549880360960 with
all the information I have so far; please let me know if a bigger
example is needed.

I've also used http://crbug.com/680531 as the launch bug instead of
https://crbug.com/697378 because the former includes more information
relevant to developers than the latter.

Raphael Kubo Da Costa

unread,
Jun 27, 2017, 2:48:25 AM6/27/17
to blink-dev
Anyone willing to lgtm3 this intent?


On Friday, 23 June 2017 17:21:34 UTC+2, Raphael Kubo Da Costa wrote:
Intent to Implement/Ship
------------------------
Change URLSearchparams' constructor from
    Constructor(optional (sequence<sequence<USVString>> or USVString or URLSearchParams) init = "")
to
    Constructor(optional (sequence<sequence<USVString>> or record<USVString, USVString> or USVString) init = ""),

Contact emails
--------------

TAMURA, Kent

unread,
Jun 27, 2017, 2:54:25 AM6/27/17
to Raphael Kubo Da Costa, blink-dev
LGTM3.


On Tue, Jun 27, 2017 at 3:48 PM, Raphael Kubo Da Costa <raphael.ku...@intel.com> wrote:
Anyone willing to lgtm3 this intent?


On Friday, 23 June 2017 17:21:34 UTC+2, Raphael Kubo Da Costa wrote:
Intent to Implement/Ship
------------------------
Change URLSearchparams' constructor from
    Constructor(optional (sequence<sequence<USVString>> or USVString or URLSearchParams) init = "")
to
    Constructor(optional (sequence<sequence<USVString>> or record<USVString, USVString> or USVString) init = ""),

Contact emails
--------------

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.



--
TAMURA Kent
Software Engineer, Google


Reply all
Reply to author
Forward
0 new messages