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

Fix the expectation of the port canonicalization in URLPattern #51312

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sisidovski
Copy link
Contributor

Fixes whatwg/urlpattern#260.

This test expectation was updated in #49782 but it turned out the browser should raise an error.

@wpt-pr-bot
Copy link
Collaborator

There are no reviewers for this pull request. Please reach out on the chat room to get help with this. Thank you!

@sisidovski
Copy link
Contributor Author

@annevk @anonrig Could you review this? Thanks!

@annevk
Copy link
Member

annevk commented Mar 13, 2025

I believe the correct version of this test is this:

  {
    "pattern": [{ "protocol": "http", "port": "80 " }],
    "inputs": [{ "protocol": "http", "port": "80" }],
    "expected_obj": {
      "protocol": "http",
      "port": "80"
    },
    "expected_match": null
  },

That is because "80 " does not fail parsing. It just does not get normalized to the empty string. But it will get normalized to "80" (without trailing space).

@annevk
Copy link
Member

annevk commented Mar 13, 2025

I created #51316 to show how I think this should look like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

port pattern canonicalization seems wrong after all
3 participants