Skip to content
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

Give browsing contexts an explicit container #5091

Merged
merged 4 commits into from
Nov 22, 2019
Merged

Conversation

annevk
Copy link
Member

@annevk annevk commented Nov 15, 2019

This is the first step for #5020.

If #5089 is merged first I can remove all "nested through" usage and fix #4409 I think.


/browsers.html ( diff )
/parsing.html ( diff )
/webappapis.html ( diff )

@annevk annevk requested a review from domenic November 15, 2019 15:57
@annevk
Copy link
Member Author

annevk commented Nov 15, 2019

@shivanigithub this could also help with top-level origin. That is, through a document's browsing context's container document we can get to the actual parent of a document. If we keep doing that we hit the top-level document. We can then get the true top-level origin that's not tainted by bfcache.

In particular, if you have document A1, which nests A2, which nests A3. And then you navigate A2 to B. And then you navigate A1 to C.

If you go through A1's (or B's) parent documents using the above pattern you'll get A1. If you use top-level browsing context's active document you'll get C, which is bad.

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seem to be some issues around disconnected iframes, and the related switch from nested -> child in various places.

source Outdated Show resolved Hide resolved
source Show resolved Hide resolved
source Show resolved Hide resolved
source Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated
data-x="concept-document-bc">browsing contexts</span> are both <span data-x="child browsing
context">child browsing contexts</span> whose <span data-x="bc-container-document">container
documents</span> are <code>Document</code> <var>C</var>, then the order of <var>A</var> and
<var>B</var> in the list must match the <span>shadow-including tree order</span> of their
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's be sure to note in the commit message that this updates to account for shadow trees

source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
@domenic
Copy link
Member

domenic commented Nov 19, 2019

Oh, shoot. I think I was under the mistaken impression that the nested browsing context stays set, even after an iframe gets removed. So a lot of my comments are wrong, from that perspective, and this PR does not make normative changes in the way I thought it did.

I think the main thing would be to straighten out the child BC definition, and then I can take another look at this with the correct frame of mind.

@annevk
Copy link
Member Author

annevk commented Nov 20, 2019

I'm not really sure what you're asking for at this point.

@domenic
Copy link
Member

domenic commented Nov 20, 2019

I've resolved all the comments that were resulting from my confusion and added an idea on how to make it less confusing.

@annevk
Copy link
Member Author

annevk commented Nov 21, 2019

Proposed commit message:

Give browsing context an explicit container

This generally sets out the infrastructure for a more secure model for working with browsing contexts, as outlined in #5020.

* Adds browsing context's container
* Adds browsing context's container document
* Makes "update the rendering" account for browsing contexts in shadow trees
* Removes nested through; closes #4409.

I think #1336 is also fixed now?

(Note to self: rebase #5095 once this lands.)

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this fully takes care of everything in #1336. In particular there are still uses of "child browsing contexts" which are associated with a Document, not another BC.

I think these are the only two instances:

Discard all the child browsing contexts of document.

and

The term plugin refers to a user-agent defined set of content handlers used by the user agent that can take part in the user agent's rendering of a Document object, but that neither act as child browsing contexts of the Document nor introduce any Node objects to the Document's DOM.

(with the first being more problematic, since the second is somewhat informal).

@domenic domenic added the clarification Standard could be clearer label Nov 21, 2019
@annevk annevk added the topic: shadow Relates to shadow trees (as defined in DOM) label Nov 22, 2019
@annevk annevk merged commit 7b4964a into master Nov 22, 2019
@annevk annevk deleted the annevk/bc-container branch November 22, 2019 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification Standard could be clearer topic: browsing context topic: shadow Relates to shadow trees (as defined in DOM)
Development

Successfully merging this pull request may close these issues.

"nested through" usage is wrong
2 participants