-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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 how disowning works #4318
Conversation
I think this ends up being editorial, but it makes it even more painfully clear how true #3874 by @gterzian is. That currently disowning has no noticeable effect. Perhaps that's okay though and better than the status quo. I could change my source comment into an issue marker for #313 and at least it'd be a little more transparent? |
I did some of this in #4284 so I thought that perhaps landing this separately would be good so not too much is done in that PR. |
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 having trouble understanding all the consequences here, in this intermediate world. Can you help me fill this in, so I can better review this and #4284?
noopener
link relations:- impacts navigation via "the rules for choosing a browsing context". Basically seems to only affect sessionStorage copying now?
- No longer sets the disowned boolean
- Thus,
window.opener
will return the opener. - This seems like a regression.
opener
setter/getter:- The setter sets the "disowned" boolean, which the getter consults. No further impact (i.e. doesn't change auxiliary vs. top-level, or anything else).
window.open()
with the noopener feature:- Goes down "the rules for choosing a browsing context" path, which as far as I can tell only consults noopener for sessionStorage copying.
- No longer sets the disowned boolean
- Thus,
window.opener
will return the opener. - This seems like a regression.
<p>The keyword indicates that any newly created <span>top-level browsing context</span> which | ||
results from following the <span>hyperlink</span> is not an <code>auxiliary browsing | ||
context</code>. E.g., its <code data-x="dom-opener">window.opener</code> attribute will be | ||
null.</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.
It might be good to link to where the processing model implements this, as an addition.
following the <span>hyperlink</span> will be <span>disowned</span>, which means that its <code | ||
data-x="dom-opener">window.opener</code> attribute will be null.</p> | ||
<p>The keyword indicates that any newly created <span>top-level browsing context</span> which | ||
results from following the <span>hyperlink</span> is not an <code>auxiliary browsing |
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.
code -> span
<p>The keyword indicates that any newly created <span>top-level browsing context</span> which | ||
results from following the <span>hyperlink</span> is not an <code>auxiliary browsing | ||
context</code>. E.g., its <code data-x="dom-opener">window.opener</code> attribute will be | ||
null.</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.
An example would be nice. A link that would normally create an auxiliary browsing context, plus one with noopener that does not.
<li><p>Return the current <span>browsing context</span>'s <span>opener browsing context</span>'s | ||
<code>WindowProxy</code> object.</p></li> | ||
</ol> | ||
|
||
<p>The <code data-x="dom-opener">opener</code> attribute's setter, must run these steps:</p> | ||
|
||
<ol> | ||
<li><p>If the given value is null, then <span data-x="disowned">disown</span> the current | ||
<li><p>If the given value is null and the current <span>browsing context</span> is an | ||
<span>auxiliary browsing context</span>, then <span data-x="disowned">disown</span> the current |
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 is a normative change to fall back to step 2 for non-auxiliary browsing contexts, instead of returning. (Well, the previous spec would probably crash, since it would try to call "disown" on a non-auxiliary BC, which per its definition is not allowed. But I think a reasonable reading is that it would return.)
Have you tested what gets done in browsers?
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 merging this into the other PR, but this is a good question that needs an answer either way.
const beforeDesc = Object.getOwnPropertyDescriptor(self, "opener"),
openerGet = beforeDesc.get;
alert(openerGet());
self.opener = null;
self.opener = "test";
alert(openerGet());
in an iframe targeted with window.open('theabovedocument.html', 'theiframename')
that alerts Window followed by null in Firefox. Chrome and Safari throw on openerGet()
however. So yeah, disowning needs to affect all browsing contexts. I'm pretty sure from other tests it only affects link targeting when it's done on a auxiliary browsing context though.
I guess I have to merge this into #4284. Disowning only affects auxiliary browsing contexts. In particular it makes the browser skip those browsing contexts when doing name lookup. It doesn't really severe the opener browsing context relationship as someone could have already obtained a reference. |
Closing in favor of #4318, which now captures everything captured here. |
/browsers.html ( diff )
/links.html ( diff )
/window-object.html ( diff )