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
Cleanup origin serialization #870
Conversation
@SimonSapin, you might want to review this. |
part of the <span>origin</span> tuple, and append the results — each component, in the same | ||
order, separated by U+002E FULL STOP characters (.) — to <var>result</var>. <ref spec=URL></p></li> | ||
<li><p>Let <var>unicodeHost</var> be <var>host</var> if <var>host</var> is not a | ||
<span>domain</span>, and the result of applying <span>domain to Unicode</span> to <var>host</var> |
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.
data="concept-domain"
*
(As a side note, I find it hard to find the one thing that changed when a paragraph is almost entirely red/green in the diff because it was re-wrapped. Please consider http://rhodesmill.org/brandon/2012/one-sentence-per-line/. Maybe not to rewrap everything at once, but for paragraphs that you otherwise edit.) |
Looks good to me. |
FWIW, I added the do not merge yet label because it needs to land after #868 and I need to fix the nit above. |
@@ -78104,7 +78103,7 @@ callback <dfn>FrameRequestCallback</dfn> = void (<span>DOMHighResTimeStamp</span | |||
<dl> | |||
|
|||
|
|||
<dt><dfn data-x="concept-origin-opaque-identifiers">Opaque identifiers</dfn></dt> | |||
<dt><dfn data-x="concept-origin-opaque-identifier">Opaque identifiers</dfn></dt> |
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.
Why change these and break incoming links?
e3dffca
to
de54b41
Compare
LGTM, land at will |
Origin serialization didn’t properly serialize hosts and ports and used the domain to Unicode and domain to ASCII algorithms incorrectly, passing domain labels rather than domains (not accounting for the fact that the host might not be a domain either). This depends on #868 landing since it assumes aliasing is no longer a thing. This also depends on the URL Standard changing to no longer pass the default port when computing the origin of a URL. Instead, an origin and a URL should share the same data model subset, where port can be null. Since they serialise the same way, that makes things easier to understand. See whatwg/url#102 for that. This fixes #611, https://www.w3.org/Bugs/Public/show_bug.cgi?id=28788, and https://www.w3.org/Bugs/Public/show_bug.cgi?id=29056.
de54b41
to
5dcc1ee
Compare
See whatwg/html#870 and whatwg/url@b0e4def. I don't believe this changes any observable output; it is just a simplification.
Origin serialization didn’t properly serialize hosts and ports and
used the domain to Unicode and domain to ASCII algorithms
incorrectly, passing domain labels rather than domains (not
accounting for the fact that the host might not be a domain either).
This depends on #868 landing since it assumes aliasing is no longer a
thing.
This also depends on the URL Standard changing to no longer pass the
default port when computing the origin of a URL. Instead, an origin
and a URL should share the same data model subset, where port can be
null. Since they serialise the same way, that makes things easier to
understand.
This fixes #611,
https://www.w3.org/Bugs/Public/show_bug.cgi?id=28788, and
https://www.w3.org/Bugs/Public/show_bug.cgi?id=29056.