-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Inline URLUtilsReadOnly since the URLUtils abstraction is not working out #187
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
Conversation
source
Outdated
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.
Ugh, the en-GB-x-hixie vs. en-US is screwing us here. Are you sure you want (some of) the data-x's to use z, matching URL, whereas the human text uses s?
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.
|'m not. I guess once we use data-x-href more consistently the internal data-x value is really irrelevant. We could also move back to US and perhaps no longer declare -hixie since he's no longer the sole editor.
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.
Yeah, maybe a good long term plan. In the meantime maybe just stay consistent? Not a blocker for this PR though IMO.
|
There's actually a small theoretical problem that I wonder if anyone has input on. When |
|
I really am not worried about that personally, since you can't run script that accesses If you make the change to having a WorkerGlobalScope, then maybe I'd think of inserting a note like "This WorkerLocation's associated WorkerGlobalScope will only have its url property set after the worker script has been fetched. This does not impact the above algorithms, since these getters can only run after script has been fetched." |
0125db0 to
05ce65a
Compare
|
My main worry now is that while |
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.
s/the/this
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.
Not doing this given the definition of self. Once IDL defines this life will get better.
|
Commit message title is too long by GitHub's standards |
|
LGTM with nits |
The URLUtils abstraction is not working out. See #164 and whatwg/url#62 for details.
05ce65a to
32a7a20
Compare
This fixes whatwg#187 by hard-coding knowledge of the post-syntax-highlighting markup into the script.
See whatwg/url#62 for details.