Extend "window open steps" to observe COOP #5760
Conversation
Specify that `window.open` and `document.open` must return `null` when COOP restrictions are being enforced.
| <span>the rules for choosing a browsing context</span> given <var>targetAttributeValue</var>, | ||
| <var>source</var>, and <var>noopener</var>.</p></li> | ||
|
|
||
| <li><p>Let <var>replace</var> be true if <var>creation condition</var> is either "<code |
There was a problem hiding this comment.
I would prefer not introducing replace here and instead using a non-null check later on with creation condition. (I also agree with @domenic on naming and would further prefer camel case variable names.)
(Same below.)
There was a problem hiding this comment.
Excellent idea. It's so much easier to grep for "windowType" than for "creation condition" and the empty space it can contain.
| @@ -78392,6 +78403,10 @@ dictionary <dfn>WindowPostMessageOptions</dfn> : <span>PostMessageOptions</span> | |||
| tab.</p> | |||
| </li> | |||
|
|
|||
| <li><p>Let <var>new</var> be true if <var>creation condition</var> is either "<code | |||
There was a problem hiding this comment.
Similar to not needing replace anymore, I don't think we need new either.
There was a problem hiding this comment.
new is referenced several times in this algorithm. It seems to me that removing it would make the subsequent steps much more verbose.
source
Outdated
| @@ -78433,7 +78448,8 @@ dictionary <dfn>WindowPostMessageOptions</dfn> : <span>PostMessageOptions</span> | |||
| </ol> | |||
| </li> | |||
|
|
|||
| <li><p>If <var>noopener</var> is true, then return null.</p></li> | |||
| <li><p>If <var>noopener</var> is true or <var>creation condition</var> is "<code | |||
| data-x="">restricted</code>", then return null.</p></li> | |||
There was a problem hiding this comment.
Is there a scenario where noopener is true but creation condition is not restricted? I guess that can happen when you do named targeting and a browsing context gets reused. I wonder if browsers still return null in that case even though there is an opener relationship.
|
Also, great work here, both at uncovering the bug and identifying the fix! |
:D Thanks for the review, folks! With the exception of the removal of new, I think I've addressed it all. |
domenic
added
the
topic: cross-origin-opener-policy
Resolves gh-5759
As discussed in that issue, introducing an additional boolean return value to "choosing a browsing context" would risk complicating call sites (since not all call sites would use the new value, and they would likely need to explicitly state that in some way). In addition, because the conditions are not independent, only three of the four combinations are possible. Changing the existing return value from boolean to a three-value enum avoids both of these drawbacks.
/browsers.html ( diff )
/form-control-infrastructure.html ( diff )
/links.html ( diff )
/window-object.html ( diff )