url: fix handling of ? in URLSearchParams creation

TimothyGu · Italo A. Casas · commit 5f10827248d4 · 2017-02-25T10:56:39.000-05:00
PR-URL: #11372 Fixes: #11093 Ref: whatwg/url#248 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
diff --git a/lib/internal/url.js b/lib/internal/url.js
@@ -95,12 +95,11 @@ function onParseComplete(flags, protocol, username, password,
   ctx.query = query;
   ctx.fragment = fragment;
   ctx.host = host;
-  if (this[searchParams]) {  // invoked from href setter
-    initSearchParams(this[searchParams], query);
-  } else {
-    this[searchParams] = new URLSearchParams(query);
+  if (!this[searchParams]) { // invoked from URL constructor
+    this[searchParams] = new URLSearchParams();
+    this[searchParams][context] = this;
   }
-  this[searchParams][context] = this;
+  initSearchParams(this[searchParams], query);
 }
 
 // Reused by URL constructor and URL#href setter.
diff --git a/test/parallel/test-whatwg-url-constructor.js b/test/parallel/test-whatwg-url-constructor.js
@@ -119,22 +119,22 @@ function runURLSearchParamTests() {
 
     // And in the other direction, altering searchParams propagates
     // back to 'search'.
-    // searchParams.append('i', ' j ')
+    searchParams.append('i', ' j ')
     // assert_equals(url.search, '?e=f&g=h&i=+j+')
     // assert_equals(url.searchParams.toString(), 'e=f&g=h&i=+j+')
-    // assert_equals(searchParams.get('i'), ' j ')
+    assert_equals(searchParams.get('i'), ' j ')
 
-    // searchParams.set('e', 'updated')
+    searchParams.set('e', 'updated')
     // assert_equals(url.search, '?e=updated&g=h&i=+j+')
-    // assert_equals(searchParams.get('e'), 'updated')
+    assert_equals(searchParams.get('e'), 'updated')
 
-    // var url2 = bURL('http://example.org/file??a=b&c=d')
-    // assert_equals(url2.search, '??a=b&c=d')
-    // assert_equals(url2.searchParams.toString(), '%3Fa=b&c=d')
+    var url2 = bURL('http://example.org/file??a=b&c=d')
+    assert_equals(url2.search, '??a=b&c=d')
+    assert_equals(url2.searchParams.toString(), '%3Fa=b&c=d')
 
-    // url2.href = 'http://example.org/file??a=b'
-    // assert_equals(url2.search, '??a=b')
-    // assert_equals(url2.searchParams.toString(), '%3Fa=b')
+    url2.href = 'http://example.org/file??a=b'
+    assert_equals(url2.search, '??a=b')
+    assert_equals(url2.searchParams.toString(), '%3Fa=b')
   }, 'URL.searchParams and URL.search setters, update propagation')
 }
 runURLSearchParamTests()
