-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Revamp the rules for choosing a browsing context #2618
Conversation
This builds on #2613 which will need to land first. |
c5b0508
to
58aaf16
Compare
|
||
<p class="example">For example, suppose there is a user agent that supports control-clicking a | ||
link to open it in a new tab. If a user clicks in that user agent on an element whose <code | ||
<p class="example">If there is a user agent that supports control-clicking a link to open it in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you change the wording here? The first sentence now doesn't make sense to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the text before it got deduplicated with the choosing a browsing context algorithm. Happy to take suggestions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pushed a suggested fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that looks fine. Would probably put "element" after iframe
.
Given that landing #2613 is now awaiting Chrome results, I think I'll try to rebase this fix on master so we can make some progress in the intervening two months. |
This makes several changes: * Stop throwing an exception in <a> and <area> activation behavior as a result of popup blocking as it doesn’t match implementations. Fixes web-platform-tests/wpt#3867. * Make matching for special names ASCII case-insensitive. Fixes #2443. * Centralize all user-configurable behavior instead of having it duplicated in various ways in all the caller algorithms. * Call out a known issue with browsing context name matching: #2126. It also modernizes the writing style and makes variables and what is returned much more explicit, including no longer relying on some out-of-band channel for communicating whether a new browsing context got created.
12aca79
to
0bdbb7c
Compare
@zcorpan I rebased this on top of master, leaving out the change to centralize processing for |
Thanks, I'll make one more improvement. |
Done, I'll land this tomorrow (and fix the commit message slightly to account for that last change) if there are no further issues reported by anyone. |
This makes several changes:
<a>
and<area>
activation behavior as aresult of popup blocking as it doesn’t match implementations. Fixes
<a> and <area> popup blocker exception is broken #2616. Formal testing is not possible and tracked by
Add ability to test with popup blocker enabled web-platform-tests/wpt#3867.
duplicated in various ways in all the caller algorithms.
It also modernizes the writing style and makes variables and what is
returned much more explicit, including no longer relying on some
out-of-band channel for communicating whether a new browsing context
got created.