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

Add rel="noopener noreferrer" to external links #3175

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tvdeyen
Copy link
Member

@tvdeyen tvdeyen commented Jan 25, 2025

What is this pull request for?

It is more secure to add rel="noopener noreferrer" to links with target="_blank".

It is more secure to add `rel="noopener noreferrer"` to
links with `target="_blank"`.
@tvdeyen tvdeyen requested a review from a team as a code owner January 25, 2025 18:52
@tvdeyen tvdeyen added this to the 7.4 milestone Jan 25, 2025
@tvdeyen tvdeyen added backport-to-7.1-stable Needs to be backported to 7.1-stable backport-to-7.2-stable Needs to be backported to 7.2-stable backport-to-7.3-stable Needs to be backported to 7.3-stable backport-to-7.4-stable Needs to be backported to 7.4-stable labels Jan 25, 2025
Copy link

codecov bot commented Jan 25, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.59%. Comparing base (45de21c) to head (c5d7539).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3175   +/-   ##
=======================================
  Coverage   96.58%   96.59%           
=======================================
  Files         236      237    +1     
  Lines        6360     6371   +11     
=======================================
+ Hits         6143     6154   +11     
  Misses        217      217           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

target: (ingredient.link_target == "blank") ? "_blank" : nil,
data: {link_target: ingredient.link_target.presence}
rel: link_rel_value(target),
target:
Copy link
Contributor

Choose a reason for hiding this comment

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

When a link's target attribute is set to blank (without an underscore), clicking the link twice will open it twice in the same window. However, when the target is set to _blank (with an underscore), clicking twice will open the link in two separate new windows.
The _blank behavior is more predictable and is what users typically expect. Looking at line 51 of the code, it appears Alchemy is saving the value as "blank" (without the underscore), though I'm not certain if this is the intended behavior.

REL_VALUE = "noopener noreferrer"

def link_rel_value(target)
if target == "_blank"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if target == "_blank"
if target == "_blank" || target == "blank"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-7.1-stable Needs to be backported to 7.1-stable backport-to-7.2-stable Needs to be backported to 7.2-stable backport-to-7.3-stable Needs to be backported to 7.3-stable backport-to-7.4-stable Needs to be backported to 7.4-stable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants