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

Should target=_blank and window.open('...', '_blank') be case sensitive? #2443

Closed
phistuck opened this issue Mar 17, 2017 · 15 comments
Closed
Assignees

Comments

@phistuck
Copy link

phistuck commented Mar 17, 2017

It is case insensitive only in Firefox (since at most version 3, if not since forever).
JSFiddle -
https://jsfiddle.net/8v7ots9x/

Is this web compatible?
It is always good to have case sensitivity, anyway. ;)

The new tests triggered this -
web-platform-tests/wpt#5145

@annevk
Copy link
Member

annevk commented Mar 20, 2017

@bzbarsky
Copy link
Contributor

bzbarsky commented Mar 20, 2017

I guess we could do that. Gecko has been case-insensitive here since at least 2000; I expect Netscape was too. But it does look at first glance like everyone else (including IE11) is case-sensitive.

I just checked, and a lot of Firefox addons use "_BLANK" as either a target or in selectors like zone-right a[target="_BLANK"] > img. This last is happening in addons like uBlock. So sites do in fact use non-lowercase spellings here. An example (still based on this uBlock list) is https://www.kuopionpursiseura.fi/ which uses it for the ad-thing right below the site menu on the right.

The catch, of course, is that a non-lowercase spelling will seem to work just like the lowercase spelling in a simple test in all browsers. It's when you click a second link (or do window.open, or whatever) with that the behavior difference appears.

It might be worth looking at uses of _BLANK in httparchive or something and seeing what behavior sites expect at those use points...

@zcorpan
Copy link
Member

zcorpan commented Mar 20, 2017

SELECT * FROM (
SELECT page, url, REGEXP_EXTRACT(body, r'<[aA](?:[rR][eE][aA])?[^>]+\s[tT][aA][rR][gG][eE][tT]\s*=\s*["\']?_([bB][lL][aA][nN][kK]|[sS][eE][lL][fF]|[pP][aA][rR][eE][nN][tT]|[tT][oO][pP])(?:["\'>]|\s)') AS match
FROM [httparchive:har.2017_03_01_android_requests_bodies]
) WHERE match != "null"
AND match != "blank"
AND match != "self"
AND match != "parent"
AND match != "top"

1424 rows. https://gist.github.com/zcorpan/871ce08164fb5078e434cb1b9b88dc4a

I think the _SELF/_PARENT/_TOP ones are probably more interesting than _BLANK, since they give different results on the first click of such a link. I have not yet analyzed these.

@phistuck
Copy link
Author

phistuck commented Mar 20, 2017

I tried the more simplified target="_BLANK" - 2623 results -
https://bigquery.cloud.google.com:443/savedquery/558355517328:6f9fd9a369f8483a8354859a439c534d

@zcorpan
Copy link
Member

zcorpan commented Mar 20, 2017

http://www.aquariumline.com/	http://www.aquariumline.com/catalog/news_with_scroll.php	Parent

This seems like it indeed intended _parent and so works better in Firefox.

http://www.bazarmajazi.com/	http://www.bazarmajazi.com/	SELF

The tabbed pages at the top, I would say works better in Firefox.

http://www.exler.ru/	https://www.exler.ru/	TOP

This is the Facebook badge, though seems OK to open in a new tab usability-wise for this page... but would be confusing if a page has several different _TOP links and they keep overwriting each other.

http://www.shuud.net/	http://shuud.club/	SELF

Top-left logo, seems like it indeed should be a _self link.

http://www.ccckmit.wikidot.com/	http://ccckmit.wdfiles.com/local--html/main/1e395d9d1baa7e9ed0e8b7a9ac0fc958fb6491ed-5165331601518671118/ccckmit.wikidot.com/	TOP

Also Facebook badge.

http://www.emegteichuud.mn/	http://www.emegteichuud.mn/banner.shtml?name=card1	SELF

This page has several _SELF links, which seem to be intended to be _self.

http://www.cnbcarabia.com/	http://zagtrader.com/external/cnbcarabiadynamic/pricebar2.php?marketId=6	PARENT

Intends _parent.

Well, that is probably enough to give a good enough insight into what website expectations looks like. As far as I can tell they expect case-insensitive and the user experience is a bit better with case-insensitive.

@zcorpan
Copy link
Member

zcorpan commented Mar 20, 2017

@bzbarsky

I just checked, and a lot of Firefox addons use "_BLANK" as either a targer or in selectors like zone-right a[target="_BLANK"] > img.

target is in the list of attributes that match case-insensitively with selectors:
https://html.spec.whatwg.org/#case-sensitivity-of-selectors

@bzbarsky
Copy link
Contributor

target is in the list of attributes that match case-insensitively with selectors:

OK, but the reason the addon had it like that is that the site involved has "_BLANK" as the target.

@zcorpan
Copy link
Member

zcorpan commented Mar 24, 2017

So given the analysis above I think these should be case-insensitive like in Gecko.

@cdumez @tkent-google WDYT?

@cdumez
Copy link

cdumez commented Mar 24, 2017

I agree with zcorpan, I think we should make these case-insensitive given the analysis. It is also safer for us to align with Gecko than the other way around here.

@tkent-google
Copy link
Contributor

+1 for case-insensitive.

@zcorpan zcorpan closed this as completed Mar 27, 2017
@phistuck
Copy link
Author

phistuck commented Mar 27, 2017

What about only making it case insensitive in quirks mode? At least some of those examples are rendered in quirks mode, as far as I can see.

@domenic
Copy link
Member

domenic commented Mar 27, 2017

There's no reason to add more quirks to the platform.

@phistuck
Copy link
Author

If it allows more customizability, I am not sure I agree. But I certainly do not feel strongly about it anyway, it was just a suggestion.

hubot pushed a commit to WebKit/WebKit-http that referenced this issue Apr 5, 2017
…-insensitive

https://bugs.webkit.org/show_bug.cgi?id=169747

Reviewed by Alex Christensen.

LayoutTests/imported/w3c:

Import test coverage from upstream web-platform-tests.

* resources/import-expectations.json:
* web-platform-tests/html/browsers/windows/browsing-context-names/001.html:
* web-platform-tests/html/browsers/windows/browsing-context-names/002.html:
* web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-_blank-expected.txt: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-_blank.html: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-choose-existing.html:
* web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-choose-parent-001-expected.txt: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-choose-parent-001.html: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-choose-parent-002-expected.txt: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-choose-parent-002.html: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-choose-parent-003-expected.txt: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-choose-parent-003.html: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-choose-parent-004-expected.txt: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-choose-parent-004.html: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-choose-self-1.html:
* web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-choose-self-2.html:
* web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-default-name-expected.txt:
* web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-default-name.html:
* web-platform-tests/html/browsers/windows/browsing-context-names/existing.html:
* web-platform-tests/html/browsers/windows/browsing-context-names/resources/parent-iframe-1.html: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/resources/parent-iframe-2.html: Copied from LayoutTests/imported/w3c/web-platform-tests/html/browsers/windows/browsing-context-names/existing.html.
* web-platform-tests/html/browsers/windows/browsing-context-names/resources/parent-iframe-3.html: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/resources/parent-iframe-insensitive-1.html: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/resources/parent-iframe-insensitive-2.html: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/resources/parent-top-nested.html: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/resources/parent-top-replace.html: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/resources/parent-top.html: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/resources/post-to-opener.html: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/resources/post-to-top-or-close.html: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/resources/post-to-top.html: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/resources/w3c-import.log: Copied from LayoutTests/imported/w3c/web-platform-tests/html/browsers/windows/browsing-context-names/w3c-import.log.
* web-platform-tests/html/browsers/windows/browsing-context-names/self1.html:
* web-platform-tests/html/browsers/windows/browsing-context-names/self2.html:
* web-platform-tests/html/browsers/windows/browsing-context-names/w3c-import.log:

Source/WebCore:

_blank / _self / _parent / _top browsing context names should be case-insensitive
as per the HTML specification:
- https://html.spec.whatwg.org/#browsing-context-names

This aligns our behavior with Firefox as well. See discussion at:
- whatwg/html#2443

Tests: imported/w3c/web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-_blank.html
       imported/w3c/web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-choose-parent-001.html
       imported/w3c/web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-choose-parent-002.html
       imported/w3c/web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-choose-parent-003.html
       imported/w3c/web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-choose-parent-004.html

* loader/FrameLoader.cpp:
(WebCore::FrameLoader::continueLoadAfterNewWindowPolicy):
(WebCore::createWindow):
* page/DOMWindow.cpp:
(WebCore::DOMWindow::open):
* page/FrameTree.cpp:
(WebCore::FrameTree::uniqueChildName):
(WebCore::FrameTree::find):


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@214944 268f45cc-cd09-0410-ab3c-d52691b4dbfc
@annevk
Copy link
Member

annevk commented May 3, 2017

This also requires changes to "The rules for choosing a browsing context given a browsing context name". I'm hoping to have a PR ready today.

@annevk annevk reopened this May 3, 2017
annevk added a commit that referenced this issue May 3, 2017
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
#2616. Formal testing is not possible and tracked by
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.
annevk added a commit that referenced this issue May 3, 2017
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.
annevk added a commit that referenced this issue May 8, 2017
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.
annevk added a commit that referenced this issue May 9, 2017
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 #2616. 
  Formal testing is not possible and tracked by
  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: #1440.

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.
inikulin pushed a commit to HTMLParseErrorWG/html that referenced this issue May 9, 2017
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 whatwg#2616. 
  Formal testing is not possible and tracked by
  web-platform-tests/wpt#3867.
* Make matching for special names ASCII case-insensitive. Fixes whatwg#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: whatwg#1440.

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.
inikulin pushed a commit to HTMLParseErrorWG/html that referenced this issue May 9, 2017
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 whatwg#2616. 
  Formal testing is not possible and tracked by
  web-platform-tests/wpt#3867.
* Make matching for special names ASCII case-insensitive. Fixes whatwg#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: whatwg#1440.

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.
alice pushed a commit to alice/html that referenced this issue Jan 8, 2019
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 whatwg#2616. 
  Formal testing is not possible and tracked by
  web-platform-tests/wpt#3867.
* Make matching for special names ASCII case-insensitive. Fixes whatwg#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: whatwg#1440.

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.
ryanhaddad pushed a commit to WebKit/WebKit that referenced this issue Dec 22, 2020
…-insensitive

https://bugs.webkit.org/show_bug.cgi?id=169747

Reviewed by Alex Christensen.

LayoutTests/imported/w3c:

Import test coverage from upstream web-platform-tests.

* resources/import-expectations.json:
* web-platform-tests/html/browsers/windows/browsing-context-names/001.html:
* web-platform-tests/html/browsers/windows/browsing-context-names/002.html:
* web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-_blank-expected.txt: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-_blank.html: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-choose-existing.html:
* web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-choose-parent-001-expected.txt: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-choose-parent-001.html: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-choose-parent-002-expected.txt: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-choose-parent-002.html: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-choose-parent-003-expected.txt: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-choose-parent-003.html: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-choose-parent-004-expected.txt: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-choose-parent-004.html: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-choose-self-1.html:
* web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-choose-self-2.html:
* web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-default-name-expected.txt:
* web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-default-name.html:
* web-platform-tests/html/browsers/windows/browsing-context-names/existing.html:
* web-platform-tests/html/browsers/windows/browsing-context-names/resources/parent-iframe-1.html: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/resources/parent-iframe-2.html: Copied from LayoutTests/imported/w3c/web-platform-tests/html/browsers/windows/browsing-context-names/existing.html.
* web-platform-tests/html/browsers/windows/browsing-context-names/resources/parent-iframe-3.html: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/resources/parent-iframe-insensitive-1.html: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/resources/parent-iframe-insensitive-2.html: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/resources/parent-top-nested.html: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/resources/parent-top-replace.html: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/resources/parent-top.html: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/resources/post-to-opener.html: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/resources/post-to-top-or-close.html: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/resources/post-to-top.html: Added.
* web-platform-tests/html/browsers/windows/browsing-context-names/resources/w3c-import.log: Copied from LayoutTests/imported/w3c/web-platform-tests/html/browsers/windows/browsing-context-names/w3c-import.log.
* web-platform-tests/html/browsers/windows/browsing-context-names/self1.html:
* web-platform-tests/html/browsers/windows/browsing-context-names/self2.html:
* web-platform-tests/html/browsers/windows/browsing-context-names/w3c-import.log:

Source/WebCore:

_blank / _self / _parent / _top browsing context names should be case-insensitive
as per the HTML specification:
- https://html.spec.whatwg.org/#browsing-context-names

This aligns our behavior with Firefox as well. See discussion at:
- whatwg/html#2443

Tests: imported/w3c/web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-_blank.html
       imported/w3c/web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-choose-parent-001.html
       imported/w3c/web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-choose-parent-002.html
       imported/w3c/web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-choose-parent-003.html
       imported/w3c/web-platform-tests/html/browsers/windows/browsing-context-names/browsing-context-choose-parent-004.html

* loader/FrameLoader.cpp:
(WebCore::FrameLoader::continueLoadAfterNewWindowPolicy):
(WebCore::createWindow):
* page/DOMWindow.cpp:
(WebCore::DOMWindow::open):
* page/FrameTree.cpp:
(WebCore::FrameTree::uniqueChildName):
(WebCore::FrameTree::find):


Canonical link: https://commits.webkit.org/187426@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@214944 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

7 participants