Skip to content

JITM: Add noopener noreferrer to jitm button link#8517

Merged
jeherve merged 1 commit intomasterfrom
add/jitm-noopener-noreferrer
Jan 16, 2018
Merged

JITM: Add noopener noreferrer to jitm button link#8517
jeherve merged 1 commit intomasterfrom
add/jitm-noopener-noreferrer

Conversation

@oskosk
Copy link
Copy Markdown
Contributor

@oskosk oskosk commented Jan 13, 2018

  • Makes the button link in JITM open a new window without sharing control of the current window to the destination site.

Changes proposed in this Pull Request:

  • Adds attribute rel="noopener noreferrer" to JITM button link in _inc/jetpack-jitm.js.

Testing instructions:

  • Checkout this branch.
  • Confirm that the JITM setup link contains the rel attribute.

@oskosk oskosk added [Type] Janitorial [Feature] JITM Just In Time Messages - pop-up tips and suggestions that appear on the dashboard and sidebar. labels Jan 13, 2018
@oskosk oskosk added this to the 5.8 milestone Jan 13, 2018
@oskosk oskosk requested a review from withinboredom January 13, 2018 14:06
@oskosk oskosk requested a review from a team as a code owner January 13, 2018 14:06
@oskosk oskosk requested a review from dereksmart January 13, 2018 14:06
@oskosk oskosk added the [Status] Needs Review This PR is ready for review. label Jan 13, 2018
@oskosk oskosk requested a review from jeherve January 13, 2018 14:07
@withinboredom
Copy link
Copy Markdown
Contributor

withinboredom commented Jan 13, 2018 •
edited
Loading

I'm a little hesitant to add the noreferrer part since it may impact analytics some. Also, I'm not sure these attributes will do much since it actually opens via

jetpack/_inc/lib/tracks/tracks-ajax.js

Line 38 in f0180a0

var newTabWindow = window.open( '', target );

We probably also need to change that code to do something like:

var newTabWindow = window.open( '', target );
newTabWindow.opener = null;

as well.

Good find!

@withinboredom withinboredom mentioned this pull request Jan 13, 2018
Merged
@oskosk
Copy link
Copy Markdown
Contributor Author

oskosk commented Jan 13, 2018

niceee

withinboredom
withinboredom approved these changes Jan 13, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@withinboredom withinboredom left a comment

Choose a reason for hiding this comment

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

LGTM

@dereksmart dereksmart removed the [Status] Needs Review This PR is ready for review. label Jan 15, 2018
@withinboredom withinboredom reopened this Jan 15, 2018
@withinboredom withinboredom added the [Status] Ready to Merge Go ahead, you can push that green button! label Jan 15, 2018
@withinboredom
Copy link
Copy Markdown
Contributor

withinboredom commented Jan 15, 2018

Well, that auto-close was unexpected. Let's merge this too because there are people who have js disabled.

jeherve
jeherve approved these changes Jan 16, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

Looking good. Merging.

@jeherve jeherve merged commit d3d8096 into master Jan 16, 2018
@jeherve jeherve deleted the add/jitm-noopener-noreferrer branch January 16, 2018 11:23
@jeherve jeherve removed the [Status] Ready to Merge Go ahead, you can push that green button! label Jan 16, 2018
jeherve added a commit that referenced this pull request Jan 25, 2018
@jeherve
Changelog: add #8517
b9f6c1c
zinigor pushed a commit that referenced this pull request Jan 30, 2018
@oskosk @zinigor
Changelog 5.8: running changelog (#8442)
1ee2c03 
* Changelog 5.8: create base for changelog.

* Update 5.8 release post link

* fix 5.8 release date

* Updates to plugin description

* Changelog: add #8499

* Changelog: add #8506

* Changelog: add #8509

* Changelog: add #8516

* Changelog: add #8517

* Changelog: add #8523

* Changelog: add #8547

* Changelog: add #8496

* Changelog: add #8584

* Changelog: add #8595

* Changelog: add #8445

* Changelog: add #8431

* Changelog: add #8284

* Changelog: add #8270

* Changelog: add #8124

* Changelog: add #8581

* Changelog: add #8463

* Changelog: add #8568 (#8646)

* Updates to testing list and changelog

* Changelog: add #8443

* Changelog: add #8459

* Changelog: add #8469

* Changelog: add #8464

* Changelog: add #8478 and #8479

* Changelog: add #8483

* Changelog: add #8488

* Changelog: add #8513

* Changelog: add #8555

* Changelog: add #8565

* Changelog: add #8601

* Changelog: add #8612

* Changelog: add first pass at Search items.

* Changelog: add more info to help test Search.

* Changelog: add #8144

* Changelog: add #8313

* Changelog: add #8419

* Changelog: add #8465

* Changelog: add #8515

* Changelog: add #8587

* Changelog: add #8591

* Changelog: add #8659

* Changelog: add #8661

* Changelog: add #8671

* Changelog: add 5.7.1 to archived changelog too.

* Reverted changes to readme, removed entry about backups.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jeherve jeherve jeherve approved these changes

@dereksmart dereksmart Awaiting requested review from dereksmart

+1 more reviewer

@withinboredom withinboredom withinboredom approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

[Feature] JITM Just In Time Messages - pop-up tips and suggestions that appear on the dashboard and sidebar. [Type] Janitorial

Projects

None yet

Milestone

5.8

Development

Successfully merging this pull request may close these issues.

5 participants

@oskosk @withinboredom @jeherve @dereksmart @matticbot