Conversation
|
(This does not fix all issues identified in #4198, but it does introduce the necessary infrastructure to fix them and should be strictly better than the status quo. In particular, we will still need to define agent allocation. I'd suggest we file a follow-up for that.) |
domenic
left a comment
domenic
left a comment
There was a problem hiding this comment.
I guess all the groundwork has made this a nicely self-contained change; cool. All my feedback seems to be "editorial", but I am fairly concerned about the clarity implications of the remove/discard distinction and the potential null-ness of group, so looking forward to your answers there.
|
This is ready for another/final look. This should also remove "and the user agent itself has a strong reference to its top-level browsing contexts" as that's now more clearly defined, but waiting for #4368 to land first to make rebasing easier and less likely to introduce mistakes. |
domenic
left a comment
domenic
left a comment
There was a problem hiding this comment.
Will give this the approve bit, because I don't want to block over another weekend. But, I'd like to do one more review if possible and our timelines line up, both with regard to my latest question, and to allow you to remove the text you mention now that #4368 has landed.
| <span>browsing context</span> is <span data-x="a browsing context is | ||
| discarded">discarded</span>. <a href="https://github.com/whatwg/html/issues/4361">Issue | ||
| #4361</a> sketches a setup that could improve this situation.</p> | ||
| </li> |
There was a problem hiding this comment.
Is there a reason we don't just return false if either group is null?
There was a problem hiding this comment.
Well, the responsible browsing context might also be null, right? But in general, they need to remain in the group they were in when they were created. So it seems better to explicitly not deal with something becoming null until we've addressed putting them in the appropriate groups better at which point it'll all follow from first principles.
The grouping concepts unit of related browsing contexts and unit of similar-origin browsing contexts were not accurate, due to browsing contexts being able to hold a sequence of (potentially cross-site) documents. Fixes #4198.
domenic
left a comment
domenic
left a comment
There was a problem hiding this comment.
LGTM again. I'm content with the XXX because as we discovered in https://freenode.logbot.info/whatwg/20190222#c2016315, discarded BCs are fairly complicated, and the right answer is not obvious. I look forward to fixing that.
* "Unit of related browsing contexts" was removed from the HTML spec in 2019 (whatwg/html#4350) * "Report task end time" moved to LONG-ANIMATION-FRAMES and has been renamed "Record task end time"
* "Unit of related browsing contexts" was removed from the HTML spec in 2019 (whatwg/html#4350) * "Report task end time" moved to LONG-ANIMATION-FRAMES and has been renamed "Record task end time" Closes: #108
2 participants
The grouping concepts unit of related browsing contexts and unit of similar-origin browsing contexts were not accurate, due to browsing contexts being able to hold a sequence of (potentially cross-site) documents.
Fixes #4198.
/browsers.html ( diff )
/infrastructure.html ( diff )
/webappapis.html ( diff )
/window-object.html ( diff )