Use the URLPattern specification instead of custom URL pattern parsing

[whimboo]

Fixes #919.

This change aligns our implementation for parsing and matching URL patterns with the URLPattern specification, ensuring consistent and spec-compliant behavior across matching operations. It reduces maintenance overhead by removing custom parsing code from the WebDriver BiDi specification, and leverages the native URLPattern implementation for improved correctness and clarity.

Existing functionality is preserved, but edge cases may now follow the semantics defined by the specification.

Follow-up work will extend support for additional URLPatternInit fields that we do not support yet, and as well for handling the baseURL, and parse options like ignoreCase.

---

Preview | Diff

[whimboo]

@juliandescottes could you maybe do the initial review?

[juliandescottes]

Looks like a great simplification. Overall I think this is fine, but we need to handle the error case.

[whimboo]

@OrKoN could you please review the changes as well? Thanks!

[whimboo]

Follow-up work will extend support for additional URLPatternInit fields that we do not support yet, and as well for handling the baseURL, and parse options like ignoreCase. This change is intended as a strict 1:1 replacement of the existing functionality without introducing new behavior.

I filed #1109 for this follow-up work.

[OrKoN]

@whimboo do you already have an implementation? I wonder what the effect on existing WPT tests will be

[whimboo]

@whimboo do you already have an implementation? I wonder what the effect on existing WPT tests will be

@OrKoN, I haven't had when you were asking. But I spend some time today in updating our interception related code to check what will change. In general I noticed that several values are handled more permissively compared to our current implementation. The main differences are:

• Wildcards (*) now work consistently across all fields and act as the default when a field is not specified.
• When using a pattern init object, fields like search treat most special characters as literals (e.g. search: "search#", which gets encoded (e.g. search%23). This can lead to mismatches with URLs such as http://example.com/?search#, since URL parsing interprets # as a delimiter. The ? character will still cause a parser error.
• An empty pathname currently matches all schemes in our implementation, but with URLPattern, this is no longer the case. For special schemes, / must be used explicitly to achieve the same behavior.
• Invalid cases like https://example.com:port/ as a string pattern will now work because example.com:port is taken as value for the hostname only.

All the relevant test changes you can find in: https://bugzilla.mozilla.org/attachment.cgi?id=9568221

As given by Julian we expected breaking changes and from our side it looks ok. What do you think?

[OrKoN]

I added a basic implementation here GoogleChromeLabs/chromium-bidi#4138 and there quite a few WPT tests that are failing, I wonder if the list of failed tests matches what you have?

[whimboo]

I added a basic implementation here GoogleChromeLabs/chromium-bidi#4138 and there quite a few WPT tests that are failing, I wonder if the list of failed tests matches what you have?

There is actually only a single test which seems to perma fail and some others that fail intermittently. Here a job which shows 3 failing tests:

https://github.com/GoogleChromeLabs/chromium-bidi/actions/runs/24882696358/job/72854858369?pr=4138

FAILED tests/network/test_add_intercept.py::test_add_intercept_type_string_one_valid_and_one_invalid - AssertionError: Regex pattern did not match.
 Regex: '{\'error\': \'invalid argument\', \'message\': "Failed to construct \'URL\': Invalid URL \'foo\'"}'
 Input: '{\'error\': \'invalid argument\', \'message\': "Invalid URL pattern: Failed to construct \'URLPattern\': Relative constructor string \'foo\' must have a base URL passed as the second argument."}'
FAILED tests/network/test_add_intercept.py::test_add_intercept_type_pattern_protocol_empty_invalid - Failed: DID NOT RAISE <class 'Exception'>
FAILED tests/network/test_add_intercept.py::test_add_intercept_type_pattern_hostname_invalid[abc:com] - Failed: DID NOT RAISE <class 'Exception'>
= 3 failed, 136 passed, 1 skipped, 1 xfailed, 1 warning, 12 rerun in 85.89s (0:01:25) =

It looks like that I did not update this particular test file as part of my patch on https://phabricator.services.mozilla.com/D293101. I should take a look. Thanks for pointing out.

[whimboo]

FAILED tests/network/test_add_intercept.py::test_add_intercept_type_string_one_valid_and_one_invalid - AssertionError: Regex pattern did not match.
 Regex: '{\'error\': \'invalid argument\', \'message\': "Failed to construct \'URL\': Invalid URL \'foo\'"}'
 Input: '{\'error\': \'invalid argument\', \'message\': "Invalid URL pattern: Failed to construct \'URLPattern\': Relative constructor string \'foo\' must have a base URL passed as the second argument."}'
FAILED tests/network/test_add_intercept.py::test_add_intercept_type_pattern_protocol_empty_invalid - Failed: DID NOT RAISE <class 'Exception'>
FAILED tests/network/test_add_intercept.py::test_add_intercept_type_pattern_hostname_invalid[abc:com] - Failed: DID NOT RAISE <class 'Exception'>
= 3 failed, 136 passed, 1 skipped, 1 xfailed, 1 warning, 12 rerun in 85.89s (0:01:25) =

@OrKoN these failing tests are not part of the public wdspec tests.

But nevertheless here a quick investigation:

• test_add_intercept_type_string_one_valid_and_one_invalid fails because its a relative URL and the Base URL is missing. Note that we have a follow-up issue for that as filed as Add support for additional fields (baseURL) and parse options (ignoreCase) for the URLPattern usage #1109.
• test_add_intercept_type_pattern_protocol_empty_invalid fails because it assumes that URLPattern should cause an error if "" for the protocol is specified. Note that new URLPattern({protocol: "", hostname: "www.example.com", port: "80" }) doesn't raise in Chrome as well.
• test_add_intercept_type_pattern_hostname_invalid[abc:com] unexpectedly passes for you when it should fail. For us it fails correctly so I assume there is a bug in your implementation? I'm going to add this case to my patch which updates the tests.

With all that I assume it should not be blocked to get the changes landed?

[whimboo]

@OrKoN over on https://phabricator.services.mozilla.com/D293101 I added a couple more tests to cover the additional features that we now allow to use by clients which URLPattern offers for us. It should apply to the upstream wpt repo (when you ignore the prefix in the path) so that you can run these tests with your implementation as well.

Please let me know how it looks like. Thanks!

[whimboo]

One thing to keep in mind, and likely the most visible change for users, is that characters with special meaning in URLPattern syntax (+, (, ), {, }, :) must be escaped with \ when used literally in string patterns. This will most commonly affect + characters in query strings, such as those resulting from URL-encoded spaces.

[whimboo]

I had to push some smaller updates based on our internal review and that I saw that no note was added describing the new behavior + issue for the follow-up work.

@juliandescottes please review again. Thanks

[whimboo]

@jgraham can you please review as well? Thanks

[jgraham]

Although this is "cheap" in that we're just passing in extra options, I'm not quite convinced that they all make sense for WebDriver use cases.

[jgraham]

Technically you can't really do this; you're trying to create a JS object outside a realm. The URLPattern specification accepts an infra Map object as URLPatternInit in cases where it's not running in a JS realm. I think you want to use the entry point at https://urlpattern.spec.whatwg.org/#build-a-url-pattern-from-an-infra-value.