-
Notifications
You must be signed in to change notification settings - Fork 23
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
Process a URLPatternInit doesn't call basic URL parser on base URL #242
Comments
Another editorial problem I noticed in the same AO is that "process a URLPatternInit" does:
But URL's host is a concept/variant, and is not a string. e.g it may be an ipv4 address which is a 32 bit integer. I believe the spec should be calling this on host's serialization instead (https://url.spec.whatwg.org/#concept-host-serializer). But I can split this into a different issue if preferred (I will probably run into a few of these type of things, I am implementing the spec from scratch). |
One more I noticed I can also split out into a different issue, but putting it down here for reference. Parse a pattern string (https://urlpattern.spec.whatwg.org/#parse-a-pattern-string), step 3.9.1 does:
But not prefix is in scope, so I believe should be 'let'. Similar story for modifier token on step 3.9.6 |
Noticed another thing (I will do a pass over this afterwards and clean up this issue). Canonicalizing an opaque pathname https://urlpattern.spec.whatwg.org/#canonicalize-an-opaque-pathname On step 3. does:
But in this case the URL path is a list. So I guess this must means to set it to a list with one item of the empty string? |
Blob handling is not required, since the resulting URL is not stored in any way that would make the blob handling visible. This is consistent with what the URL constructor and similar uses do. This change should not be observable. Partially addresses whatwg#242.
re. canonicalize an opaque pathname, the URL path can be either a list of path segments (in the case of a non-opaque path) or a single path segment (in the case of an opaque path). As this is the latter case, I think the spec is correct. Whether your implementation internally treats this case as a list containing a single path segment plus an opaque flag is up to you, but I believe this is intended to cause the basic URL parser's opaque path state (which expects this to be a string, not a list of strings) to operate correctly. The other three things you've reported I agree with and have opened pull requests. Thank you very much for pointing them out. |
Oh right, thanks! Yeah that last one was my misunderstanding! |
Use the basic URL parser when parsing URLs Blob handling is not required, since the resulting URL is not stored in any way that would make the blob handling visible. This is consistent with what the URL constructor and similar uses do. This change should not be observable. Partially addresses #242.
What is the issue with the URL Pattern Standard?
process a URLPatternInit does:
Which has blob handling, which does not seem required. This seems like it should be changed to invoke the basic URL parser instead: https://url.spec.whatwg.org/#concept-basic-url-parser
The text was updated successfully, but these errors were encountered: