-
Notifications
You must be signed in to change notification settings - Fork 136
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
Editorial: fold cannot-be-base-URL into path #655
Conversation
Fetch PR: TODO. HTML PR: TODO. Fixes #634.
I think URLPattern would also need to be updated after this change. |
Thanks Ben! This took a bit more effort than expected, but I think it does look rather nice and having the URL path serializer will also allow for some nice cleanup elsewhere, in particular in HTML. This will need to be validated in jsdom's whatwg-url (ideally by someone that's not me) and I'd like @karwa @TimothyGu @alwinb @domenic to be okay with it. I'm not a big fan of "string path state". I considered renaming path in the other path-related states to hierarchical path, but I think it would be confusing if the lone path state ended up being the exceptional case. |
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.
I really like this cleanup! It would also match how Go's net/url package stores paths better, and convergence is always nice.
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.
I like this a lot, much better!
I addressed the review comments. This made me think "hierarchical path" was no longer pulling its weight. I hope that's still agreeable. We could always add it back at some future point once there is a more compelling need. I'll see how easy it is to update whatwg-url these days. Edit: it was straightforward and the tests are passing. |
I just spotted that the id attribute on the opaque path state is unchanged as "cannot-be-a-base-url-path-state". I don't know if that is intentional. |
Yeah that is intentional. I prefer keeping old IDs around so old links keep working. I also like that it shows the document has some history. We can of course add a new ID as well, but it's not always clear to me that is worth the additional bytes and nodes. |
These URLs now have a path whose value is a string rather than a list. See whatwg/url#655 for context.
These URLs now have a path whose value is a string rather than a list. See whatwg/url#655 for context.
This helps whatwg/urlpattern#141. #655 will export the remaining state.
This helps whatwg/urlpattern#141. #655 exported the remaining state.
These URLs now have a path whose value is a string rather than a list. See whatwg/url#655 for context.
These URLs now have a path whose value is a string rather than a list. See whatwg/url#655 for context.
Follows whatwg/url#655. Also introduces some new APIs.
(Apologies for not responding sooner; I've been away. I didn't expect all this activity! 😅) Thank you so much @annevk! This is really fantastic; it seems to have involved coordinating a lot of updates to other standards, which can't have been easy, but I think it makes this standard significantly better. The motivation for #634 was so I could explain to users of my URL library which operations may fail and why, without inventing my own terminology that they can't search for on the web. This helps make a lot of the limitations around URLs with opaque paths much less... well, opaque:
Which is a lot better than saying "because the parser decided to set some flag which forbids it". So I definitely appreciate it, and I'm sure my users will also very much appreciate it. |
Automatic update from web-platform-tests URL: test origin of blob: URLs For whatwg/url#655. -- wpt-commits: 5acc42721ce5811462acc297bff75d33f999cd8f wpt-pr: 31305
Automatic update from web-platform-tests URL: test origin of blob: URLs For whatwg/url#655. -- wpt-commits: 5acc42721ce5811462acc297bff75d33f999cd8f wpt-pr: 31305
* Editorial: align with URL's cannot-be-base-URL removal These URLs now have a path whose value is a string rather than a list. See whatwg/url#655 for context. * Apply suggestions from code review Co-authored-by: Anne van Kesteren <annevk@annevk.nl> Co-authored-by: Ben Kelly <ben@wanderview.com>
These URLs now have a path whose value is a string rather than a list. See whatwg/url#655 for context.
These URLs now have a path whose value is a string rather than a list. See whatwg/url#655 for context.
These URLs now have a path whose value is a string rather than a list. See whatwg/url#655 for context.
Fetch PR: whatwg/fetch#1337.
HTML PR: whatwg/html#7240.
URLPattern PR: whatwg/urlpattern#143.
Fixes #634.
Preview | Diff