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

Links: Add ref="noreferrer noopener" for target="_blank" links #6316

Merged
merged 2 commits into from Apr 24, 2018

Conversation

youknowriad
Copy link
Contributor

closes #6186

This PR adds ref="noreferrer noopener" to target="_blank" links for security reasons.

Testing instructions

  • Add a new link that open in a separate tab using the link formatting control.
  • Ensure the attribute ref="noreferrer noopener" is added to the link in the code editor.

@youknowriad youknowriad added the [Feature] Rich Text Related to the Rich Text component that allows developers to render a contenteditable label Apr 20, 2018
@youknowriad youknowriad added this to the 2.8 milestone Apr 20, 2018
@youknowriad youknowriad self-assigned this Apr 20, 2018
Copy link
Member

@noisysocks noisysocks left a comment

Choose a reason for hiding this comment

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

Thanks for adding this! 😍

Looks like there's a regression with unsetting this toggle:

  1. Insert a link with Open in a new window turned ON
  2. Preview the post. Notice that the link opens in a new tab
  3. Edit the link and turn Open in a new window OFF
  4. Preview the post. Notice that the link still opens in a new tab

@@ -744,7 +744,8 @@ export class RichText extends Component {
if ( ! anchor ) {
this.removeFormat( 'link' );
}
this.applyFormat( 'link', { href: formatValue.value, target: formatValue.target }, anchor );
const { value: href, ...params } = formatValue;
this.applyFormat( 'link', { href, ...params }, anchor );
Copy link
Member

Choose a reason for hiding this comment

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

Feels like we could avoid some friction if we named the attribute href instead of value, but eh.

@youknowriad youknowriad dismissed noisysocks’s stale review April 23, 2018 07:42

Good catch, it looks like TinyMCE only patches the format instead of resetting it. Updated by explicitly passing null values.

Copy link
Member

@noisysocks noisysocks left a comment

Choose a reason for hiding this comment

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

Excellent 👍

@youknowriad youknowriad merged commit e05a542 into master Apr 24, 2018
@youknowriad youknowriad deleted the add/rel-noreferrer branch April 24, 2018 08:14
nuzzio pushed a commit to nuzzio/gutenberg that referenced this pull request Apr 25, 2018
…ress#6316)

* Links: Add ref="noreferrer noopener" for target="_blank" links

* Fix resetting the "open in new tab" option
@kfts
Copy link

kfts commented Nov 12, 2018

Is it possible to change this behavior with a plugin, and if so, how? noreferrer can cause issues for affiliate programs tracking the link. TinyMCE used to use add both noreferrer and noopener but has since switched to only adding noopener. Core has a filter that can be used to switch to just using noopener but how can this be done in Gutenberg?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Rich Text Related to the Rich Text component that allows developers to render a contenteditable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add rel="noreferrer noopener" when setting 'open in new tab' option
3 participants