#53843 closed enhancement (fixed)
Remove adding of rel="noopener" to links with target="_blank"
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.7 | Priority: | normal |
Severity: | normal | Version: | |
Component: | General | Keywords: | has-patch has-unit-tests changes-requested |
Focuses: | Cc: |
Description
#43187 introduced adding of rel="noopener noreferrer"
to links with target="_blank"
as a security precaution. Later, in #49558 noreferrer
was removed as no longer needed.
Since then most browsers were updated to imply noopener
on links with target="_blank"
making #43187 and similar changes not relevant any more. See:
https://html.spec.whatwg.org/multipage/links.html#following-hyperlinks (specs),
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#browser_compatibility.
Adding this ticket now as a reminder to remove all noopener
when all newer browsers implement the specs.
Attachments (4)
Change History (35)
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
3 years ago
This ticket was mentioned in Slack in #core by presskopp. View the logs.
16 months ago
#6
@
14 months ago
- Keywords needs-patch added
- Milestone changed from Future Release to 6.5
The time has come, I think.
Quotes from https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#browser_compatibility :
Note: Setting target="_blank" on <a> elements implicitly provides the same rel behavior as setting rel="noopener" which does not set window.opener.
[I]n newer browser versions setting target="_blank" implicitly provides the same protection as setting rel="noopener".
All major browsers have shown this behavior for more than 2-3 years now, meaning that all browser that are still supported by WordPress support this (unless I have overlooked something).
We should consider deprecating wp_targeted_link_rel()
and related functions and stop adding it as a filter hook callback to a bunch of hooks.
noopener
in static <a>
tags in wp-admin can be removed, too. I don't think it's worth the risk to remove it from post content though.
[Brought to new attention by https://twitter.com/wesbos/status/1734619070127915021 .]
#7
@
14 months ago
- Keywords has-patch added; needs-patch removed
@TobiasBg
https://core.trac.wordpress.org/ticket/53843
Removed "rel="noopener" tag from all files.
#8
follow-up:
↓ 9
@
13 months ago
Hi there,
I'd say 53843.diff is a good first step before eventually deprecating wp_targeted_link_rel()
and related functions like wp_targeted_link_rel_callback()
.
I think this is ready to ship for 6.5, and we can target 6.6 to deprecate the functions in a follow-up ticket. What do you think @azaozz?
#9
in reply to:
↑ 8
@
13 months ago
- Keywords 2nd-opinion added
Replying to audrasjb:
I think this is ready to ship for 6.5, and we can target 6.6 to deprecate the functions...
Yea, seems that rel="noopener"
may not be needed any more. Thinking it would be good to confirm this 100%. As far as I see the current compatibility chart on MDN shows the Android WebView as not supporting this yet (this is the browser used by default on phones and also embedded in all kinds of devices like "smart" TVs, etc.).
Looking at the current patch it only removes the few rel="noopener"
that are hard-coded in wp-admin. If that's not needed any more thinking we should stop adding it to the user content and deprecate the functions, see wp_targeted_link_rel()
, wp_targeted_link_rel_callback()
, wp_init_targeted_link_rel_filters()
, wp_remove_targeted_link_rel_filters()
starting here: https://github.com/WordPress/wordpress-develop/blob/6.4/src/wp-includes/formatting.php#L3304.
#10
@
13 months ago
Hhm, that's odd. I just found https://chromestatus.com/feature/6140064063029248 which seems to imply that this was also shipped in Webview in version 88, just like other Chromium-based browsers...
#11
@
13 months ago
@azaozz @TobiasBg
I also found same on rel="noopener" here as well.https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/rel/noopener which support all browser including Webview android.
#12
follow-up:
↓ 13
@
13 months ago
@dhruval04: Yes, rel="noopener"
is supported by all modern browsers, but in this ticket we actually want to remove it :-) So the question is whether the note in the blue box is true for all modern browsers. There is some uncertainty regarding Android Webview here.
#13
in reply to:
↑ 12
@
13 months ago
Replying to TobiasBg:
Yep, the question here is whether Android Webview implies noopener
when the target="_blank"
attribute is present. Would be great if somebody can try to get more info on that, and which version(s) of Webview are affected.
#14
follow-up:
↓ 15
@
12 months ago
Yep, the question here is whether Android Webview implies noopener when the target="_blank" attribute is present. Would be great if somebody can try to get more info on that, and which version(s) of Webview are affected.
Well, caniuse.com still says no. And AFAIK Android WebView ≠ Chrome on Android, so https://chromestatus.com/feature/6140064063029248 does not affect Android WebView.
Question is: do we specifically support this "browser"?
#15
in reply to:
↑ 14
@
12 months ago
Replying to swissspidy:
so https://chromestatus.com/feature/6140064063029248 does not affect Android WebView.
But Webview is specifically mentioned, with "Shipping: 88"?
Can we somehow check the source code on this?
#16
@
12 months ago
But Webview is specifically mentioned, with "Shipping: 88"?
As I said, AFAIK the Android WebView (the built-in system widget) is different from "Chrome for Android, in a WebView".
The Chrome commit changed /chrome/browser
in the Chromium repo, but Android WebView is maintained in /android_webview
and was not modified.
#18
@
9 months ago
Hi @azaozz @audrasjb @swissspidy
As i see the current compatibility chart on MDN https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#browser_compatibility shows that Android WebView is now supporting noopener when the target="_blank" attribute is present.
#19
@
9 months ago
That MDN change was done in this pull request.
The decision was to trust https://chromestatus.com/feature/6140064063029248 which was also mentioned above.
The Chromium ticket seems to be https://issues.chromium.org/issues/40599708 with the commit at https://chromium.googlesource.com/chromium/src.git/+/a8a460cbc25a2474b48b844756503d749c584738 .
The question still is what "Webview" really is, as in @swissspidy's comment.
#20
@
9 months ago
- Keywords 2nd-opinion removed
I just asked about this internally and from what I understood it should work in Android WebView just as well.
So I think we're good to proceed here.
#21
@
9 months ago
- Keywords needs-refresh needs-patch added; has-patch removed
- Milestone changed from Future Release to 6.7
Thanks for checking on this, @swissspidy!
As the beta phase for the 6.6 release cycle has already started, this enhancement would probably need to go into 6.7, unless we want to get in the removal of noopener
from static <a>
tags now already. (53843.diff might need a refresh and double-check though, given that it's five months old.)
In 6.7, we could deprecate wp_targeted_link_rel()
and related functions and stop adding it as a filter hook callback to a bunch of hooks. This would still need a patch.
I'm setting milestone and keywords accordingly.
This ticket was mentioned in PR #6814 on WordPress/wordpress-develop by @DorZki.
8 months ago
#22
- Keywords has-patch has-unit-tests added; needs-refresh needs-patch removed
Trac ticket: https://core.trac.wordpress.org/ticket/53843
This ticket was mentioned in Slack in #core by dorzki. View the logs.
8 months ago
#25
@
8 months ago
Sorry to hop into this as mentioned in here https://core.trac.wordpress.org/ticket/61482 I think the code needs patching anyway as it does not only work on target="_blank"
If i add
<a target="_blank" href="https://myforum.com">Go to the Forum</a> <a target="_self" href="https://myforum.com">Go to the Forum</a> <a target="_parent" href="https://myforum.com">Go to the Forum</a> <a target="_top" href="https://myforum.com">Go to the Forum</a>
they all get the rel="noopener" added to the a-tag
<a target="_blank" href="https://myforum.com" rel="noopener">Go to the Forum</a> <a target="_self" href="https://myforum.com" rel="noopener">Go to the Forum</a> <a target="_parent" href="https://myforum.com" rel="noopener">Go to the Forum</a> <a target="_top" href="https://myforum.com" rel="noopener">Go to the Forum</a>
#26
@
6 months ago
- Keywords changes-requested added
The linked pull request is very close, I've left a couple of notes to relocation a retain functions as no-op calls. @DorZki I'm happy to push to your branch if you don't have bandwidth to work on the patch.
#27
@
6 months ago
Hi @peterwilsoncc,
Sorry for the late reply, been busy :)
I will go over the comments and fix whatever needs to be fixed :)
This ticket was mentioned in Slack in #core by stoyangeorgiev. View the logs.
5 months ago
#29
@
5 months ago
- Owner set to peterwilsoncc
- Resolution set to fixed
- Status changed from new to closed
In 59120:
In case anyone wonders about the comment author links on the Comments admin screen (r52007), those would keep the
rel
attribute because they do not open in a new tab automatically.