URLPattern: Set unmatched optional groups to undefined instead of ''.

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. Bug: 1292699 Change-Id: I27a0be2567bb9ce4592ca15a5216fd5a58bdbf17 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3428631 Reviewed-by: Jeremy Roman <[email protected]> Commit-Queue: Ben Kelly <[email protected]> Cr-Commit-Position: refs/heads/main@{#968611}
DanielRyanSmith · Feb 28, 2022 · 8b06d4f · 8b06d4f
1 parent 9c46444
commit 8b06d4f
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 10 deletions.
diff --git 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
@@ -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}`);
       }