-
Notifications
You must be signed in to change notification settings - Fork 141
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
Is file:///C|/foo/bar meant to be a syntax violation? #209
Comments
Yes, |
It's probably be a good idea to add a note to that effect to https://url.spec.whatwg.org/#windows-drive-letter definition (otherwise you need to step through the state machine to find this out) I was uncertain because there are bunch of test suite files in https://github.com/w3c/web-platform-tests/tree/master/conformance-checkers and https://github.com/validator/tests showing file:///C|/foo/bar as a must-be-valid case. I'll raise an issue in the web-platform-tests repo |
This is supposedly covered by https://url.spec.whatwg.org/#url-syntax which is pretty clear that the | form is disallowed. But I guess there's no overall note explaining that that section is meant to provide a conformance guideline. |
So duplicate of #118 then? |
I guess one thing that's novel to this issue is where those terms in the Infrastructure section should go. They're mostly there because I couldn't find a better place, but maybe they should go into the top-level section 4. |
I think one thing that could be clearer in the current spec is the high level division of URLs into:
Adding some syntax violations to the examples table would make this clearer. Would be good to add file: URLs to the examples table (both conforming and non-conforming) since the long history of interop problems with file: URLs suggests they're a source of much confusion, so need most clarity. |
Also point out that the variant with a trailing “|” is not conforming. Fixes part of #209.
I created some PRs that go somewhat towards those goals @dd8. Hope they help. The main problem with the table at the moment is that it's specific to the parser. I guess you're suggesting we should generalize it? We discussed that a bit in #177 but didn't go for it. I guess I can do that once the above two PRs are accepted as improving the status quo and close this issue with that table change. |
Also point out that the variant with a trailing “|” is not conforming. Fixes part of #209.
Also make sure the base URLs are actually represented in serialized form by adding trailing slashes. Fixes #209.
I posted the final PR, but I forgot to add file URL examples. Will do that now. If anyone has suggestions for ones they think should be included, that would be most welcome. |
file:///C:/foo/bar parses ok using the state machine, but file:///C|/foo/bar flags a syntax violation in the path state because | is not a URL code point while it's scanning for the / at end of the C| path segment:
If c is not a URL code point and not "%", syntax violation.
Is that intended? C| is not the normalised window drive letter form, but it's not clear from any of the narrative it's a syntax violation.
The text was updated successfully, but these errors were encountered: