![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png){kind=logo}
Opened 7 years ago
Closed 7 years ago
#46421 closed defect (bug) (fixed)
rel="noopener noreferrer" should be added to links with target="_blank" which are in text and HTML widgets
| Reported by: | mark-k | Owned by: | audrasjb |
|---|---|---|---|
| Priority: | normal | Milestone: | 5.2 |
| Component: | Widgets | Version: | 5.1 |
| Severity: | normal | Keywords: | has-patch has-unit-tests commit |
| Cc: | Focuses: |
Description
From inspecting the code related to #43187 it seems like links in all of the content that should open in new window get the rel="noopener noreferrer" attribute, but it leaves the text and HTML widgets with no such treatment. IMHO it needs to happen in that context as well.
Attachments (2)
Change History (14)
#3
@
7 years ago
- Keywords has-patch 2nd-opinion added
46421.diff adds rel noopener/noreferrer attribute to both Text and Custom HTML widgets.
I propose to use the existing function wp_targeted_link_rel() which is located and documented in wp-inludes/formatting.php.
The patch works fine on my side for both Text and Custom HTML widgets.
#4
@
7 years ago
@audrasjb, 46421.diff patch working fine for me in both the widgets.
Below HTML code is tested in both the widgets.
Code:
<a href="https://wordpress.org/" target="_blank">WordPress</a> <a href="https://wordpress.org/" target="_top">WordPress</a> <a href="https://wordpress.org/" target="_self">WordPress</a> <a href="https://wordpress.org/" target="" rel="noopener">WordPress</a> <a href="https://wordpress.org/" target="" rel="noreferrer">WordPress</a> <a href="https://wordpress.org/" target="" rel="noreferrer noopener">WordPress</a> <a href="https://wordpress.org/" rel="noreferrer noopener">WordPress</a>
Result:
<a href="https://wordpress.org/" target="_blank" rel="noopener noreferrer">WordPress</a> <a href="https://wordpress.org/" target="_top" rel="noopener noreferrer">WordPress</a> <a href="https://wordpress.org/" target="_self" rel="noopener noreferrer">WordPress</a> <a href="https://wordpress.org/" target="" rel="noopener noreferrer">WordPress</a> <a href="https://wordpress.org/" target="" rel="noreferrer noopener">WordPress</a> <a href="https://wordpress.org/" target="" rel="noreferrer noopener">WordPress</a> <a href="https://wordpress.org/" rel="noreferrer noopener">WordPress</a>
Can we change docs for function wp_targeted_link_rel() in both widget?
// Adds rel noreferrer and noopener to all HTML A elements that have a target.
something
// Adds rel noreferrer and noopener to all HTML A elements that have a target in addition to any existing values.
#5
@
7 years ago
- Keywords has-unit-tests added
46421-2.diff adds the following test cases:
- Ensure that
rel="noopener noreferrer"is added to links with a target in the output of the Text widget. - Ensure these relationships are not added to links without a target in the output of the Text widget.
- Ensure that
rel="noopener noreferrer"is added to links with a target in the output of the Custom HTML widget. - Ensure these relationships are not added to links without a target in the output of the Custom HTML widget.
@mukesh27 I guess you mean to make it clear that adding these relationships, is not causing any duplication.
I modified your suggestion to:
// Adds noreferrer and noopener relationships, without duplicating values, to all HTML A elements that have a target.
in 46421-2.diff.
What do you think about this modified inline comment version?
#6
@
7 years ago
Another suggestion of the inline comment could be:
// Adds noreferrer and noopener relationships, without duplicating values, to all anchor tags that have a target attribute.
#7
@
7 years ago
@birgire patch 46421-2.diff inline comment suggestion look good for me.
@audrasjb can you please review patch and inline comment text.
#8
@
7 years ago
- Keywords commit added; 2nd-opinion removed
Tested on my side and 46421-2.diff looks great. Thanks for the patch and the unit-tests @birgire !
Adding commit keyword.
#10
in reply to: ↑ 9
@
7 years ago
Replying to afercia:
One thing to discuss is the actual usefulness of
noreferrer:
As mentionned on #43280 I think both of the rel attributes should be added, in order to keep some consistency with https://developer.wordpress.org/reference/functions/wp_targeted_link_rel/
If we want to remove noreferrer, I believe we should remove it from that function first ;)
Add rel noopener/noreferrer to both Widgets Text and Custom HTML