Closed Bug 1074886 Opened 5 years ago Closed 5 years ago
Params .get() should return null (not empty string) when not find pair
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:32.0) Gecko/20100101 Firefox/32.0 Build ID: 20140923175406 Steps to reproduce: Pers URL spec for get() method: https://url.spec.whatwg.org/#dom-urlsearchparams-get "The get(name) method must return the value of the first name-value pair whose name is name, and null if there is no such pair." but now Firefox return empty string. Small test: <script> var USP = new URLSearchParams(); document.write(typeof USP.get("")+ "<br>"); // string document.write(USP.get("") + "<br>"); // "" document.write(USP.get("").length + "<br>"); 0 </script>
Summary: URLSearchParams.get() should return null (not empty string) → URLSearchParams.get() should return null (not empty string) when not find pair
Fix is simple - instead of truncating the string in URLSearchParams::Get(), call SetIsVoid(true) on it.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [good first bug][lang=C++]
No, call SetDOMStringToNull. SetIsVoid() is an implementation detail. Are there web-platform-tests for this spec? If not, we should probably add some...
Attachment #8497666 - Flags: review?(bzbarsky)
Is there a reason why this bug is platform specific?
Because bugs pick up the platform of the user that filed them rather than "All" by default.
OS: Windows 7 → All
Hardware: x86_64 → All
Comment on attachment 8497666 [details] [diff] [review] bug1074886.patch r=me, but please do add a test. In our tests for now, a followup for adding to w-p-t is good.
Attachment #8497666 - Flags: review?(bzbarsky) → review+
Hi Donato, I just landed a patch for bug 1074963. You have to rebase your patch on top of mine. Tell me if you have any problem. For the test, probably you want to change dom/base/test/test_urlSearchParams.html
The new patch has been rebased on top of Andreas's changes and adds some tests.
Comment on attachment 8500154 [details] [diff] [review] bug1074886.patch >+ is(u.get('foo'), null, 'URLSearchParams.get(foo)'); ise(), please, just to make sure JS is not messing with your equality tests. >+ is(u.get(''), null, "URL.searchParams.get('') should be null"); And here. >+ is(url.searchParams.get('b'), null, "URL.searchParams.get('b') should be null"); And here. r=me
Attachment #8500154 - Flags: review?(bzbarsky) → review+
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=24a0cb7de6d8 is the try run with that patch using is(). This should be ok, since "" == null is in fact false in JS, so is() and ise() are equivalent enough here.
The dom/workers/test/test_urlSearchParams.html test is failing with this patch. Presumably it needs to be adjusted accordingly?
This new patch fixes the failing test. A grep on the entire repo did not point out other modifications.
Comment on attachment 8501189 [details] [diff] [review] bug1074886.patch r=me
Attachment #8501189 - Flags: review?(bzbarsky) → review+
Assignee: nobody → sciarp
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.