Store the top-level and parent browsing contexts explicitly. #1543
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jgraham
requested review from
AutomatedTester,
shs96c and
andreastt
as code owners
|
Please note that appropriate WebDriver tests will be added/updated via https://bugzilla.mozilla.org/show_bug.cgi?id=1493108 once that PR has been approved. |
whimboo
reviewed
Sep 8, 2020
whimboo
reviewed
Sep 8, 2020
mjzffr
reviewed
Sep 8, 2020
Previously the parent and top-level browsing contexts were computed properties based on the current browsing context. This fails in the case where the frame containing the current browsing context is removed from the DOM tree since the browsing context is then discarded meaning that trying to compute its parents will fail. In practice we expect operations working with the top-level browsing context to continue working in that case (and indeed implementations are mostly/entirely not following the current spec), so this makes the top-level and parent browsing contexts explictly stored properties of the session that are updated at the point where we change browsing contexts.
This ensures that we only fail these operations if the browsing context we're trying to switch to is missing, not the current browsing context.
jgraham
force-pushed
the
set_browsing_context
branch
from
c8bafd6 to
43f6e94
Compare
whimboo
approved these changes
Sep 8, 2020
There was a problem hiding this comment.
From my perspective it looks fine, but it would be good to also get a review from @AutomatedTester and/or @shs96c.
AutomatedTester
approved these changes
Sep 14, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously the parent and top-level browsing contexts were computed
properties based on the current browsing context. This fails in the
case where the frame containing the current browsing context is
removed from the DOM tree since the browsing context is then discarded
meaning that trying to compute its parents will fail. In practice we
expect operations working with the top-level browsing context to
continue working in that case (and indeed implementations are
mostly/entirely not following the current spec), so this makes the
top-level and parent browsing contexts explictly stored properties of
the session that are updated at the point where we change browsing
contexts.
Preview | Diff