-
Notifications
You must be signed in to change notification settings - Fork 3k
Move has cross-site ancestor
to the environment, from the ESO
#11540
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?
Move has cross-site ancestor
to the environment, from the ESO
#11540
Conversation
Updates whatwg#11133 This reverts commit c736250.
I'm curious what the motivation for this change is? It seems we landed on this being an algorithm on environment settings object in #11133 (comment). Can you document in the OP what necessitates the reversal here? |
I spoke with @annevk about this- We need this bit before the Fetch for a document is issued, to determine the correct partitioning for that fetch's cookies. At that stage, we don't have an ESO yet, since it is built when converting the fetch response into the Document IIUC. |
See whatwg/storage#182 (comment) for context. I agree we should include that context in the eventual commit message. |
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.
Sorry to have gone OOO just after you did this. I'm back now!
I'd love thoughts from @domfarolino, @noamr, or anyone else on my inline question about why we only copy some things from reserved environments and not others. It shouldn't block this PR but getting more people to understand these parts of the spec is always a win.
data-x="concept-settings-object-origin">origin</span> is not <span>same site</span> with | ||
<var>navigable</var>'s <span data-x="nav-document">active document</span>'s <span>relevant | ||
settings object</span>'s <span data-x="concept-settings-object-origin">origin</span>, then | ||
set <var>hasCrossSiteAncestor</var> to true.</p></li> |
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.
Unless I'm misunderstanding the goal here, "navigable's active documents' relevant settings object" seems like the wrong thing to inspect here.
(Side note: as a nit, if you have a document, you can just check its concept-document-origin
directly instead of getting its relevant settings object.)
Consider navigating an iframe from embedded in https://a.example/
from https://a.example/
to https://b.example/
which redirects to https://c.example/
. What the spec as written in this PR does:
- parentEnvironment's origin is
https://a.example
. - navigable's active document's (relevant settings object's) origin is also
https://a.example
, since the document currently active in the iframe is displayinghttps://a.example/
- So, we set hasCrossSiteAncestor to false.
- So, the reserved client we create sets it to false.
- The same is true for both legs of the redirect, i.e. both paths through the step 21 "While true" loop.
I think you probably want to use responseOrigin, which is computed down in step 21.11. It's... not obvious how to pull step 21.11 up to before 21.2, since it depends on step 21.10, which depends on step 21.9, which depends on step 21.2.
URL</span> to <var>topLevelCreationURL</var>, <var>settings object</var>'s <span>top-level | ||
origin</span> to <var>topLevelOrigin</var>, and <var>settings object</var>'s <span | ||
data-x="concept-environment-cross-site-ancestor">has cross-site ancestor</span> to | ||
<var>hasCrossSiteAncestor</var>.</p></li> |
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.
Can we copy it from the reserved client instead of re-computing it? It seems like the intent is to perform the same calculation.
On the other hand, we don't copy top-level origin or top-level creation URL, so probably your version is more correct. I just don't fully understand why.
@@ -108624,6 +108637,15 @@ new PaymentRequest(…); // Allowed to use | |||
involved.</p> | |||
</dd> | |||
|
|||
<dt>A <dfn data-x="concept-environment-cross-site-ancestor" export for="environment">has | |||
cross-site ancestor</dfn></dt> | |||
<dd><p>A boolean. For window <span>environment settings object</span>s this will be set to true |
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.
<dd><p>A boolean. For window <span>environment settings object</span>s this will be set to true | |
<dd><p>A boolean. For window <span data-x="environment settings object">environment settings objects</span> this will be set to true |
Updates #11133
(See WHATWG Working Mode: Changes for more details.)
/browsing-the-web.html ( diff )
/document-lifecycle.html ( diff )
/document-sequences.html ( diff )
/nav-history-apis.html ( diff )
/webappapis.html ( diff )
/workers.html ( diff )
/worklets.html ( diff )