24367 2014-01-22 20:57:55 +0000 Behaviour of URLUtils.searchParams setter 2014-03-19 13:55:35 +0000 1 1 1 Unclassified WHATWG URL unspecified PC Windows NT RESOLVED FIXED P2 normal Unsorted 1 sof annevk d erik.arvidsson mike sideshowbarker+urlspec oldest_to_newest 98903 0 sof 2014-01-22 20:57:55 +0000 The spec currently requires that upon setting URLUtils.searchParams with a URLSearchParams object with an existing "url object" association, it should create a copy of that object, http://url.spec.whatwg.org/#dom-url-searchparams Hence, var url = new URL("..."); var us = new URLSearchParams("a=b"); // url object is not set for "us", assign it as "url"'s query object. url.searchParams = us; us.set("a", "c"); // us.set() observable. assert_equals(url.search, "?a=c"); // url object is set for "us", copy it (and clear "us"' url object connection.) url.searchParams = us; us.set("a", "d"); // us.set() not observable. assert_equals(url.search, "?a=c"); // url object not set for "us", assign it as "url"'s query object. url.searchParams = us; us.set("a", "e"); // us.set() observable (again.) assert_equals(url.search, "?a=e"); Should the semantics of re-setting the query object via URLUtils.searchParams be different, or is this intended? 99011 1 erik.arvidsson 2014-01-24 16:39:20 +0000 Sigbjørn, I find the current behavior a bit confusing. I think we should just create a copy on assign. It just makes the semantics clear. And I believe that is what the spec says? This is the behavior I want. var url = new URL("http://example.com/?a=b"); var us = url.searchParams; us.set("a", "c"); assert_equals(url.search, "?a=c"); url.searchParams = us; // us is now disconnected from url us.set("a", "d"); assert_equals(url.search, "?a=c"); assert_equals(us.toString(), "a=d"); If we do not disconnect it we get into strange cases when it is assigned to multiple urls. (Firefox seems to ignore the set if the searchParams is set to the existing searchParams.) 99013 2 sof 2014-01-24 16:50:08 +0000 The spec does not allow copying if the "url object" of the URLSearchParams isn't set (step 3 and 4), http://url.spec.whatwg.org/#dom-url-searchparams It could either always copy, or only copy in step 3 if the assigned object differs from the current one. 99015 3 sof 2014-01-24 17:05:32 +0000 FWIW, I'm fine with either :) (including not having URLSearchParams as something separately constructable.) 99017 4 annevk 2014-01-24 18:00:09 +0000 Mostly copying makes sense to me. Should we special case when you assign the same object? I.e. when you do url.searchParams = url.searchParams? Not copying in that case would make sense to me. 99019 5 erik.arvidsson 2014-01-24 18:06:08 +0000 I prefer not special casing. It leads to more things to remember and you cannot look at the code to see what is going on. function f(url, params) { url.searchParams = params; params.set('a', 'b'); // mutates url or not? } 99020 6 annevk 2014-01-24 18:17:29 +0000 Fair. baku suggested on IRC to let a URLSearchParams be associated with many URL objects. As that seemed closer to JavaScript. The problem with that is that then you need to update many URL objects if your URLSearchParams changes (and maybe they need to do that in order if it becomes observable). 99021 7 sof 2014-01-24 18:26:45 +0000 arv's example convinced me; simple & functional. 99023 8 annevk 2014-01-24 19:19:23 +0000 Domenic also argued for what baku suggested. The reason is that then you preserve x.y = z x.y === z That would mean that whenever we assign a URLSearchParams to a URL, we'd do something like this: 1. If URL already has an associated URLSearchParams, remove URL from that URLSearchParams. 2. Add URL to the new URLSearchParams And whenever a URLSearchParams is updated we iterate through its URLs and update each in turn. 99024 9 annevk 2014-01-24 19:19:58 +0000 The example in comment 5 would then always result in mutating. 99027 10 erik.arvidsson 2014-01-24 20:57:33 +0000 (In reply to Anne from comment #8) > And whenever a URLSearchParams is updated we iterate through its URLs and > update each in turn. That is how most JS implementations would work. I'm fine with that too even though it is a bit more complex to implement on the C++ side. 99028 11 annevk 2014-01-24 21:26:40 +0000 Ok. I will make it like that then. I might not get this fixed for a few weeks so please proceed assuming comment 8 is part of the standard. 99030 12 sof 2014-01-24 21:31:33 +0000 I'm struggling to see the need for such generality. One way to sidestep all of this is to make url.searchParams a readonly attribute, and remove all the URLParams constructors. You can then (only) use url.searchParams to manipulate the query portion of one URL. If you want to copy the query portion from one URLUtils-implementing object to another, use url.search. 99031 13 annevk 2014-01-24 21:40:11 +0000 If we offer url.searchParams.replaceWith(new URLSearchParams(...)) or some such that might be okay too. Hmm... 99032 14 erik.arvidsson 2014-01-24 21:43:14 +0000 (In reply to Anne from comment #13) > If we offer url.searchParams.replaceWith(new URLSearchParams(...)) or some > such that might be okay too. Hmm... Ugh. Here are the options 1. Copy on assign 2. Keep references to multiple owners 3. Read only (you can always use .search) in the order I prefer. 99036 15 d 2014-01-24 22:48:02 +0000 I think copy on assign is kind of wonky, as it doesn't match normal semantics of assigning to a property. (I.e., those you expect from a data property.) Yes, the web platform has (ab)used setters to make assignment mean some pretty different things in the past, but I personally would prefer to keep the semantics as much as possible. That's why "Keep references to multiple owners" seems best. After that I think non-writable searchParams + `url.searchParams.replaceWith(new URLSearchParams(...))` is pretty decent. But if Arv doesn't like it, then I think the liked-or-at-least-tolerated-by-everyone option is indeed "Keep references to multiple owners." 99499 16 annevk 2014-01-30 21:22:10 +0000 https://github.com/whatwg/url/commit/c9904d891ef9ce23f94206f94e8550f4a6f4b31f 99553 17 sof 2014-01-31 22:07:15 +0000 That was quick! One potential user pitfall here is aliasing vs copying, i.e., url2.searchParams = url1.searchParams; vs url2.searchParams = new URLSearchParams(url1.searchParams); I still don't understand the use for 1-N associations, but ok. 102321 18 sof 2014-03-13 21:54:47 +0000 According to the current spec text, the following holds: var usp = new URLSearchParams(); url.searchParams = usp; url.searchParams = null; usp.set("a", "b"); assert_true("?a=b", url.searchParams.search); i.e., "url.searchParams = null;" has no effect, unlike "url.search = '';" (or "url.searchParams = new URLSearchParams();") As intended, or should the setter steps for http://url.spec.whatwg.org/#dom-url-searchparams be re-ordered a bit for the null case? 102352 19 sof 2014-03-14 07:04:50 +0000 (In reply to sof from comment #18) > According to the current spec text, the following holds: > .. > assert_true("?a=b", url.searchParams.search); > Typo, should have been: assert_true("?a=b", url.search); 102548 20 annevk 2014-03-18 11:30:08 +0000 So what effect do you think it should have? If we had different signatures for set and get I would allow returning null but not setting to null. So maybe it should throw a TypeError instead? 102573 21 sof 2014-03-18 16:05:50 +0000 I guess I need to understand why the attribute is nullable to start with to answer that. Could you explain, please? :) 102602 22 annevk 2014-03-18 19:19:50 +0000 I think it was nullable because at one point not all URLs had a query. That changed, so we should change the API too (to not be nullable). 102630 23 annevk 2014-03-19 13:48:33 +0000 https://github.com/whatwg/url/commit/3069fa45acb8134cc0c722ea548992e844b2d240 102632 24 sof 2014-03-19 13:55:35 +0000 That takes care of it from here; thank you.