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

[URL] Split forbidden host/domain code points and add more tests for each #32524

Merged
merged 2 commits into from
Jan 28, 2022

Conversation

karwa
Copy link
Contributor

@karwa karwa commented Jan 25, 2022

No description provided.

@annevk
Copy link
Member

annevk commented Jan 25, 2022

@domenic can you verify these with whatwg-url? From a cursory look this seems good.

@domenic
Copy link
Member

domenic commented Jan 25, 2022

It's not clear to me this is actually the direction we want to go per whatwg/url#685 (comment), so I'll hold off on further reviews until we get that clarified. It feels like we jumped to the PRs stage earlier than makes sense.

domenic pushed a commit that referenced this pull request Jan 25, 2022
These give much more full coverage for host code points. The expectations will be changed in #32524 to follow the spec change proposed in whatwg/url#685; this initial PR gives us a baseline so we can be clear about what the changes are.
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, will let @annevk push the merge button when he's ready to merge whatwg/url#685

annevk pushed a commit to whatwg/url that referenced this pull request Jan 28, 2022
In particular C0 controls and U+007F.

Tests: web-platform-tests/wpt#32524.

Fixes #319 and fixes #627.
@annevk annevk merged commit 2a64dae into web-platform-tests:master Jan 28, 2022
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Feb 4, 2022
jamienicol pushed a commit to jamienicol/gecko that referenced this pull request Feb 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants