Skip to content

Conversation

mstriemer
Copy link
Contributor

@mstriemer mstriemer commented Jun 2, 2016

Without this the page won't load since it isn't on discovery.addons.mozilla.org.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 0175793 on mstriemer:see-more-link-blank into 72e0efb on mozilla:master.

{results.map((item, i) => <Addon {...camelCaseProps(item)} key={i} />)}
<div className="amo-link">
<a href="https://addons.mozilla.org/">{i18n.gettext('See more add-ons!')}</a>
<a href="https://addons.mozilla.org/" target="_blank">
Copy link
Contributor

@muffinresearch muffinresearch Jun 2, 2016

Choose a reason for hiding this comment

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

We could add rel="noreferrer" which gives us the same effect as rel="noopener"

Copy link
Contributor

Choose a reason for hiding this comment

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

Here's a demo since Matias' page doesn't have one: http://staticfil.es/moz/opener.html

Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose this is being very paranoid since we own the destination but still.

/me puts on tinfoil hat

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking we might want to log that they came from the disco pane. Maybe that logging is better off in here though?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah that would get implicitly logged via UA I would think. E.g. should show up as an exit page.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is that a good thing then or should I still add the noreferrer?

@muffinresearch
Copy link
Contributor

Nice catch, I wondered about this and then totally forgot to test it under the iframe.

r+wc

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 1ae2849 on mstriemer:see-more-link-blank into 72e0efb on mozilla:master.

@mstriemer mstriemer merged commit c254967 into mozilla:master Jun 2, 2016
@mstriemer mstriemer deleted the see-more-link-blank branch June 2, 2016 18:27
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.

3 participants