Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a section on other specs integrating with URLPattern #199

Merged
merged 11 commits into from
Nov 30, 2023

Conversation

jeremyroman
Copy link
Collaborator

@jeremyroman jeremyroman commented Nov 21, 2023

A section which covers how other specifications should use URLPattern and how developer-facing APIs should work is added, along with helpful algorithms.

One of these is whether a pattern has regexp groups (which may require an ECMAScript regexp engine); a corresponding WebIDL attribute is added to expose this property to authors as well.

(See WHATWG Working Mode: Changes for more details.)


Preview | Diff

A section which covers how other specifications should use URLPattern
and how developer-facing APIs should work is added, along with helpful
algorithms.

One of these is whether a pattern has regexp groups (which may require
an ECMAScript regexp engine); a corresponding WebIDL attribute is added
to expose this property to authors as well.
@jeremyroman
Copy link
Collaborator Author

jeremyroman commented Nov 21, 2023

This relates to #182 and #191.

spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated
1. Otherwise, if |rawPattern| is a [=map=], then:
1. Let |init| be «[ "`baseURL`" → |serializedBaseURL| ]», representing a dictionary of type {{URLPatternInit}}.
1. [=map/For each=] |key| → |value| of |rawPattern|:
1. If |key| is not the [=identifier=] of a [=dictionary member=] of {{URLPatternInit}} or one of its [=inherited dictionaries=], |value| is not a [=string=], or the member's type is not declared to be {{USVString}}, then return null.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not 100% clear to me that every spec wants the kind of restrictive error handling we've used here. But I feel like we discussed this in the past when writing the speculation rules spec. I'm OK carrying it over for now, until someone raises an issue.

spec.bs Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Show resolved Hide resolved
spec.bs Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated
1. Return a new [=component=] whose [=component/pattern string=] is |pattern string|, [=component/regular expression=] is |regular expression|, and [=component/group name list=] is |name list|.
1. Let |has regexp groups| be false.
1. [=list/For each=] |part| of |part list|:
1. If |part|'s [=part/type=] is "<a for=token/type>`regexp`</a>", then set |has regexp groups| to true.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<a for=token/type>`regexp`</a> : part/type is correct instead of token/type?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're correct; done.

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found a few more nits, but it's looking quite nice to me.

It might be worth adding (either here or in a followup PR) a section on using URL patterns in HTTP fields, since that's another use case that's coming up in practice. Initial thoughts:

  • Encourage just using strings for now, and not necessarily accepting dictionaries
  • The algorithm can be a wrapper around "build a URLPattern from an Infra value" that is only ever called with a string
  • Give them some help on figuring out exactly which base URL to use. Maybe consider both the request and response header cases?
  • Maybe omit the realm argument and just use "an implementation-defined realm", because it's quite silly to make HTTP specs deal with realms? We can still have an XXX box, but I'd hate to have the realm as part of the public interface for this particular call site. (Related discussion regarding "implementation-defined realm": "Parse JSON to Infra" algorithms shouldn't require a current JS realm infra#625)

spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
@jeremyroman
Copy link
Collaborator Author

Found a few more nits, but it's looking quite nice to me.

It might be worth adding (either here or in a followup PR) a section on using URL patterns in HTTP fields, since that's another use case that's coming up in practice. Initial thoughts:

  • Encourage just using strings for now, and not necessarily accepting dictionaries
  • The algorithm can be a wrapper around "build a URLPattern from an Infra value" that is only ever called with a string
  • Give them some help on figuring out exactly which base URL to use. Maybe consider both the request and response header cases?
  • Maybe omit the realm argument and just use "an implementation-defined realm", because it's quite silly to make HTTP specs deal with realms? We can still have an XXX box, but I'd hate to have the realm as part of the public interface for this particular call site. (Related discussion regarding "implementation-defined realm": "Parse JSON to Infra" algorithms shouldn't require a current JS realm infra#625)

Yeah, I thought about including HTTP fields in this PR, but wasn't sure if we'd hammered out enough of the details yet.

At surface I would have thought the constructor string syntax would have been the most natural fit, but from what I understand of WICG/compression-dictionary-transport#52 they're currently looking at separate match-path and match-search (or match-query) params (contrary to your proposal here), and it's not clear to me if their reasons for doing so generalize. Like, here, it's somewhat pointless to match on things like protocol and port, but that's not necessarily the case for most hypothetical future header fields.

Besides that one I don't have a clear enough concept of the likely uses to be confident writing guidance, and would probably rather they file an issue or otherwise bring it up to us for now.

@domenic
Copy link
Member

domenic commented Nov 22, 2023

At surface I would have thought the constructor string syntax would have been the most natural fit, but from what I understand of WICG/compression-dictionary-transport#52 they're currently looking at separate match-path and match-search (or match-query) params (contrary to your proposal here), and it's not clear to me if their reasons for doing so generalize. Like, here, it's somewhat pointless to match on things like protocol and port, but that's not necessarily the case for most hypothetical future header fields.

Hmm, we should discuss with them what's going on. This seems contrary to the design of URL patterns...

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. It'd be great to get @yoshisatoyanagisawa's review as well, as someone who would be working on the service worker static routing spec, which follows these guidelines.

spec.bs Outdated Show resolved Hide resolved
@jeremyroman
Copy link
Collaborator Author

While I'm here, want to note -- the ignoreCase option has the potential to be an obstacle to the cases which can't handle regexp groups, but I think it's actually fine because I think the match algorithm ends up making everything ASCII. Otherwise, the fact that vi regexps do Unicode case folding (so k could match not only uppercase K but also kelvin symbol K , for instance) would be a potential issue for implementations that implemented matching without a regexp engine.

Copy link
Collaborator

@sisidovski sisidovski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I think the new sections explain well. SW static routing API would be able to follow this change.

@jeremyroman
Copy link
Collaborator Author

jeremyroman commented Nov 24, 2023

For multi-implementer interest, @annevk is WebKit cool with this? You seemed supportive of at least #191 (which is the real behavior change here right now, since nothing depends on the #182 aspects as yet).

To promote consistency on the web platform, other documents integrating with this specification should adhere to the following guidelines, unless there is good reason to diverge.

1. **Accept shorthands**. Most author patterns will be simple and straightforward. Accordingly, APIs should accept shorthands for those common cases and avoid the need for authors to take additional steps to transform these into complete {{URLPattern}} objects.
1. **Respect the base URL**. Just as URLs are generally parsed relative to a base URL for their environment (most commonly, a [=document base URL=]), URL patterns should respect this as well. The {{URLPattern}} constructor itself is an exception because it directly exposes the concept itself, similar to how the <a interface spec=URL>URL</a> constructor does not respect the base URL even though the rest of the platform does.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"most commonly" might mean that each specification can choose the base URL to something most relevant to it, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Below it suggests the environment settings object's API base URL, which is the document base URL most of the time (in a document), but in a worker it's the global scope's base URL. In an HTTP header it would probably be the request or response URL. Most of the time there's a fairly strong precedent about what URLs are resolved relative to already, and specs should probably follow that unless they have a compelling reason to deviate.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok. thanks for the clarification.

spec.bs Outdated
To accomplish this, specifications should accept {{URLPatternCompatible}} as an argument to an [=operation=] or [=dictionary member=], and process it using the following algorithm, using the appropriate [=environment settings object=]'s [=environment settings object/API base URL=] or equivalent.

<div algorithm>
To <dfn for=URLPattern>build a {{URLPattern}} from a WebIDL value</dfn> {{URLPatternCompatible}} |input| given [=/URL=] |baseURL| and [=ECMAScript/realm=]</a> |realm|, perform the following steps:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit but where is a counterpart of this ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what you're asking; can you elaborate?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I should have updated this when I noticed the comment can only be given per line X(

In this line, there is </a>. However, I could not find opening of this </a>.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was an mistakenly left behind when rewriting back and forth between Bikeshed shorthand and <a> syntax. Thanks for catching; removed.

<p class="XXX">Ideally we wouldn't need a realm here. If we extricate the URL pattern concept from the {{URLPattern}} interface, we won't anymore.</p>
</div>

This allows authors to concisely specify most patterns, and use the <a constructor for="URLPattern">constructor</a> to access uncommon options if necessary. The implicit use of the base URL is similar to, and consistent with, <cite>HTML</cite>'s [=parse a URL=] algorithm. [[HTML]]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit but this phrase reminds me #200. It seems that the URLPattern initialization for the short-hand is slightly different from the URL algorithm.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is indeed slightly different, yeah. Still, I think it's good to keep it broadly similar where we can.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. I hope we can discuss about the topic in #200.

@yoshisatoyanagisawa
Copy link
Contributor

Thanks for looping me in, @domenic.
LGTM. I left some minor comments.

@domenic
Copy link
Member

domenic commented Nov 27, 2023

Given the comments like

Thanks. I'll have to wait for the PR to get resolved before it is something I could reference but as it looks right now, URLPatternCompatible is specific to the Javascript API.

over in WICG/compression-dictionary-transport#52 it seems like it'd be good to add a section on HTTP header usage, either in this PR or as a near-future followup.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Nov 27, 2023
This behavior change was originally started in
whatwg/urlpattern#182, and follows the spec
change in whatwg/urlpattern#199.

This CL changes the behavior of the router rule registration in the
ServiceWorker Static Routing API, especially when the |urlPattern|
condition receives URLPatternInit or USVString.

Before this CL, the URLPatternInit input was accepted as it is, that
means any unspecified fields are resulted in the wildcards (*). This
behavior is inconsistent with the case when |urlPattern| accepts a
string. When a string is passed, missing fields are complemented by
baseURL, the SW script URL is internally treated as baseURL.

After this CL, the URLPatternInit input also internally uses the SW
script URL as a baseURL if it's not explicitly provided. This is
achieved by the helper method `URLPattern::From()`, which was added in
[1].

This change doesn't affect the case when the input is URLPattern, which
means the input is the object constructed via `new URLPattern()`.

[1] crrev.com/c/5053645

Bug: 1371756
Change-Id: I5cce80fde05cf18237c8b6412b00e017ff5aad5b
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Nov 27, 2023
This behavior change was originally started in
whatwg/urlpattern#182, and follows the spec
change in whatwg/urlpattern#199.

This CL changes the behavior of the router rule registration in the
ServiceWorker Static Routing API, especially when the |urlPattern|
condition receives URLPatternInit or USVString.

Before this CL, the URLPatternInit input was accepted as it is, that
means any unspecified fields are resulted in the wildcards (*). This
behavior is inconsistent with the case when |urlPattern| accepts a
string. When a string is passed, missing fields are complemented by
baseURL, the SW script URL is internally treated as baseURL.

After this CL, the URLPatternInit input also internally uses the SW
script URL as a baseURL if it's not explicitly provided. This is
achieved by the helper method `URLPattern::From()`, which was added in
[1].

This change doesn't affect the case when the input is URLPattern, which
means the input is the object constructed via `new URLPattern()`.

[1] crrev.com/c/5053645

Bug: 1371756
Change-Id: I5cce80fde05cf18237c8b6412b00e017ff5aad5b
@jeremyroman
Copy link
Collaborator Author

Given the comments like

Thanks. I'll have to wait for the PR to get resolved before it is something I could reference but as it looks right now, URLPatternCompatible is specific to the Javascript API.

over in WICG/compression-dictionary-transport#52 it seems like it'd be good to add a section on HTTP header usage, either in this PR or as a near-future followup.

I agree it would be good to cover once we're agreed on what to do and feel fairly confident it will generalize. I was hoping to avoid entangling these two, so doing HTTP header fields in a followup. It feels slightly silly to write our opinions here and then turn around and use ourselves as authority to back up our own position, but if that's helpful we can do that too.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Nov 28, 2023
This behavior change was originally started in
whatwg/urlpattern#182, and follows the spec
change in whatwg/urlpattern#199.

This CL changes the behavior of the router rule registration in the
ServiceWorker Static Routing API, especially when the |urlPattern|
condition receives URLPatternInit or USVString.

Before this CL, the URLPatternInit input was accepted as it is, that
means any unspecified fields are resulted in the wildcards (*). This
behavior is inconsistent with the case when |urlPattern| accepts a
string. When a string is passed, missing fields are complemented by
baseURL, the SW script URL is internally treated as baseURL.

After this CL, the URLPatternInit input also internally uses the SW
script URL as a baseURL if it's not explicitly provided. This is
achieved by the helper method `URLPattern::From()`, which was added in
[1].

This change doesn't affect the case when the input is URLPattern, which
means the input is the object constructed via `new URLPattern()`.

[1] crrev.com/c/5053645

Bug: 1371756
Change-Id: I5cce80fde05cf18237c8b6412b00e017ff5aad5b
aarongable pushed a commit to chromium/chromium that referenced this pull request Nov 28, 2023
This behavior change was originally started in
whatwg/urlpattern#182, and follows the spec
change in whatwg/urlpattern#199.

This CL changes the behavior of the router rule registration in the
ServiceWorker Static Routing API, especially when the |urlPattern|
condition receives URLPatternInit or USVString.

Before this CL, the URLPatternInit input was accepted as it is, that
means any unspecified fields are resulted in the wildcards (*). This
behavior is inconsistent with the case when |urlPattern| accepts a
string. When a string is passed, missing fields are complemented by
baseURL, the SW script URL is internally treated as baseURL.

After this CL, the URLPatternInit input also internally uses the SW
script URL as a baseURL if it's not explicitly provided. This is
achieved by the helper method `URLPattern::From()`, which was added in
[1].

This change doesn't affect the case when the input is URLPattern, which
means the input is the object constructed via `new URLPattern()`.

[1] crrev.com/c/5053645

Bug: 1371756
Change-Id: I5cce80fde05cf18237c8b6412b00e017ff5aad5b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5039680
Reviewed-by: Takashi Toyoshima <[email protected]>
Reviewed-by: Kouhei Ueno <[email protected]>
Commit-Queue: Shunya Shishido <[email protected]>
Reviewed-by: Yoshisato Yanagisawa <[email protected]>
Auto-Submit: Shunya Shishido <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1229724}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Nov 28, 2023
This behavior change was originally started in
whatwg/urlpattern#182, and follows the spec
change in whatwg/urlpattern#199.

This CL changes the behavior of the router rule registration in the
ServiceWorker Static Routing API, especially when the |urlPattern|
condition receives URLPatternInit or USVString.

Before this CL, the URLPatternInit input was accepted as it is, that
means any unspecified fields are resulted in the wildcards (*). This
behavior is inconsistent with the case when |urlPattern| accepts a
string. When a string is passed, missing fields are complemented by
baseURL, the SW script URL is internally treated as baseURL.

After this CL, the URLPatternInit input also internally uses the SW
script URL as a baseURL if it's not explicitly provided. This is
achieved by the helper method `URLPattern::From()`, which was added in
[1].

This change doesn't affect the case when the input is URLPattern, which
means the input is the object constructed via `new URLPattern()`.

[1] crrev.com/c/5053645

Bug: 1371756
Change-Id: I5cce80fde05cf18237c8b6412b00e017ff5aad5b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5039680
Reviewed-by: Takashi Toyoshima <[email protected]>
Reviewed-by: Kouhei Ueno <[email protected]>
Commit-Queue: Shunya Shishido <[email protected]>
Reviewed-by: Yoshisato Yanagisawa <[email protected]>
Auto-Submit: Shunya Shishido <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1229724}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Nov 28, 2023
This behavior change was originally started in
whatwg/urlpattern#182, and follows the spec
change in whatwg/urlpattern#199.

This CL changes the behavior of the router rule registration in the
ServiceWorker Static Routing API, especially when the |urlPattern|
condition receives URLPatternInit or USVString.

Before this CL, the URLPatternInit input was accepted as it is, that
means any unspecified fields are resulted in the wildcards (*). This
behavior is inconsistent with the case when |urlPattern| accepts a
string. When a string is passed, missing fields are complemented by
baseURL, the SW script URL is internally treated as baseURL.

After this CL, the URLPatternInit input also internally uses the SW
script URL as a baseURL if it's not explicitly provided. This is
achieved by the helper method `URLPattern::From()`, which was added in
[1].

This change doesn't affect the case when the input is URLPattern, which
means the input is the object constructed via `new URLPattern()`.

[1] crrev.com/c/5053645

Bug: 1371756
Change-Id: I5cce80fde05cf18237c8b6412b00e017ff5aad5b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5039680
Reviewed-by: Takashi Toyoshima <[email protected]>
Reviewed-by: Kouhei Ueno <[email protected]>
Commit-Queue: Shunya Shishido <[email protected]>
Reviewed-by: Yoshisato Yanagisawa <[email protected]>
Auto-Submit: Shunya Shishido <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1229724}
@annevk
Copy link
Member

annevk commented Nov 28, 2023

