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.

Sign up for GitHub

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

Jump to bottom

Make the match algorithm accept the URL struct #221

Merged
merged 6 commits into from
Feb 29, 2024
Merged

Make the match algorithm accept the URL struct #221

merged 6 commits into from
Feb 29, 2024

Conversation

yoshisatoyanagisawa
Copy link
Contributor

@yoshisatoyanagisawa yoshisatoyanagisawa commented Feb 27, 2024 •
edited by pr-preview bot
Loading

In w3c/ServiceWorker#1701, the ServiceWorker static routing API uses the match algorithm to match the request URL. However, the original match algorithm accepts a URLPatternInput object, and it does not accept a URL struct. To allow the ServiceWorker static routing API to use the match algorithm, we need to make the match algorithm to accept the URL struct.

Fixes: #218.

  • At least two implementers are interested (and none opposed):
    This is a part of the ServiceWorker static routing API.
    • Google Chrome
    • Firefox
  • Tests are written and can be reviewed and commented upon at:
    • n/a (this is just clarifying an implicit behavior. There is no actual behavior change.)
  • Implementation bugs are filed:
  • MDN issue is filed: n/a (change is on a spec detail not expressly documented)
  • The top of this comment includes a clear commit message to use.

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

Preview | Diff

@yoshisatoyanagisawa
Make test() and exec() accept URL
9aa8046 
The original specification did not explicitly accept a URL object as an
input, and expected implicit conversion from the URL object to the
URLPatternInit object for matching.

This change make the URL object accepted as an input explicitly.
This is a step to fix whatwg#218.
domenic
domenic requested changes Feb 28, 2024
View reviewed changes
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
@yoshisatoyanagisawa yoshisatoyanagisawa changed the title Make test() and exec() accept URL Make the match algorithm accept URL Feb 28, 2024
@yoshisatoyanagisawa yoshisatoyanagisawa changed the title Make the match algorithm accept URL Make the match algorithm accept the URL struct Feb 28, 2024
@yoshisatoyanagisawa
Addressed comments.
6983a8c 
- Not chagne `exec()` or `test()` arguments.
- make the match algorithm accept [=/url=].
- By merging existing steps, omitted the explicit declaration of
  anchors.
@yoshisatoyanagisawa
Minor fix on Let and Set.
1e9b190
sisidovski
sisidovski approved these changes Feb 28, 2024
View reviewed changes
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 w/ nit

spec.bs Outdated Show resolved Hide resolved
domenic
domenic approved these changes Feb 28, 2024
View reviewed changes
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 with nit

spec.bs Outdated Show resolved Hide resolved
@yoshisatoyanagisawa
Copy link
Contributor Author

Thanks for the review, both.
@jeremyroman Will you take a look?

jeremyroman
jeremyroman approved these changes Feb 28, 2024
View reviewed changes
spec.bs Outdated Show resolved Hide resolved
@yoshisatoyanagisawa @jeremyroman
Update spec.bs
483bccb 
Co-authored-by: Jeremy Roman <[email protected]>
@yoshisatoyanagisawa
Copy link
Contributor Author

Thanks for the review.
Will somebody merge this?

@sisidovski sisidovski merged commit 0154992 into whatwg:main Feb 29, 2024
2 checks passed
@yoshisatoyanagisawa yoshisatoyanagisawa deleted the accept_url branch February 29, 2024 01:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sisidovski sisidovski sisidovski approved these changes

@domenic domenic domenic approved these changes

@jeremyroman jeremyroman jeremyroman approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Accept URL as the match algorithm's input.
4 participants
@yoshisatoyanagisawa @jeremyroman @domenic @sisidovski