1271247 - Remove usage of target="_blank" and use rel="noopener" in cases where it is justifiable.

Status: RESOLVED FIXED

(www.mozilla.org :: Pages & Content, defect)

Description

[Schalk Neethling [:espressive]]

We should be cautious when using target="_blank" across the site as this can potentially cause a security vulnerability.

Would suggest that we use it only where absolutely required, and when used, we need to include the following attribute on the anchor tag:

`rel="noopener noreferrer"`

See https://mathiasbynens.github.io/rel-noopener/ for reference

Comment 1

[Alex Gibson [:agibson]]

+1 for doing this - the potential vulnerability is now another reason why forcing links to open in new tabs should generally be considered bad practice, past the usual UX and a11y concerns.

Here's a list of our current occurrences:

https://github.com/mozilla/bedrock/search?utf8=%E2%9C%93&q=target%3D%22_blank%22

Comment 2

[Mike Alexis [:malexis]]

Is this a "good first bug" candidate?

Comment 3

[Alex Gibson [:agibson]]

(In reply to Mike Alexis [:malexis] from comment #2)
> Is this a "good first bug" candidate?

Yep I would say most definitely

Comment 4

[Mainak Roy Chowdhury [:rcmainak]]

Hello, I would like to contribute. I noticed that rel="noopener noreferrer" does not work in Edge, IE & Safari. Not sure how to solve this problem. Also please specify the cases where it is absolutely required to use target="_blank".

I have a silly question, the html files are written in Jinja right?

It would be my first contribution in terms of contributing to actual codebase so please guide me through the process.

Comment 5

[Alex Gibson [:agibson]]

(In reply to Mainak Roy Chowdhury from comment #4)
> Hello, I would like to contribute. I noticed that rel="noopener noreferrer"
> does not work in Edge, IE & Safari. Not sure how to solve this problem. Also
> please specify the cases where it is absolutely required to use
> target="_blank".
> 
> I have a silly question, the html files are written in Jinja right?
> 
> It would be my first contribution in terms of contributing to actual
> codebase so please guide me through the process.

Thanks for showing interest in this bug, Mainak!

I would not worry about rel="noopener noreferrer" not working in Edge/IE/Safari. We can only add protection for browsers that support it. This is all the more reason to try and remove target="_blank" where possible. There are probably only a few pages where it's strictly necessary (like the /contribute task pages). What I would suggest is just to use your own judgement as you go through the pages. We can discuss your decisions once you open a pull request and make any necessary changes from there based on discussion.

You can find the bedrock docs here to help you get set up: http://bedrock.readthedocs.io/en/latest/

If you have any trouble or any questions, best bet is to drop into #www in IRC and someone can assist you.

Assigning this bug to you, thanks!

Comment 6

[Mainak Roy Chowdhury [:rcmainak]]

Great. I'll start working on it. Setting up everything will take some time. See you on IRC.

Comment 7

[Alex Gibson [:agibson]]

Hi Mainak,

Just checking in here, are you still working on this bug?

Comment 8

[Mainak Roy Chowdhury [:rcmainak]]

Yes, I'm still working on it and keeps on getting weird errors lol. I'm planning to end this work soon.

Comment 9

[Eric Renaud [:erenaud]]

Hi Mainak,

Just checking on progress here.  Could you please provide a status update and indicate if you need any assistance?

Many thanks,

Eric

Comment 10

[Mainak Roy Chowdhury [:rcmainak]]

I created a pull request today. There are several files which are out of my technical skills. I've added the details in the description field of the PR. Sorry for slacking. I appreciate everyone's patients. I tried make my PR perfect from the beginning, which made me redo the whole thing several times. Finally decided to not make it perfect as i don't have much experience with Django. I'm looking forward for the code review. *fingers crossed* 

Oh if you're wondering here is the PR https://github.com/mozilla/bedrock/pull/4255

Comment 11

[Alex Gibson [:agibson]]

Attachment: GitHub pull request

Comment 12

[[github robot]]

Commit pushed to master at https://github.com/mozilla/bedrock

https://github.com/mozilla/bedrock/commit/b20c87f919ec8c4795e3754e498457c4006ec303
[fix bug 1271247] Remove usage of target='_blank' and use rel='noopener'