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

rel attribute for external links #236

Open
ovlb opened this issue Jun 20, 2020 · 4 comments
Open

rel attribute for external links #236

ovlb opened this issue Jun 20, 2020 · 4 comments
Labels
help wanted Good for external contributors Status · Needs Submitter Info Requires more information from submitter before proceeding Type · Frontend Frontend work

Comments

@ovlb
Copy link
Collaborator

ovlb commented Jun 20, 2020

In the discussion of #218 @sarenji discovered that we were using malformed rel attributes on external links. I think we should reevaluate the usage of noreferrer. Using rel with this value hides Self-Defined from pages that are visited through us. Is that something we want?

Besides that, what we should add is noopener as this has a real performance benefit.

Would love to have your input which value (or combination thereof) we should use going forward.

@ovlb ovlb added help wanted Good for external contributors Type · Frontend Frontend work Status · Needs Submitter Info Requires more information from submitter before proceeding labels Jun 20, 2020
@ovlb ovlb mentioned this issue Jun 20, 2020
@sarenji
Copy link
Contributor

sarenji commented Jun 20, 2020

I think noreferrer/noopener only really needs to be set when the link also has target="_blank" for security and performance reasons. Some good news: rel="noreferrer" also implies rel="noopener". eslint-plugin-react has a good discussion on the spec & legacy browser support. So we just need to pick between noreferrer and noopener, and only if the link also has target="_blank".

It doesn't look like SelfDefined uses target="_blank" on its links. In that case, I'd be okay with removing the rel attribute altogether. If target="_blank" is used in the future, I don't have any strong opinions on whether to use noreferrer or noopener 😬

@tatianamac
Copy link
Collaborator

We can fix this; this was an error on my part. For posterity, I found this article helpful, and will place here in case this is brought up again in the future.

On another note: nofollow, for instance, might be a better replacement:

  • Remove any rel links
  • Add to documentation (if relevant) to avoid target="_blank"

I can make a PR for this in a little bit if we agree to this approach.

@sarenji
Copy link
Contributor

sarenji commented Jun 25, 2020

That checklist sounds good to me! I don't know enough about SEO to have an opinion about nofollow.

@ovlb
Copy link
Collaborator Author

ovlb commented Jun 25, 2020

Checklist looks good, yes. I’d say the addition of nofollow makes the most sense in the definition content and further reading sections, as we may link to sources that we do not necessarily want to be associated with. It’s pretty straight forward for further reading. For the content itself less so, as we have to write a plugin for markdown-it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Good for external contributors Status · Needs Submitter Info Requires more information from submitter before proceeding Type · Frontend Frontend work
Projects
None yet
Development

No branches or pull requests

3 participants