@jeremyroman I don't understand the references to Deno in #199 (comment).

WebKit is supportive of figuring out an interface to URLPattern so it can be reused in environments where regular expression engines are a no-no.

I'm not entirely sure that the JavaScript API needs to make the difference observable (as that won't be accessible in those environments), but I guess it's reasonable.

@jeremyroman
Copy link
Collaborator Author

@jeremyroman I don't understand the references to Deno in #199 (comment).

Those were supposed to be links to issues in this repository; for some reason Github autolinked to the wrong repository. (???)

WebKit is supportive of figuring out an interface to URLPattern so it can be reused in environments where regular expression engines are a no-no.

I'm not entirely sure that the JavaScript API needs to make the difference observable (as that won't be accessible in those environments), but I guess it's reasonable.

One reason might be registering service worker static routes, where the pattern is registered from a JavaScript environment but evaluated in the fetch handling path.

@annevk
Copy link
Member

annevk commented Nov 28, 2023

Makes sense, yeah. I do think we need a string-only version (realm-free) of the part you're not explicitly seeking feedback on yet, but we can get there through iteration.

spec.bs Outdated Show resolved Hide resolved
Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should also export the "match" definition https://urlpattern.spec.whatwg.org/#match

spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
@jeremyroman
Copy link
Collaborator Author

@yoshisatoyanagisawa @domenic still lgty both? I'll merge if so, as I think we've met the process requirements.

@domenic
Copy link
Member

domenic commented Nov 30, 2023

Still LGTM except I don't think you saw this comment:

You should also export the "match" definition https://urlpattern.spec.whatwg.org/#match

@yoshisatoyanagisawa
Copy link
Contributor

Thanks for heads up.
Still lgtm.

@jeremyroman
Copy link
Collaborator Author

Still LGTM except I don't think you saw this comment:

You should also export the "match" definition https://urlpattern.spec.whatwg.org/#match

Whoops, super easy to miss unfortunately because of how GitHub requires me to switch to Files to be able to start a review (and not send tons of individual emails), which hides comments not attached to lines. Will fix.

@jeremyroman jeremyroman merged commit 2c23b49 into whatwg:main Nov 30, 2023
2 checks passed
@jeremyroman jeremyroman deleted the integrating branch November 30, 2023 02:16
yoshisatoyanagisawa added a commit to yoshisatoyanagisawa/ServiceWorker that referenced this pull request Nov 30, 2023
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Dec 2, 2023
…while routing rule registration, a=testonly

Automatic update from web-platform-tests
Add baseURL to the URLPattern condition while routing rule registration

This behavior change was originally started in
whatwg/urlpattern#182, and follows the spec
change in whatwg/urlpattern#199.

This CL changes the behavior of the router rule registration in the
ServiceWorker Static Routing API, especially when the |urlPattern|
condition receives URLPatternInit or USVString.

Before this CL, the URLPatternInit input was accepted as it is, that
means any unspecified fields are resulted in the wildcards (*). This
behavior is inconsistent with the case when |urlPattern| accepts a
string. When a string is passed, missing fields are complemented by
baseURL, the SW script URL is internally treated as baseURL.

After this CL, the URLPatternInit input also internally uses the SW
script URL as a baseURL if it's not explicitly provided. This is
achieved by the helper method `URLPattern::From()`, which was added in
[1].

This change doesn't affect the case when the input is URLPattern, which
means the input is the object constructed via `new URLPattern()`.

[1] crrev.com/c/5053645

Bug: 1371756
Change-Id: I5cce80fde05cf18237c8b6412b00e017ff5aad5b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5039680
Reviewed-by: Takashi Toyoshima <[email protected]>
Reviewed-by: Kouhei Ueno <[email protected]>
Commit-Queue: Shunya Shishido <[email protected]>
Reviewed-by: Yoshisato Yanagisawa <[email protected]>
Auto-Submit: Shunya Shishido <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1229724}

--

wpt-commits: ffe06ef95d4d48be147e640fd9d1c489b810f929
wpt-pr: 43272
vinnydiehl pushed a commit to vinnydiehl/mozilla-unified that referenced this pull request Dec 3, 2023
…while routing rule registration, a=testonly

Automatic update from web-platform-tests
Add baseURL to the URLPattern condition while routing rule registration

This behavior change was originally started in
whatwg/urlpattern#182, and follows the spec
change in whatwg/urlpattern#199.

This CL changes the behavior of the router rule registration in the
ServiceWorker Static Routing API, especially when the |urlPattern|
condition receives URLPatternInit or USVString.

Before this CL, the URLPatternInit input was accepted as it is, that
means any unspecified fields are resulted in the wildcards (*). This
behavior is inconsistent with the case when |urlPattern| accepts a
string. When a string is passed, missing fields are complemented by
baseURL, the SW script URL is internally treated as baseURL.

After this CL, the URLPatternInit input also internally uses the SW
script URL as a baseURL if it's not explicitly provided. This is
achieved by the helper method `URLPattern::From()`, which was added in
[1].

This change doesn't affect the case when the input is URLPattern, which
means the input is the object constructed via `new URLPattern()`.

[1] crrev.com/c/5053645

Bug: 1371756
Change-Id: I5cce80fde05cf18237c8b6412b00e017ff5aad5b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5039680
Reviewed-by: Takashi Toyoshima <[email protected]>
Reviewed-by: Kouhei Ueno <[email protected]>
Commit-Queue: Shunya Shishido <[email protected]>
Reviewed-by: Yoshisato Yanagisawa <[email protected]>
Auto-Submit: Shunya Shishido <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1229724}

--

wpt-commits: ffe06ef95d4d48be147e640fd9d1c489b810f929
wpt-pr: 43272
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Dec 7, 2023
…while routing rule registration, a=testonly

Automatic update from web-platform-tests
Add baseURL to the URLPattern condition while routing rule registration

This behavior change was originally started in
whatwg/urlpattern#182, and follows the spec
change in whatwg/urlpattern#199.

This CL changes the behavior of the router rule registration in the
ServiceWorker Static Routing API, especially when the |urlPattern|
condition receives URLPatternInit or USVString.

Before this CL, the URLPatternInit input was accepted as it is, that
means any unspecified fields are resulted in the wildcards (*). This
behavior is inconsistent with the case when |urlPattern| accepts a
string. When a string is passed, missing fields are complemented by
baseURL, the SW script URL is internally treated as baseURL.

After this CL, the URLPatternInit input also internally uses the SW
script URL as a baseURL if it's not explicitly provided. This is
achieved by the helper method `URLPattern::From()`, which was added in
[1].

This change doesn't affect the case when the input is URLPattern, which
means the input is the object constructed via `new URLPattern()`.

[1] crrev.com/c/5053645

Bug: 1371756
Change-Id: I5cce80fde05cf18237c8b6412b00e017ff5aad5b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5039680
Reviewed-by: Takashi Toyoshima <toyoshimchromium.org>
Reviewed-by: Kouhei Ueno <kouheichromium.org>
Commit-Queue: Shunya Shishido <sisidovskichromium.org>
Reviewed-by: Yoshisato Yanagisawa <yyanagisawachromium.org>
Auto-Submit: Shunya Shishido <sisidovskichromium.org>
Cr-Commit-Position: refs/heads/main{#1229724}

--

wpt-commits: ffe06ef95d4d48be147e640fd9d1c489b810f929
wpt-pr: 43272

UltraBlame original commit: 8cba5e465039e56ad23ca65c4e4b1c307f2f8379
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Dec 7, 2023
…while routing rule registration, a=testonly

Automatic update from web-platform-tests
Add baseURL to the URLPattern condition while routing rule registration

This behavior change was originally started in
whatwg/urlpattern#182, and follows the spec
change in whatwg/urlpattern#199.

This CL changes the behavior of the router rule registration in the
ServiceWorker Static Routing API, especially when the |urlPattern|
condition receives URLPatternInit or USVString.

Before this CL, the URLPatternInit input was accepted as it is, that
means any unspecified fields are resulted in the wildcards (*). This
behavior is inconsistent with the case when |urlPattern| accepts a
string. When a string is passed, missing fields are complemented by
baseURL, the SW script URL is internally treated as baseURL.

After this CL, the URLPatternInit input also internally uses the SW
script URL as a baseURL if it's not explicitly provided. This is
achieved by the helper method `URLPattern::From()`, which was added in
[1].

This change doesn't affect the case when the input is URLPattern, which
means the input is the object constructed via `new URLPattern()`.

[1] crrev.com/c/5053645

Bug: 1371756
Change-Id: I5cce80fde05cf18237c8b6412b00e017ff5aad5b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5039680
Reviewed-by: Takashi Toyoshima <toyoshimchromium.org>
Reviewed-by: Kouhei Ueno <kouheichromium.org>
Commit-Queue: Shunya Shishido <sisidovskichromium.org>
Reviewed-by: Yoshisato Yanagisawa <yyanagisawachromium.org>
Auto-Submit: Shunya Shishido <sisidovskichromium.org>
Cr-Commit-Position: refs/heads/main{#1229724}

--

wpt-commits: ffe06ef95d4d48be147e640fd9d1c489b810f929
wpt-pr: 43272

UltraBlame original commit: 8cba5e465039e56ad23ca65c4e4b1c307f2f8379
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Dec 7, 2023
…while routing rule registration, a=testonly

Automatic update from web-platform-tests
Add baseURL to the URLPattern condition while routing rule registration

This behavior change was originally started in
whatwg/urlpattern#182, and follows the spec
change in whatwg/urlpattern#199.

This CL changes the behavior of the router rule registration in the
ServiceWorker Static Routing API, especially when the |urlPattern|
condition receives URLPatternInit or USVString.

Before this CL, the URLPatternInit input was accepted as it is, that
means any unspecified fields are resulted in the wildcards (*). This
behavior is inconsistent with the case when |urlPattern| accepts a
string. When a string is passed, missing fields are complemented by
baseURL, the SW script URL is internally treated as baseURL.

After this CL, the URLPatternInit input also internally uses the SW
script URL as a baseURL if it's not explicitly provided. This is
achieved by the helper method `URLPattern::From()`, which was added in
[1].

This change doesn't affect the case when the input is URLPattern, which
means the input is the object constructed via `new URLPattern()`.

[1] crrev.com/c/5053645

Bug: 1371756
Change-Id: I5cce80fde05cf18237c8b6412b00e017ff5aad5b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5039680
Reviewed-by: Takashi Toyoshima <toyoshimchromium.org>
Reviewed-by: Kouhei Ueno <kouheichromium.org>
Commit-Queue: Shunya Shishido <sisidovskichromium.org>
Reviewed-by: Yoshisato Yanagisawa <yyanagisawachromium.org>
Auto-Submit: Shunya Shishido <sisidovskichromium.org>
Cr-Commit-Position: refs/heads/main{#1229724}

--

wpt-commits: ffe06ef95d4d48be147e640fd9d1c489b810f929
wpt-pr: 43272

UltraBlame original commit: 8cba5e465039e56ad23ca65c4e4b1c307f2f8379
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants