Skip to content

Adds rel nofollow to external links#4918

Merged
balloob merged 2 commits into
currentfrom
nofollow-external-links
Mar 15, 2018
Merged

Adds rel nofollow to external links#4918
balloob merged 2 commits into
currentfrom
nofollow-external-links

Conversation

@frenck
Copy link
Copy Markdown
Member

@frenck frenck commented Mar 14, 2018

Description:

Adds rel="external nofollow" to all external links on the website.

Fixes #4583

Pull request in home-assistant (if applicable): N/A

Checklist:

  • Branch: Fixes, changes and adjustments should be created against current. New documentation for platforms/components and features should go to next.
  • The documentation follow the standards.

Fixes #4583

Signed-off-by: Franck Nijhof <frenck@geekchimp.com>
@frenck frenck requested a review from balloob March 14, 2018 23:20
@frenck frenck changed the title ✨ Adds rel nofollow to external links Adds rel nofollow to external links Mar 14, 2018
Comment thread plugins/no_follow.rb
dom.css('a').each do |link|

# All external links start with 'http', skip when this one does not
next unless link.get_attribute('href') =~ /\Ahttp/i
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This assumption is not correct. For the release notes I use absolute urls because I copy paste same notes between GitHub and our site.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nevermind, just saw the home assistant check a few lines below

Comment thread plugins/no_follow.rb Outdated
next unless link.get_attribute('href') =~ /\Ahttp/i

# Play nice with links that already have a rel attribute set, skip it
next if link.get_attribute('rel')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't know about this, that means one can cheat our system

Copy link
Copy Markdown
Member Author

@frenck frenck Mar 14, 2018

Choose a reason for hiding this comment

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

Another take: It allows for exceptions and keeps the documented Jekyll behavior as is.
(Lets put everybody in prison to prevent crimes... 😉)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we just append ' external nofollow` if it exists?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Sounds like a plan

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done.

@frenck frenck changed the title Adds rel nofollow to external links [WIP] Adds rel nofollow to external links Mar 14, 2018
@frenck frenck changed the title [WIP] Adds rel nofollow to external links Adds rel nofollow to external links Mar 14, 2018
@frenck frenck changed the title Adds rel nofollow to external links [WIP] Adds rel nofollow to external links Mar 15, 2018
@frenck
Copy link
Copy Markdown
Member Author

frenck commented Mar 15, 2018

Latest change seems to add a lot of build time, need to investigate a little more. Adding WIP status.

@balloob
Copy link
Copy Markdown
Member

balloob commented Mar 15, 2018

I'm not surprised if it takes longer, it has to parse all HTML -> DOM and back

@frenck
Copy link
Copy Markdown
Member Author

frenck commented Mar 15, 2018

It actually wasn't that bad... ~500 sec for the full site generation (including the dom parsing and the installation/compilation of Nokogiri). After the last change, it seems impacted a lot.

@frenck frenck changed the title [WIP] Adds rel nofollow to external links Adds rel nofollow to external links Mar 15, 2018
@balloob balloob merged commit 23d593c into current Mar 15, 2018
@balloob balloob deleted the nofollow-external-links branch March 15, 2018 21:22
nielstron pushed a commit to nielstron/home-assistant.github.io that referenced this pull request Mar 22, 2018
* ✨ Adds rel nofollow to external links

Fixes home-assistant#4583

Signed-off-by: Franck Nijhof <frenck@geekchimp.com>

* 🎀 Adds support for appending nofollow to existing external links
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants