Refactor browsing context creation #4284

annevk · 2019-01-11T15:26:39Z

Some initial work to make introducing browsing context groups easier.

In particular:

Gives creation of top-level and auxiliary browsing contexts their own algorithm.
Makes the creator browsing context variables set explicitly during creation. (The current definition didn't really make sense, as the created browsing context had no parent, yet we were asking for information from the parent...)
Makes setting the opener browsing context explicit.

@mikewest currently "choosing a browsing context" has some sandboxing integration. It seems this can move to "creating a new auxiliary browsing context"? I haven't made that change yet, but it seems like that's what should happen as presumably it doesn't apply when we create a new non-auxiliary top-level browsing context.

cc @mystor @farre

/browsers.html ( diff )
/iframe-embed-object.html ( diff )
/index.html ( diff )
/links.html ( diff )
/obsolete.html ( diff )
/urls-and-fetching.html ( diff )
/webappapis.html ( diff )
/webstorage.html ( diff )
/window-object.html ( diff )

Some initial work to make introducing browsing context groups easier.

mystor

This seems like a pretty good improvement to me :-) - I don't have as much context as some other contributors to the spec as to other places which might need to be updated, but the parts I see here look pretty good.

I added some comments which are somewhat style-focused - feel free to disregard them.

source

mystor · 2019-01-11T21:06:34Z

@mikewest currently "choosing a browsing context" has some sandboxing integration. It seems this can move to "creating a new auxiliary browsing context"? I haven't made that change yet, but it seems like that's what should happen as presumably it doesn't apply when we create a new non-auxiliary top-level browsing context.

We might want to propagate some sandboxing information into noopener-created toplevel browsing contexts, so I am not sure if we'd want to make that change. That being said, we probably should consider them.

IIRC we do / want to do some propagation of e.g. popup blocking flags into noopener-created toplevel browsing contexts.

annevk · 2019-01-22T14:18:01Z

Edit: so per some local testing sandboxing as well as name setting goes for both auxiliary and non-auxiliary top-level browsing contexts. The difference between auxiliary and non-auxiliary top-level browsing contexts is only whether you get a reference (opener/openee) and session storage, as far as I can tell. It'd be great if someone could confirm. (E.g., @mikewest.)

I'll make the patch reflect that, even if it seems less than ideal.

annevk · 2019-01-23T13:06:18Z

Note: this fixes #2645.

annevk · 2019-01-24T10:29:20Z

The annoying thing with refactoring is that you always run into new things:

Each browsing context can have an opener browsing context (due to window.open() setting it) (would fix Sort out window.opener behavior when a window.open with target targets an existing window #2636)
Each browsing context can be disowned (at least in terms of opener getter returning null, the remainder is Disowning owner should perhaps affect link targeting #313)
noopener ensures a browsing context is created that has no opener browsing context, but there's no reason for it to also be disowned

I'll try to sort that out and also a restructuring of the sections that it requires as well as writing some tests I suppose. It'd be good to get this done though as this blocks making target=_blank behave like noopener, adding Cross-Origin-Opener-Policy, browsing context groups, etc.

annevk · 2019-01-24T15:52:07Z

Tests to write (though perhaps noopener bits should be lightly tested given #1826 will likely change things a bit):

<a> / <area> / <form> only set opener when creating a popup.
window.open() always sets opener, unless noopener is specified.
- Also test that then setting opener to null works and if you then set opener to something meaningless (e.g., a string, undefined, or self) and use the original getter you get null.

domenic

Some editorial suggestions. But my biggest concern is changing the definition of "top-level browsing context" to exclude auxiliary BCs. There seem to be ~50 uses of "top-level browsing context" in this spec alone. How many of those change their meaning substantially now?

Spot-checking, many of them now seem broken, e.g.

source

domenic · 2019-01-24T21:39:22Z

source

@@ -31164,7 +31184,8 @@ interface <dfn>HTMLObjectElement</dfn> : <span>HTMLElement</span> {

        <p>If the <code>object</code> element's <span>nested browsing context</span> is null, set
        the element's <span>nested browsing context</span> to a <span data-x="creating a new
-        browsing context">newly-created browsing context</span>.</p>
+        browsing context">newly-created browsing context</span> with the element's <span>node
+        document</span>.</p>


Same two-steps comment as above

This one is much harder as it's knee-deep in an existing algorithm. So you cannot really abstract object element out without rewriting the entire thing.

source

domenic · 2019-01-24T21:50:13Z

source

@@ -77343,6 +77363,46 @@ dictionary <dfn>DragEventInit</dfn> : <span>MouseEventInit</span> {
   <span>top-level browsing context</span>'s <code>WindowProxy</code> object.</p></li>
  </ol>

+  <p>The <dfn><code data-x="dom-opener">opener</code></dfn> attribute's getter must run these


I would put this last, if you want to keep it in the same section. top/parent are both about navigating the tree, whereas this is about crossing to a new tree, kinda. Feels different.

I put it in IDL order.

source

Needed for whatwg/html#4284.

annevk · 2019-01-25T15:21:15Z

web-platform-tests/wpt#15078 has tests.

domenic

LGTM.

Do you think maybe "create a new top-level browsing context" should get a new name, since it's immediately followed by "create a new auxiliary browsing context"? Or maybe just a clarifying note that "create a new auxiliary browsing context" is a more specialized way of creating a certain type of top-level BC.

I think in the future we could also have a "create a nested BC" algorithm which takes an element; that would be a nice cleanup.

domenic · 2019-01-28T20:40:40Z

source

-  browsing context</span>. <span w-nodev>When the browsing context is created, if the attribute
-  is present, the <span>browsing context name</span> must be set to the value of this attribute;
-  otherwise, the <span>browsing context name</span> must be set to the empty string.</span></p>
+  browsing context</span>.</p>


Very nice to move this.

domenic · 2019-01-28T20:41:23Z

source

@@ -76949,7 +76960,8 @@ dictionary <dfn>DragEventInit</dfn> : <span>MouseEventInit</span> {
  <p>A <span>browsing context</span> has an <dfn data-export="">opener browsing context</dfn>, which
  is null or a <span>browsing context</span>. It is initially null.</p>

-  <p>A <span>browsing context</span> can be <dfn id="disowned-its-opener">disowned</dfn>.</p>
+  <p>A <span>browsing context</span> can be <dfn id="disowned-its-opener">disowned</dfn>, which is a
+  boolean. It is initially false.</p>


Ehh, "can be disowned, which is a boolean" is not really grammatical. "Has a disowned boolean" would be better.

Needed for whatwg/html#4284.

…nested browsing context, a=testonly Automatic update from web-platform-tests HTML: window.open() can set opener of a nested browsing context Needed for whatwg/html#4284. -- wpt-commits: 5cf7f41336801449ce7499a5f41ccf6848650e9c wpt-pr: 15078

…nested browsing context, a=testonly Automatic update from web-platform-tests HTML: window.open() can set opener of a nested browsing context Needed for whatwg/html#4284. -- wpt-commits: 5cf7f41336801449ce7499a5f41ccf6848650e9c wpt-pr: 15078 UltraBlame original commit: 5eb72f3fdd6a5d8a9f9eb3015af3522b7196fcb2

annevk added 2 commits January 11, 2019 16:20

Refactor browsing context creation

e7311e5

Some initial work to make introducing browsing context groups easier.

nits

01cc8aa

mystor reviewed Jan 11, 2019

View reviewed changes

source Show resolved Hide resolved

source Outdated Show resolved Hide resolved

source Show resolved Hide resolved

source Show resolved Hide resolved

source Show resolved Hide resolved

source Show resolved Hide resolved

annevk added 2 commits January 15, 2019 13:45

more nits

913c61c

Make some slight corrections

c55e64d

annevk added 2 commits January 22, 2019 15:21

preserve status quo on name setting

7d85f7e

sandboxing affects non-auxiliary top-level too

b65178f

annevk requested a review from domenic January 22, 2019 16:55

annevk mentioned this pull request Jan 23, 2019

Cleanup how disowning works #4318

Closed

annevk added 4 commits January 24, 2019 14:16

incorperate #4318

532713e

sigh why is error information wrong

6c77a71

tada

a4ffb2f

window.open() usually sets opener browsing context because why not

28f5cf3

domenic reviewed Jan 24, 2019

View reviewed changes

annevk added a commit to web-platform-tests/wpt that referenced this pull request Jan 25, 2019

HTML: window.open() can set opener of a nested browsing context

a9f381b

Needed for whatwg/html#4284.

annevk mentioned this pull request Jan 25, 2019

HTML: window.open() can set opener of a nested browsing context web-platform-tests/wpt#15078

Merged

annevk added 2 commits January 25, 2019 17:21

address review feedback

1bfdc89

maybe i should build locally

5ac6db8

annevk mentioned this pull request Jan 25, 2019

Making postMessage() work for SharedArrayBuffer (Cross-Origin-Embedder-Policy) #4175

Closed

annevk requested a review from domenic January 28, 2019 17:09

domenic approved these changes Jan 28, 2019

View reviewed changes

annevk mentioned this pull request Jan 29, 2019

Restricting cross-origin WindowProxy access (Cross-Origin-Opener-Policy) #3740

Closed

clarify top-level bc creation and fix disowned boolean phrasing

736d30c

annevk merged commit a48895e into master Jan 29, 2019

annevk deleted the annevk/bc-creation-refactor branch January 29, 2019 13:52

annevk added a commit to web-platform-tests/wpt that referenced this pull request Jan 31, 2019

HTML: window.open() can set opener of a nested browsing context

5cf7f41

Needed for whatwg/html#4284.

hober mentioned this pull request Dec 10, 2019

HTML horizontal review: Browsing contexts w3ctag/design-reviews#452

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor browsing context creation #4284

Refactor browsing context creation #4284

annevk commented Jan 11, 2019 •

edited by pr-preview bot

Loading

mystor left a comment

mystor commented Jan 11, 2019

annevk commented Jan 22, 2019 •

edited

Loading

annevk commented Jan 23, 2019

annevk commented Jan 24, 2019 •

edited

Loading

annevk commented Jan 24, 2019 •

edited

Loading

domenic left a comment •

edited

Loading

domenic Jan 24, 2019

annevk Jan 25, 2019

domenic Jan 24, 2019

annevk Jan 25, 2019

annevk commented Jan 25, 2019

domenic left a comment

domenic Jan 28, 2019

domenic Jan 28, 2019

Refactor browsing context creation #4284

Refactor browsing context creation #4284

Conversation

annevk commented Jan 11, 2019 • edited by pr-preview bot Loading

mystor left a comment

Choose a reason for hiding this comment

mystor commented Jan 11, 2019

annevk commented Jan 22, 2019 • edited Loading

annevk commented Jan 23, 2019

annevk commented Jan 24, 2019 • edited Loading

annevk commented Jan 24, 2019 • edited Loading

domenic left a comment • edited Loading

Choose a reason for hiding this comment

domenic Jan 24, 2019

Choose a reason for hiding this comment

annevk Jan 25, 2019

Choose a reason for hiding this comment

domenic Jan 24, 2019

Choose a reason for hiding this comment

annevk Jan 25, 2019

Choose a reason for hiding this comment

annevk commented Jan 25, 2019

domenic left a comment

Choose a reason for hiding this comment

domenic Jan 28, 2019

Choose a reason for hiding this comment

domenic Jan 28, 2019

Choose a reason for hiding this comment

annevk commented Jan 11, 2019 •

edited by pr-preview bot

Loading

annevk commented Jan 22, 2019 •

edited

Loading

annevk commented Jan 24, 2019 •

edited

Loading

annevk commented Jan 24, 2019 •

edited

Loading

domenic left a comment •

edited

Loading