-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Add cross-site ancestor flag to environment. #11133
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
This looks good to me, but talking to @cfredric we'd slightly prefer the term "ancestry" with "cross-site" or "same-site" as values. @bvandersloot-mozilla WDYT? |
@@ -102793,6 +102808,15 @@ location.href = '#foo';</code></pre> | |||
|
|||
<li><p>Set <var>topLevelOrigin</var> to <var>parentEnvironment</var>'s <span>top-level | |||
origin</span>.</p></li> | |||
|
|||
<li><p>If <var>embedder</var> is not null, then set <var>hasCrossSiteAncestor</var> to |
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.
embedder does not exist in this algorithm.
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.
seems the algorithm changed around the patch here. I want to compare navaigable's active document's relevant settings object's origin to the parentEnvironment, taking the parentEnvironment's bit as a sufficient condition to set it.
not <span>same site</span> with the current window's <span | ||
data-x="concept-settings-object-origin">origin</span> and otherwise false. For workers and | ||
worklets it is set to the <span data-x="concept-environment-cross-site-ancestor">has cross-site | ||
ancestor</span> of its creator.</p></dd> |
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.
This also seems like it could live on environment settings objects, instead of environments.
Also, instead of computing this ahead of time, why not just make it a "getter" that looks at the ancestor chain when consulted?
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 have no strong opinion on putting it between the environment settings objects and environment. LMK your preference.
I think computing ahead of time is probably what we want to be certain there are no partition changes within the lifetime of an environment.
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 would prefer putting this on environment settings objects, and reserving environment fields for things that need to exist before the global is created. (Because, e.g., they impact service worker processing.)
The necessary changes:
- Environment settings objects have algorithms, instead of fields. The algorithm can be a simple getter for a static value though, so if you want to preserve the current architecture of computing and snapshotting, you can keep passing in a boolean to "create a window environment settings object", and have the algorithm return that boolean.
- We need to update
#set-up-a-worker-environment-settings-object
(easy, same pattern),#set-up-a-worklet-environment-settings-object
(currently missing), and https://w3c.github.io/ServiceWorker/#setup-serviceworkerglobalscope-algorithm (currently missing)
(To be clear, even if we stuck with environment, the fact that we haven't updated the worklet and service worker creation steps is a bug with the current PR.)
However, let's discuss computing ahead of time vs. doing this as an algorithm, because if it's done as an algorithm it would significantly simplify this spec change.
If we did this as an algorithm, the algorithm would presumably be something like the following, for windows:
- If window's navigable's parent is null, then return false.
- Let parentDocument be window's navigable's parent's active document.
- If parentDocument's relevant settings object's has cross-site ancestor is true, then return true.
- If parentDocument's origin is not same site with window's associated
Document
, then return true. - Return false.
How could this ever change over time?
Note that a navigable's parent can only change in a single way: from a specific parent, to null. It can never be re-parented or something like that.
Well, if the iframe gets disconnected, then it could flip from true to false. Is that bad? I dunno, all storage APIs should be guarded so that they don't do anything in disconnected documents, so it seems probably OK...
Could the cross siteness change? I don't believe so. The only way that origins are mutable is with document.domain
, which does not affect site computation.
So, I think this would be better expressed as an algorithm. Then you would not have to thread this through anything.
<var>origin</var> is not <span>same site</span> with <var>embedder</var>'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> | ||
|
||
<li><p><span>Set up a window environment settings object</span> with <code>about:blank</code>, | ||
<var>realm execution context</var>, null, <var>topLevelCreationURL</var>, and | ||
<var>topLevelOrigin</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.
I think this needs to pass in hasCrossSiteAncestor.
|
||
<li><p>If <var>hasCrossSiteAncestor</var> is false, <var>parentEnvironment</var>'s <span | ||
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 |
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.
<var>navigable</var>'s' <span data-x="nav-document">active document</span>'s | |
<var>navigable</var>'s <span data-x="nav-document">active document</span>'s |
<li><p>Set <var>hasCrossSiteAncestor</var> to <var>parentEnvironment</var>'s <span | ||
data-x="concept-environment-cross-site-ancestor">has cross-site ancestor</span>.</p></li> | ||
|
||
<li><p>If <var>hasCrossSiteAncestor</var> is false, <var>parentEnvironment</var>'s <span |
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.
<li><p>If <var>hasCrossSiteAncestor</var> is false, <var>parentEnvironment</var>'s <span | |
<li><p>If <var>parentEnvironment</var>'s <span |
<var>embedder</var>'s <span>relevant settings object</span>'s <span | ||
data-x="concept-environment-cross-site-ancestor"> has cross-site ancestor</span>.</p></li> | ||
|
||
<li><p>If <var>hasCrossSiteAncestor</var> is false, <var>embedder</var> is not null, and |
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.
<li><p>If <var>hasCrossSiteAncestor</var> is false, <var>embedder</var> is not null, and | |
<li><p>If <var>embedder</var> is not null and |
<li><p>Set <var>hasCrossSiteAncestor</var> to <var>parentEnvironment</var>'s <span | ||
data-x="concept-environment-cross-site-ancestor">has cross-site ancestor</span>.</p></li> | ||
|
||
<li><p>If <var>hasCrossSiteAncestor</var> is false and <var>navigationParams</var>'s <span |
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.
<li><p>If <var>hasCrossSiteAncestor</var> is false and <var>navigationParams</var>'s <span | |
<li><p>If <var>navigationParams</var>'s <span |
not <span>same site</span> with the current window's <span | ||
data-x="concept-settings-object-origin">origin</span> and otherwise false. For workers and | ||
worklets it is set to the <span data-x="concept-environment-cross-site-ancestor">has cross-site | ||
ancestor</span> of its creator.</p></dd> |
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 would prefer putting this on environment settings objects, and reserving environment fields for things that need to exist before the global is created. (Because, e.g., they impact service worker processing.)
The necessary changes:
- Environment settings objects have algorithms, instead of fields. The algorithm can be a simple getter for a static value though, so if you want to preserve the current architecture of computing and snapshotting, you can keep passing in a boolean to "create a window environment settings object", and have the algorithm return that boolean.
- We need to update
#set-up-a-worker-environment-settings-object
(easy, same pattern),#set-up-a-worklet-environment-settings-object
(currently missing), and https://w3c.github.io/ServiceWorker/#setup-serviceworkerglobalscope-algorithm (currently missing)
(To be clear, even if we stuck with environment, the fact that we haven't updated the worklet and service worker creation steps is a bug with the current PR.)
However, let's discuss computing ahead of time vs. doing this as an algorithm, because if it's done as an algorithm it would significantly simplify this spec change.
If we did this as an algorithm, the algorithm would presumably be something like the following, for windows:
- If window's navigable's parent is null, then return false.
- Let parentDocument be window's navigable's parent's active document.
- If parentDocument's relevant settings object's has cross-site ancestor is true, then return true.
- If parentDocument's origin is not same site with window's associated
Document
, then return true. - Return false.
How could this ever change over time?
Note that a navigable's parent can only change in a single way: from a specific parent, to null. It can never be re-parented or something like that.
Well, if the iframe gets disconnected, then it could flip from true to false. Is that bad? I dunno, all storage APIs should be guarded so that they don't do anything in disconnected documents, so it seems probably OK...
Could the cross siteness change? I don't believe so. The only way that origins are mutable is with document.domain
, which does not affect site computation.
So, I think this would be better expressed as an algorithm. Then you would not have to thread this through anything.
This is commandeering #8036, and is meant to land with whatwg/fetch#1807 in its place.
(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 )