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

Add rel="noopener noreferrer" wherever target="_blank" is used #7680

Merged
merged 13 commits into from
Sep 1, 2016
Merged
1 change: 0 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ module.exports = {
'react/jsx-curly-spacing': [ 1, 'always' ],
'react/jsx-no-bind': 1,
'react/jsx-no-duplicate-props': 1,
'react/jsx-no-target-blank': 1,
'react/jsx-space-before-closing': 1,
'react/jsx-uses-react': 1,
'react/jsx-uses-vars': 1,
Expand Down
2 changes: 1 addition & 1 deletion client/auth/connect.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const Connect = React.createClass( {
<Button href={ this.props.authUrl }>{ this.translate( 'Authorize App' ) }</Button>
</div>
}
<a className="auth__help" target="_blank" title={ this.translate( 'Visit the Calypso GitHub repository for help' ) } href="https://github.com/Automattic/wp-calypso">
<a className="auth__help" target="_blank" rel="noopener noreferrer" title={ this.translate( 'Visit the Calypso GitHub repository for help' ) } href="https://github.com/Automattic/wp-calypso">
<Gridicon icon="help" />
</a>
<div className="auth__links">
Expand Down
4 changes: 2 additions & 2 deletions client/auth/login.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const LostPassword = React.createClass( {
render: function() {
return (
<p className="auth__lost-password">
<a href="https://wordpress.com/wp-login.php?action=lostpassword" target="_blank">
<a href="https://wordpress.com/wp-login.php?action=lostpassword" target="_blank" rel="noopener noreferrer">
{ this.translate( 'Lost your password?' ) }
</a>
</p>
Expand Down Expand Up @@ -174,7 +174,7 @@ module.exports = React.createClass( {
{ errorMessage && <Notice text={ errorMessage } status={ errorLevel } showDismiss={ false } /> }
{ requires2fa && <AuthCodeButton username={ this.state.login } password={ this.state.password } /> }
</form>
<a className="auth__help" target="_blank" title={ this.translate( 'Visit the WordPress.com support site for help' ) } href="https://en.support.wordpress.com/">
<a className="auth__help" target="_blank" rel="noopener noreferrer" title={ this.translate( 'Visit the WordPress.com support site for help' ) } href="https://en.support.wordpress.com/">
<Gridicon icon="help" />
</a>
<div className="auth__links">
Expand Down
2 changes: 1 addition & 1 deletion client/blocks/comments/post-trackback.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export default class PostTrackback extends React.Component {
</div>

{ comment.author.URL
? <a href={ comment.author.URL } target="_blank" className="comments__comment-username">{ unescapedAuthorName }<Gridicon icon="external" /></a>
? <a href={ comment.author.URL } target="_blank" rel="noopener noreferrer" className="comments__comment-username">{ unescapedAuthorName }<Gridicon icon="external" /></a>
: <strong className="comments__comment-username">{ unescapedAuthorName }</strong> }

<small className="comments__comment-timestamp">
Expand Down
7 changes: 4 additions & 3 deletions client/blocks/credit-card-form/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -263,10 +263,11 @@ const CreditCardForm = React.createClass( {
'and {{managePurchasesSupportPage}}how to cancel{{/managePurchasesSupportPage}}.',
{
components: {
tosLink: <a href="//wordpress.com/tos/" target="_blank" />,
autoRenewalSupportPage: <a href={ support.AUTO_RENEWAL } target="_blank" />,
tosLink: <a href="//wordpress.com/tos/" target="_blank" rel="noopener noreferrer" />,
autoRenewalSupportPage: <a href={ support.AUTO_RENEWAL } target="_blank" rel="noopener noreferrer" />,
managePurchasesSupportPage: <a href={ support.MANAGE_PURCHASES }
target="_blank" />
target="_blank"
rel="noopener noreferrer" />
}
}
) }
Expand Down
2 changes: 2 additions & 0 deletions client/blocks/reader-author-link/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,13 @@ const ReaderAuthorLink = ( { author, post, siteUrl, children } ) => {
return children;
}

/* eslint-disable react/jsx-no-target-blank */
return (
<ExternalLink className="reader-author-link" href={ linkUrl } target="_blank" onClick={ recordAuthorClick }>
{ children }
</ExternalLink>
);
/* eslint-enable react/jsx-no-target-blank */
};

ReaderAuthorLink.propTypes = {
Expand Down
5 changes: 4 additions & 1 deletion client/blocks/reader-full-post/header.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const ReaderFullPostHeader = ( { post } ) => {
recordGaEvent( 'Clicked Post Permalink', 'timestamp' );
};

/* eslint-disable react/jsx-no-target-blank */
return (
<div className="reader-full-post__header">
{ post.title
Expand All @@ -38,7 +39,8 @@ const ReaderFullPostHeader = ( { post } ) => {
<a className="reader-full-post__header-date-link"
onClick={ recordDateClick }
href={ post.URL }
target="_blank">
target="_blank"
rel="noopener noreferrer">
<PostTime date={ post.date } />
</a>
</span> : null }
Expand All @@ -51,6 +53,7 @@ const ReaderFullPostHeader = ( { post } ) => {
</div>
</div>
);
/* eslint-enable react/jsx-no-target-blank */
};

ReaderFullPostHeader.propTypes = {
Expand Down
2 changes: 2 additions & 0 deletions client/blocks/site/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,14 @@ export default React.createClass( {
url = this.props.site.options.admin_url + 'options-general.php';
}

/* eslint-disable react/jsx-no-target-blank */
return (
<ExternalLink icon={ true } href={ url } target="_blank" className="site__edit-icon" onClick={ this.onEditIconClick }>
<SiteIcon site={ this.props.site } />
<span className="site__edit-icon-text">{ this.translate( 'Edit Icon' ) }</span>
</ExternalLink>
);
/* eslint-enable react/jsx-no-target-blank */
},

getHref() {
Expand Down
1 change: 1 addition & 0 deletions client/components/app-promo/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ export default React.createClass( {
title="Try the desktop app!"
href={ promo_link }
target="_blank"
rel="noopener noreferrer"
>
<img
className="app-promo__icon"
Expand Down
5 changes: 4 additions & 1 deletion client/components/button/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ export default class Button extends PureComponent {
scary: PropTypes.bool,
type: PropTypes.string,
href: PropTypes.string,
borderless: PropTypes.bool
borderless: PropTypes.bool,
target: PropTypes.string,
rel: PropTypes.string
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be argued that these be included in the else block in render to push into omitProps for the button case, since they're not valid attributes for a button. This assumes someone renders as a button with rel and target but not href, which is unlikely, be "possible".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call. Added in b88562b.

};

static defaultProps = {
Expand All @@ -30,6 +32,7 @@ export default class Button extends PureComponent {
omitProps.push( 'type' );
} else {
tag = 'button';
omitProps.push( 'target', 'rel' );
}

return createElement( tag, {
Expand Down
4 changes: 2 additions & 2 deletions client/components/card/docs/example.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ var Cards = React.createClass( {
<Card>I am another Card.</Card>
<Card className="awesome sauce">I am a third Card with custom classes!</Card>
<Card href="#cards">I am a linkable Card</Card>
<Card href="#cards" target="_blank">I am a externally linked Card</Card>
<Card href="#cards" target="_blank" rel="noopener noreferrer">I am a externally linked Card</Card>
</div>
);
} else {
Expand All @@ -50,7 +50,7 @@ var Cards = React.createClass( {
<CompactCard>I am another CompactCard.</CompactCard>
<CompactCard className="awesome sauce">I am a third CompactCard with custom classes!</CompactCard>
<CompactCard href="#cards">I am a linkable CompactCard</CompactCard>
<CompactCard href="#cards" target="_blank">I am a externally linked CompactCard</CompactCard>
<CompactCard href="#cards" target="_blank" rel="noopener noreferrer">I am a externally linked CompactCard</CompactCard>
</div>
);
}
Expand Down
3 changes: 2 additions & 1 deletion client/components/domains/map-domain-step/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ var MapDomainStep = React.createClass( {
components: {
a: <a
target="_blank"
rel="noopener noreferrer"
href={ `https://dotblog.wordpress.com/
?email=${ this.props.currentUser && encodeURIComponent( this.props.currentUser.email ) || '' }
&domain=${ domain }` }/>
Expand Down Expand Up @@ -229,7 +230,7 @@ var MapDomainStep = React.createClass( {
{
components: {
strong: <strong />,
a1: <a target="_blank" href="http://wordpressfoundation.org/trademark-policy/"/>,
a1: <a target="_blank" rel="noopener noreferrer" href="http://wordpressfoundation.org/trademark-policy/"/>,
a2: <a href={ support.CALYPSO_CONTACT }/>
}
}
Expand Down
18 changes: 10 additions & 8 deletions client/components/empty-content/empty-content.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,29 +30,31 @@ module.exports = React.createClass( {
] ),
actionURL: React.PropTypes.string,
actionCallback: React.PropTypes.func,
actionTarget: React.PropTypes.string,
secondaryAction: React.PropTypes.oneOfType( [
React.PropTypes.string,
React.PropTypes.element
] ),
secondaryActionURL: React.PropTypes.string,
secondaryActionCallback: React.PropTypes.func,
secondaryActionTarget: React.PropTypes.string,
className: React.PropTypes.string,
isCompact: React.PropTypes.bool
},

componentDidMount: function() {
componentDidMount() {
debug( 'Empty Content React component mounted.' );
},

getDefaultProps: function() {
getDefaultProps() {
return {
title: "You haven't created any content yet.",
illustration: '/calypso/images/drake/drake-empty-results.svg',
isCompact: false
};
},

primaryAction: function() {
primaryAction() {
if ( 'string' !== typeof this.props.action ) {
return this.props.action;
}
Expand All @@ -62,7 +64,7 @@ module.exports = React.createClass( {
} else if ( this.props.actionURL ) {
let targetProp = {};
if ( this.props.actionTarget ) {
targetProp = { target: this.props.actionTarget };
targetProp = { target: this.props.actionTarget, rel: "noopener noreferrer" };
}

return <a className="empty-content__action button is-primary" href={ this.props.actionURL } { ...targetProp }>{ this.props.action }</a>;
Expand All @@ -71,7 +73,7 @@ module.exports = React.createClass( {
}
},

secondaryAction: function() {
secondaryAction() {
if ( 'string' !== typeof this.props.secondaryAction ) {
return this.props.secondaryAction;
}
Expand All @@ -80,8 +82,8 @@ module.exports = React.createClass( {
return <a className="empty-content__action button" onClick={ this.props.secondaryActionCallback } href={ this.props.secondaryActionURL }>{ this.props.secondaryAction }</a>;
} else if ( this.props.secondaryActionURL ) {
let targetProp = {};
if ( this.props.actionTarget ) {
targetProp = { target: this.props.secondaryActionTarget };
if ( this.props.secondaryActionTarget ) {
targetProp = { target: this.props.secondaryActionTarget, rel: "noopener noreferrer" };
}

return <a className="empty-content__action button" href={ this.props.secondaryActionURL } { ...targetProp }>{ this.props.secondaryAction }</a>;
Expand All @@ -90,7 +92,7 @@ module.exports = React.createClass( {
}
},

render: function() {
render() {
const action = this.props.action && this.primaryAction();
const secondaryAction = this.props.secondaryAction && this.secondaryAction();
const illustration = this.props.illustration && <img src={ this.props.illustration } width={ this.props.illustrationWidth } className="empty-content__illustration" />;
Expand Down
4 changes: 3 additions & 1 deletion client/components/external-link/docs/example.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,19 @@ export default React.createClass( {
displayName: 'ExternalLink',

render() {
/* eslint-disable react/jsx-no-target-blank */
return (
<div className="design-assets__group">
<h2>
<a href="/devdocs/design/external-link">External Link</a>
</h2>
<Card>
<p><ExternalLink icon={ true } href="https://wordpress.org" >WordPress.org</ExternalLink></p>
<p><ExternalLink icon={ true } target="_blank" href="https://wordpress.org">WordPress.org</ExternalLink></p>
<p><ExternalLink href="https://wordpress.org">WordPress.org</ExternalLink></p>
</Card>

</div>
);
/* eslint-enable react/jsx-no-target-blank */
}
} );
7 changes: 6 additions & 1 deletion client/components/external-link/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ export default React.createClass( {
href: React.PropTypes.string,
onClick: React.PropTypes.func,
icon: React.PropTypes.bool,
iconSize: React.PropTypes.number
iconSize: React.PropTypes.number,
target: React.PropTypes.string
},

getDefaultProps() {
Expand All @@ -41,6 +42,10 @@ export default React.createClass( {
rel: 'external'
} );

if ( props.target ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need the rel if target is anything other than _blank ?

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#attr-target

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you specify an arbitrary target like banana, you still seem to get a window.opener in the resulting new window. I thought it was best to be safe and always append the rel.

props.rel = props.rel.concat( ' noopener noreferrer' );
Copy link
Contributor

Choose a reason for hiding this comment

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

TIL String#concat. Then again, this could have just been props.rel += ' noopener noreferrer';

No need to change anything.

}

return (
<a { ...props }>
{ this.props.children }
Expand Down
2 changes: 1 addition & 1 deletion client/components/happiness-support/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ const HappinessSupport = React.createClass( {
}

return (
<Button href={ url } target="_blank">
<Button href={ url } target="_blank" rel="noopener noreferrer">
{ this.translate( 'Search our support site' ) }
</Button>
);
Expand Down
3 changes: 3 additions & 0 deletions client/components/purchase-detail/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const PurchaseDetail = ( {
onClick,
requiredText,
target,
rel,
title
} ) => {
const classes = classNames( 'purchase-detail', {
Expand All @@ -40,6 +41,7 @@ const PurchaseDetail = ( {
href={ href }
onClick={ onClick }
target={ target }
rel={ rel }
text={ buttonText } />
);
}
Expand Down Expand Up @@ -91,6 +93,7 @@ PurchaseDetail.propTypes = {
onClick: PropTypes.func,
requiredText: PropTypes.string,
target: PropTypes.string,
rel: PropTypes.string,
title: PropTypes.string
};

Expand Down
3 changes: 3 additions & 0 deletions client/components/purchase-detail/purchase-button.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const PurchaseButton = ( {
disabled,
onClick = () => {},
target,
rel,
text,
primary = true
} ) => {
Expand All @@ -24,6 +25,7 @@ const PurchaseButton = ( {
href={ href }
onClick={ onClick }
target={ target }
rel={ rel }
primary={ primary }
>
{ text }
Expand All @@ -37,6 +39,7 @@ PurchaseButton.propTypes = {
disabled: PropTypes.bool,
onClick: PropTypes.func,
target: PropTypes.string,
rel: PropTypes.string,
text: PropTypes.string,
primary: PropTypes.bool
};
Expand Down
3 changes: 2 additions & 1 deletion client/components/signup-form/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,8 @@ export default React.createClass( {
a: <a
href={ this.getTermsOfServiceUrl() }
onClick={ this.handleOnClickTos }
target="_blank" />
target="_blank"
rel="noopener noreferrer" />
}
}
)
Expand Down
2 changes: 1 addition & 1 deletion client/components/tinymce/plugins/wplink/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ function wpLink( editor ) {
renderHtml: function() {
return (
'<div id="' + this._id + '" class="wp-link-preview">' +
'<a href="' + this.url + '" target="_blank" tabindex="-1">' + this.url + '</a>' +
'<a href="' + this.url + '" target="_blank" rel="noopener noreferrer" tabindex="-1">' + this.url + '</a>' +
'</div>'
);
},
Expand Down
1 change: 1 addition & 0 deletions client/components/web-preview/toolbar.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ export const PreviewToolbar = props => {
className="web-preview__external"
href={ externalUrl || previewUrl }
target="_blank"
rel="noopener noreferrer"
>
<Gridicon icon="external" />
</a>
Expand Down
2 changes: 1 addition & 1 deletion client/devdocs/doc.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ module.exports = React.createClass( {
<div className="devdocs devdocs__doc">
<CompactCard className="devdocs__doc-header">
Path: <code>{ this.props.path }</code>
<a href={ editURL } target="_blank">Improve this document on GitHub &rarr;</a>
<a href={ editURL } target="_blank" rel="noopener noreferrer">Improve this document on GitHub &rarr;</a>
</CompactCard>
<div
className="devdocs__doc-content"
Expand Down
2 changes: 1 addition & 1 deletion client/layout/community-translator/invitation.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export default React.createClass( {
content: this.translate( 'Help translate the WordPress.com dashboard into your' +
' native language using the Community Translator tool. ' +
'{{docsLink}}Find out more{{/docsLink}}. ',
{ components: { docsLink: <a target="_blank"
{ components: { docsLink: <a target="_blank" rel="noopener noreferrer"
className="community-translator__translator-invitation__link"
href="https://en.support.wordpress.com/community-translator/"
onClick={ this.docsLink } /> } } )
Expand Down
Loading