-
Notifications
You must be signed in to change notification settings - Fork 3.1k
[Origin API] Define an Origin interface. (#11534)
#11846
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
base: main
Are you sure you want to change the base?
Conversation
|
I would like to see mikewest/origin-api#6 addressed. |
annevk
left a comment
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.
Some editorial comments. Generally this looks good to me.
whatwg/html#11846 (comment) noted that we should verify schemeful same-site comparison; this CL adds that test. Bug: 434131026 Change-Id: I48878979fc0f8c3dab2caefbe788a14d367e053b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7130621 Reviewed-by: Antonio Sartori <[email protected]> Commit-Queue: Mike West <[email protected]> Cr-Commit-Position: refs/heads/main@{#1541759}
whatwg/html#11846 (comment) noted that we should verify schemeful same-site comparison; this CL adds that test. Bug: 434131026 Change-Id: I48878979fc0f8c3dab2caefbe788a14d367e053b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7130621 Reviewed-by: Antonio Sartori <[email protected]> Commit-Queue: Mike West <[email protected]> Cr-Commit-Position: refs/heads/main@{#1541759}
whatwg/html#11846 (comment) noted that we should verify schemeful same-site comparison; this CL adds that test. Bug: 434131026 Change-Id: I48878979fc0f8c3dab2caefbe788a14d367e053b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7130621 Reviewed-by: Antonio Sartori <[email protected]> Commit-Queue: Mike West <[email protected]> Cr-Commit-Position: refs/heads/main@{#1541759}
… schemeful., a=testonly Automatic update from web-platform-tests [Origin API] Verify that comparisons are schemeful. whatwg/html#11846 (comment) noted that we should verify schemeful same-site comparison; this CL adds that test. Bug: 434131026 Change-Id: I48878979fc0f8c3dab2caefbe788a14d367e053b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7130621 Reviewed-by: Antonio Sartori <[email protected]> Commit-Queue: Mike West <[email protected]> Cr-Commit-Position: refs/heads/main@{#1541759} -- wpt-commits: 08b913918dd419fe48d01ef685781a69ced0a111 wpt-pr: 55938
… schemeful., a=testonly Automatic update from web-platform-tests [Origin API] Verify that comparisons are schemeful. whatwg/html#11846 (comment) noted that we should verify schemeful same-site comparison; this CL adds that test. Bug: 434131026 Change-Id: I48878979fc0f8c3dab2caefbe788a14d367e053b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7130621 Reviewed-by: Antonio Sartori <[email protected]> Commit-Queue: Mike West <[email protected]> Cr-Commit-Position: refs/heads/main@{#1541759} -- wpt-commits: 08b913918dd419fe48d01ef685781a69ced0a111 wpt-pr: 55938
… schemeful., a=testonly Automatic update from web-platform-tests [Origin API] Verify that comparisons are schemeful. whatwg/html#11846 (comment) noted that we should verify schemeful same-site comparison; this CL adds that test. Bug: 434131026 Change-Id: I48878979fc0f8c3dab2caefbe788a14d367e053b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7130621 Reviewed-by: Antonio Sartori <antoniosartorichromium.org> Commit-Queue: Mike West <mkwstchromium.org> Cr-Commit-Position: refs/heads/main{#1541759} -- wpt-commits: 08b913918dd419fe48d01ef685781a69ced0a111 wpt-pr: 55938 UltraBlame original commit: 313ef5bf80f71a4e57fc5bebfd0d784dd667b024
… schemeful., a=testonly Automatic update from web-platform-tests [Origin API] Verify that comparisons are schemeful. whatwg/html#11846 (comment) noted that we should verify schemeful same-site comparison; this CL adds that test. Bug: 434131026 Change-Id: I48878979fc0f8c3dab2caefbe788a14d367e053b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7130621 Reviewed-by: Antonio Sartori <antoniosartorichromium.org> Commit-Queue: Mike West <mkwstchromium.org> Cr-Commit-Position: refs/heads/main{#1541759} -- wpt-commits: 08b913918dd419fe48d01ef685781a69ced0a111 wpt-pr: 55938 UltraBlame original commit: 313ef5bf80f71a4e57fc5bebfd0d784dd667b024
… schemeful., a=testonly Automatic update from web-platform-tests [Origin API] Verify that comparisons are schemeful. whatwg/html#11846 (comment) noted that we should verify schemeful same-site comparison; this CL adds that test. Bug: 434131026 Change-Id: I48878979fc0f8c3dab2caefbe788a14d367e053b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7130621 Reviewed-by: Antonio Sartori <antoniosartorichromium.org> Commit-Queue: Mike West <mkwstchromium.org> Cr-Commit-Position: refs/heads/main{#1541759} -- wpt-commits: 08b913918dd419fe48d01ef685781a69ced0a111 wpt-pr: 55938 UltraBlame original commit: 313ef5bf80f71a4e57fc5bebfd0d784dd667b024
This patch shifts the normative portions of https://mikewest.github.io/origin-api/ into a patch against HTML for discussion. As a followup, this will require changes to [[URL]] and [[ServiceWorker]] as well to define the "extract an origin" for relevant objects. See discussion in w3ctag/design-reviews#1130, WebKit/standards-positions#538, and mozilla/standards-positions#1280.
source
Outdated
| <p class="note">Note that same-site checks might return different values for the same origins in | ||
| different user agents, or even in the same user agent at different times, as the public suffix | ||
| list is updated and distributed in implementation-defined ways. <span | ||
| data-x="dom-Origin-isSameSite">isSameSite()</span> reflects the user agent's current understanding | ||
| of the relationship between two origins, but makes no promises about the future.</p> |
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 added this note in response to @erik-anderson's suggestion in mikewest/origin-api#11. Otherwise, I think this PR is pretty good to go.
If @annevk and @zcorpan are happy-enough, I'll go file bugs against browsers, and see if Chromium folks will let me get it out the door.
WDYT?
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'm not a big fan of this as there's quite a number of APIs this applies to and I don't think we want to call this out all over. I could maybe see adding it to URL (or even better would be the Public Suffix standard once we finally rescue that project from its current state).
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.
Ok. I've taken it back out, and I'll put it up as a PR against URL.
source
Outdated
| <p>The <dfn method for="Origin"><code | ||
| data-x="dom-Origin-isSameOrigin">isSameOrigin(other)</code></dfn> method returns true if | ||
| <span>this</span>'s <span data-x="dom-Origin-origin">origin</span> is <span>same origin</span> | ||
| with <var>other</var>'s <span data-x="dom-Origin-origin">origin</span>, and false otherwise.</p> |
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.
| with <var>other</var>'s <span data-x="dom-Origin-origin">origin</span>, and false otherwise.</p> | |
| with <var>other</var>'s <span data-x="dom-Origin-origin">origin</span>; otherwise false.</p> |
source
Outdated
| <p class="note">Note that same-site checks might return different values for the same origins in | ||
| different user agents, or even in the same user agent at different times, as the public suffix | ||
| list is updated and distributed in implementation-defined ways. <span | ||
| data-x="dom-Origin-isSameSite">isSameSite()</span> reflects the user agent's current understanding | ||
| of the relationship between two origins, but makes no promises about the future.</p> |
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'm not a big fan of this as there's quite a number of APIs this applies to and I don't think we want to call this out all over. I could maybe see adding it to URL (or even better would be the Public Suffix standard once we finally rescue that project from its current state).
mikewest
left a comment
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.
Thanks!
source
Outdated
| <p class="note">Note that same-site checks might return different values for the same origins in | ||
| different user agents, or even in the same user agent at different times, as the public suffix | ||
| list is updated and distributed in implementation-defined ways. <span | ||
| data-x="dom-Origin-isSameSite">isSameSite()</span> reflects the user agent's current understanding | ||
| of the relationship between two origins, but makes no promises about the future.</p> |
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.
Ok. I've taken it back out, and I'll put it up as a PR against URL.
annevk
left a comment
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 was looking at this again today thinking about Document vs Window and now I wonder about Location and WorkerLocation.
I followed blame back to https://bugzilla.mozilla.org/show_bug.cgi?id=931884#c10 (starting at 6bdba31) which suggests to me that obtaining an Origin from Location or WorkerLocation is not something we should offer by default. You can still do it by passing location.href, if you really know what you're doing, but in general those objects don't really hold the authority and so we shouldn't encourage that.
For a moment I thought they'd work anyway because of the string overload, but because we take any it's different I think and we won't attempt to coerce any argument to a string. Which in this case seems for the best.
I understand your point, but it seems unlikely we're going to be able to get rid of |
|
Right, I don't mean to suggest we get rid of that. Each URL-like object should support that, but I do think it's worth trying to hold the line here and get people to adopt the pattern that'll do the right thing in sandboxed environments and the like. We can always relent later. I'm also not entirely sure about |
|
Ah, got it. Sandboxed contexts in which the origin differs between I don't know of any particularly valuable use cases for |
|
That works for me. I wonder how much longer we want to wait with feedback. Perhaps the Wednesday after Thanksgiving week (Dec 3) is a reasonable time to merge this if nothing else comes up (modulo the |
|
I took care of dropping |
|
One other thing we need by that date is a WPT PR to make the tests non-tentative and account for the recent changes. |
This patch adds the
Origininterface to HTML by shifting the normativeportions of https://mikewest.github.io/origin-api/ into this document.
As a followup, this will require changes to [[URL]] and [[ServiceWorker]] as well to define the "extract an origin" for relevant objects.
See discussion in w3ctag/design-reviews#1130, WebKit/standards-positions#538, and mozilla/standards-positions#1280.
OriginObject WebKit/standards-positions#538 is tending positive, though @annevk and @marcoscaceres have provided positive feedback.OriginObject mozilla/standards-positions#1280 is similarly inconclusive.OriginAPI. mdn/mdn#767(See WHATWG Working Mode: Changes for more details.)
/browsers.html ( diff )
/comms.html ( diff )
/index.html ( diff )
/links.html ( diff )
/webappapis.html ( diff )
/workers.html ( diff )