Skip to content

Commit

Permalink
URLPattern: Set unmatched optional groups to undefined instead of ''.
Browse files Browse the repository at this point in the history
This addresses the issues raised in:

whatwg/urlpattern#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.

It turned out, however, that StringUTF8Adaptor was also converting
empty strings to null strings.  It was necessary to fix this in order
to return undefined and empty string in the correct places in the
returned values.

Bug: 1292699
Change-Id: I27a0be2567bb9ce4592ca15a5216fd5a58bdbf17
  • Loading branch information
wanderview authored and chromium-wpt-export-bot committed Feb 1, 2022
1 parent 34b9a71 commit 05be0c3
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 10 deletions.
30 changes: 20 additions & 10 deletions urlpattern/resources/urlpatterntestdata.json
Original file line number Diff line number Diff line change
Expand Up @@ -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 } }
}
},
{
Expand Down Expand Up @@ -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 } }
}
},
{
Expand Down Expand Up @@ -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 } }
}
},
{
Expand Down Expand Up @@ -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 } }
}
},
{
Expand Down Expand Up @@ -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": {} }
}
},
Expand Down Expand Up @@ -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": {} }
}
},
Expand Down Expand Up @@ -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 } }
}
},
{
Expand Down Expand Up @@ -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 }}
}
},
{
Expand Down
8 changes: 8 additions & 0 deletions urlpattern/resources/urlpatterntests.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,14 @@ function runTests(data) {
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}`);
}
Expand Down

0 comments on commit 05be0c3

Please sign in to comment.