URLPattern: Set unmatched optional groups to undefined instead of ''.
This addresses the issues raised in:
https://github.com/WICG/urlpattern/issues/162
The main changes in this CL are:
1. The webidl is modified to allow passed back undefined. I'm told
real webidl should support `(USVString or undefined)` here, but our
webidl compiler does not support that. So we use `any` instead.
2. We must examine if the WTF::String is null or not before populating
the returned array. If its null, then we convert to undefined
instead.
Bug: 1292699
Change-Id: I27a0be2567bb9ce4592ca15a5216fd5a58bdbf17
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3428631
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/main@{#968611}
diff --git a/urlpattern/resources/urlpatterntestdata.json b/urlpattern/resources/urlpatterntestdata.json
index 8992c95..6773648 100644
--- a/urlpattern/resources/urlpatterntestdata.json
+++ b/urlpattern/resources/urlpatterntestdata.json
@@ -353,8 +353,9 @@
{
"pattern": [{ "pathname": "/foo/:bar?" }],
"inputs": [{ "pathname": "/foo" }],
+ "//": "The `null` below is translated to undefined in the test harness.",
"expected_match": {
- "pathname": { "input": "/foo", "groups": { "bar": "" } }
+ "pathname": { "input": "/foo", "groups": { "bar": null } }
}
},
{
@@ -418,8 +419,9 @@
{
"pattern": [{ "pathname": "/foo/:bar*" }],
"inputs": [{ "pathname": "/foo" }],
+ "//": "The `null` below is translated to undefined in the test harness.",
"expected_match": {
- "pathname": { "input": "/foo", "groups": { "bar": "" } }
+ "pathname": { "input": "/foo", "groups": { "bar": null } }
}
},
{
@@ -472,15 +474,17 @@
"expected_obj": {
"pathname": "/foo/*?"
},
+ "//": "The `null` below is translated to undefined in the test harness.",
"expected_match": {
- "pathname": { "input": "/foo", "groups": { "0": "" } }
+ "pathname": { "input": "/foo", "groups": { "0": null } }
}
},
{
"pattern": [{ "pathname": "/foo/*?" }],
"inputs": [{ "pathname": "/foo" }],
+ "//": "The `null` below is translated to undefined in the test harness.",
"expected_match": {
- "pathname": { "input": "/foo", "groups": { "0": "" } }
+ "pathname": { "input": "/foo", "groups": { "0": null } }
}
},
{
@@ -656,15 +660,17 @@
"expected_obj": {
"pathname": "/foo/**"
},
+ "//": "The `null` below is translated to undefined in the test harness.",
"expected_match": {
- "pathname": { "input": "/foo", "groups": { "0": "" } }
+ "pathname": { "input": "/foo", "groups": { "0": null } }
}
},
{
"pattern": [{ "pathname": "/foo/**" }],
"inputs": [{ "pathname": "/foo" }],
+ "//": "The `null` below is translated to undefined in the test harness.",
"expected_match": {
- "pathname": { "input": "/foo", "groups": { "0": "" } }
+ "pathname": { "input": "/foo", "groups": { "0": null } }
}
},
{
@@ -1810,9 +1816,10 @@
"hostname": "(sub.)?example.com",
"pathname": "/foo"
},
+ "//": "The `null` below is translated to undefined in the test harness.",
"expected_match": {
"protocol": { "input": "https", "groups": {} },
- "hostname": { "input": "example.com", "groups": { "0": "" } },
+ "hostname": { "input": "example.com", "groups": { "0": null } },
"pathname": { "input": "/foo", "groups": {} }
}
},
@@ -1848,9 +1855,10 @@
"hostname": "(sub(?:.))?example.com",
"pathname": "/foo"
},
+ "//": "The `null` below is translated to undefined in the test harness.",
"expected_match": {
"protocol": { "input": "https", "groups": {} },
- "hostname": { "input": "example.com", "groups": { "0": "" } },
+ "hostname": { "input": "example.com", "groups": { "0": null } },
"pathname": { "input": "/foo", "groups": {} }
}
},
@@ -2297,9 +2305,10 @@
"protocol": "data",
"pathname": "text/javascript,let x = 100/:tens?5;"
},
+ "//": "The `null` below is translated to undefined in the test harness.",
"expected_match": {
"protocol": { "input": "data", "groups": {} },
- "pathname": { "input": "text/javascript,let x = 100/5;", "groups": { "tens": "" } }
+ "pathname": { "input": "text/javascript,let x = 100/5;", "groups": { "tens": null } }
}
},
{
@@ -2619,8 +2628,9 @@
"expected_obj": {
"pathname": "*(.*)?"
},
+ "//": "The `null` below is translated to undefined in the test harness.",
"expected_match": {
- "pathname": { "input": "foobar", "groups": { "0": "foobar", "1": "" }}
+ "pathname": { "input": "foobar", "groups": { "0": "foobar", "1": null }}
}
},
{
diff --git a/urlpattern/resources/urlpatterntests.js b/urlpattern/resources/urlpatterntests.js
index 40b3edc..f774699 100644
--- a/urlpattern/resources/urlpatterntests.js
+++ b/urlpattern/resources/urlpatterntests.js
@@ -146,6 +146,14 @@
expected_obj.groups['0'] = '';
}
}
+ // JSON does not allow us to use undefined directly, so the data file
+ // contains null instead. Translate to the expected undefined value
+ // here.
+ for (const key in expected_obj.groups) {
+ if (expected_obj.groups[key] === null) {
+ expected_obj.groups[key] = undefined;
+ }
+ }
assert_object_equals(exec_result[component], expected_obj,
`exec() result for ${component}`);
}
