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

HTML: opener and discarded auxiliary browsing context #15518

Merged
merged 2 commits into from
Mar 7, 2019

Conversation

annevk
Copy link
Member

@annevk annevk commented Feb 22, 2019

assert_true(openerDescImmaterial.writable);
assert_true(openerDescImmaterial.enumerable);
assert_true(openerDescImmaterial.configurable);
testOpener(frameW, window, () => frame.remove());

t.done();
Copy link
Member

Choose a reason for hiding this comment

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

Can use t.step_func_done for this one too, right?

Copy link
Member Author

@annevk annevk Feb 23, 2019

Choose a reason for hiding this comment

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

No, due to different load event behavior (there's a comment to that effect).

@annevk
Copy link
Member Author

annevk commented Feb 23, 2019

This probably should not be merged until whatwg/html#4379 (comment) is resolved somehow.

Copy link
Contributor

@bzbarsky bzbarsky left a comment

Choose a reason for hiding this comment

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

This seems fine, modulo sorting out the close() behavior. I commented over in whatwg/html#4379 about what's going on there in Firefox...

assert_equals(otherW.opener, null, "opener after removal");
assert_equals(openerGet(), null, "opener after removal via directly invoking the getter");

otherW.opener = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

It's a little weird to do this after the opener is already asserted to be null, but I guess we were already testing that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Mainly to ensure the setter doesn't do anything weird. I could remove this if desired...

@annevk annevk requested review from bzbarsky and domenic and removed request for bzbarsky March 4, 2019 13:57
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.

New version LGTM again. It assumes that the spec will queue a task for discarding, which per IRC conversations sounds like it's a good assumption.

@annevk annevk merged commit 3df0716 into master Mar 7, 2019
@annevk annevk deleted the annevk/opener-and-discarded-abc branch March 7, 2019 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants