-
Notifications
You must be signed in to change notification settings - Fork 67
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
URLManifestItem.url shouldn't contain non-URL-unit characters #181
base: master
Are you sure you want to change the base?
Conversation
96ba17f
to
d74b72a
Compare
(There's also the variant of this RFC where all we do is |
Co-authored-by: Ms2ger <[email protected]>
Given how few such files there are, let's just disallow them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Storing URLs (and therefore test ids) in url-encoded form does make sense.
Migrating expectation data is a challenge. I wonder if for the wpt expectation files (which handle spaces fine) we could just unencode the test id before using the manifest? That makes it more likely that the manifest will match whatever people write in the <meta name=varaint>
. Alternatively if we're going to change to using the URL encoded form everywhere, I think we should have a lint to ensure we use it consistently (maybe we should have a lint either way, so that we don't get %20
in some places and
in another).
I don't think banning spaces in test names is worthwhile.
I just searched for spaces in filenames and found two cases that don't seem intentional: web-platform-tests/wpt#46253 Is there much of a downside to banning spaces? |
Rendered