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

Always use rel="noreferrer noopener" linking to another site with target="_blank" #1125

Closed
xchs opened this issue Oct 3, 2017 · 11 comments
Closed
Assignees
Labels
Milestone

Comments

@xchs
Copy link
Contributor

xchs commented Oct 3, 2017

Should we (automatically) add an rel="noreferrer noopener" attribute for all target="_blank" links?

See:

@leofeyer leofeyer added this to the 4.5.0 milestone Oct 4, 2017
@leofeyer
Copy link
Member

leofeyer commented Oct 4, 2017

I guess we should. I have noticed that TinyMCE already does.

@leofeyer
Copy link
Member

I have added rel="noopener" where possible in 23820d7. However, this is not comprehensive, because there might be existing rel attributes such as rel="nofollow", which we have to merge accordingly. This, however, is a much bigger task.

@leofeyer leofeyer removed this from the 4.5.0 milestone Nov 20, 2017
@Xendiadyon
Copy link

However, there is a problem:
If TinyMCE sets "noopener noreferrer" for external Links, they cannot be tracked in Analytics.
If you have a Project page and would like to track from which pages the traffic comes, you will not be able to see origin pages with external "noreferrer" links.

Best way would be to decide if we would like to use noreferrer links or not.

@may17
Copy link
Contributor

may17 commented Mar 26, 2018

window.open should be fixed too. See this blog post a couple of days ago: https://pineco.de/security-and-performance-benefit-from-the-rel-noopener/

Related fix from the post is:
var newWindow = window.open(); newWindow.opener = null;

@Xendiadyon did you got some more details about that problem? Google recommends that every link should had noopener als rel attr. See here https://developers.google.com/web/tools/lighthouse/audits/noopener

@Xendiadyon
Copy link

I would recommend as well that we always use rel="noopener".

But we should not use the "noreferrer". Because otherwise, tracking pages would not be able to read the referring page. This means, Google Analytics should know where the user is from. Usually, this is highly desired.
See: https://www.3whitehats.com/knowledge/wordpress-noreferrer-google-analytics/
https://www.techwyse.com/blog/search-engine-optimization/what-you-need-to-know-about-rel-noreferrer-attribute/

In detail, we had two pages, the company page setting links for a specific action page, which was active only during a couple of weeks. We wanted to track how many users came from the company page to the action page - and through which pathways they made it to the action page.
Unfortunately, all referral information was blocked and thus we had a lot of "direct" links but no information whatsoever where the users came from.

So: rel="noopener" should always be set. But rel="noreferrer" should only be used if explicitely decided.

(Hint: We are using noreferrer, as some Firefox versions do not interpret correctly the noopener Tag)
https://www.jitbit.com/alexblog/256-targetblank---the-most-underestimated-vulnerability-ever/
https://mathiasbynens.github.io/rel-noopener/ (already posted above)

@leofeyer
Copy link
Member

Because otherwise, tracking pages would not be able to read the referring page. This means, Google Analytics should know where the user is from. Usually, this is highly desired.

By whom? I do not desire that Google knows where I am coming from or where I am going!

@Xendiadyon
Copy link

In user perspective, I agree. In marketing perspective, this information is essential :)

@leofeyer
Copy link
Member

Hopefully the GDPR will help to reduce these "essential marketing practices" in the future.

@ausi
Copy link
Member

ausi commented Mar 29, 2018

rel="noopener" is not supported by IE/Edge so we have to use rel="noreferrer noopener" to be secure.

If you need the referrer for analytics, don’t use target="_blank" links.

@Xendiadyon
Copy link

If this works, this would be a great specific workaround!

@leofeyer
Copy link
Member

Fixed in f9d13e2.

christian-kolb pushed a commit to christian-kolb/contao that referenced this issue Oct 11, 2018
leofeyer added a commit that referenced this issue Dec 23, 2019
Description
-----------

See https://symfony.com/blog/new-in-symfony-4-4-simpler-event-listeners

Commits
-------

a31e80ba Simplify the event registration
cf8a71a6 Use an __invoke() method in the MergeHttpHeadersListener class
4a95e51b Specify the method name instead of the event if a listener does not have an __invoke() method
ea53f5a2 Fix the unit tests
leofeyer pushed a commit that referenced this issue Dec 23, 2019
Description
-----------

Changed method signature in #1125.

Commits
-------

4387e96d Renamed method after changing its signature
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants