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

Link Support: target and rel attributes #413

Closed
alexsasharegan opened this issue May 18, 2017 · 18 comments
Closed

Link Support: target and rel attributes #413

alexsasharegan opened this issue May 18, 2017 · 18 comments
Assignees

Comments

@alexsasharegan
Copy link
Member

When working with some of the special components that take to or href attributes (basically components using the link mixin), there isn't support for passing down the target and rel attributes.

I would love it if the default prop value for rel was rel="noopener noreferrer" to add a security-boosting default per this article: https://www.jitbit.com/alexblog/256-targetblank---the-most-underestimated-vulnerability-ever/. Maybe that could be triggered with a computed property so it only kicks in if a target other than _self is specified.

<template>
    <b-navbar toggleable
              type="inverse"
              variant="primary">
        <b-nav is-nav-bar>
            <b-nav-item :href="FACEBOOK"
                        target="_blank"
                        rel="noopener noreferrer">
                <i class="fa fa-fw fa-facebook"></i>
            </b-nav-item>
        </b-nav>
    </b-navbar>
</template>
@tmorehouse
Copy link
Member

tmorehouse commented May 18, 2017

Rather than putting too much logic into the component, we could just provide new props (target and rel) to the link component(s), and rely on the user assigning values appropriately for their use case.

@alexsasharegan
Copy link
Member Author

Definitely! Slim and trim!

@tmorehouse
Copy link
Member

I'll work on a PR for adding the two new props.

@tmorehouse tmorehouse self-assigned this May 18, 2017
@alexsasharegan
Copy link
Member Author

Gracias, señor!

@pi0
Copy link
Member

pi0 commented May 18, 2017

I agree with troy. But only about rel=noopener it is REQUIRED for PWA checklist when target=_blank so what's your idea using a computed prop for rel and adding this automatically when no rel provided?

@tmorehouse
Copy link
Member

Hmmm... so create a computed prop computedRel that is based on both this.rel and this.target (depending on their values or lack of values)?

@tmorehouse
Copy link
Member

From https://www.jitbit.com/alexblog/256-targetblank---the-most-underestimated-vulnerability-ever/

Update: FF does not support "noopener" so add this.

rel="noopener noreferrer"

So should it default to both if target="_blank"?

What about cases where a user is linking to their own website (i.e. same domain) and wants _blank, but also needs the referer set?

Maybe if this.rel is null then default the rel for target _blank, but if this.rel is set to an empty string then remove the rel attribute?

@tmorehouse
Copy link
Member

I can see defaulting rel for user supplied link content, but if it is author supplied content, then it shouldn't be set (maybe)?

@alexsasharegan
Copy link
Member Author

Not to complicate things, but you can put any string in target that isn't _self (I believe), and you alias a new window.

@alexsasharegan
Copy link
Member Author

alexsasharegan commented May 18, 2017

I would say the rel has a default value of noopener noreferrer, but only kicks in when the target attribute is null or not _self. Then if the user overrides it, they can set it to null. So that has computed prop written all over it to me.

@tmorehouse
Copy link
Member

@alexsasharegan true, any target that is not preceded by an _ opens a new (referenceable) window.

@tmorehouse
Copy link
Member

take a look at PR #418 and see what you think

@alexsasharegan
Copy link
Member Author

So, I was getting myself confused about opening in a new window or in the same window. Really, you would only want to add the rel attribute if the link is not same domain. That's hard to do without adding too much code.

Took a look at you addition. Looks good, but I think the noopener should be on any non-null target. Thoughts?

@tmorehouse
Copy link
Member

But what if the user does want window to window communication for their own domain?

@alexsasharegan
Copy link
Member Author

alexsasharegan commented May 18, 2017

That's why I think it might be more advantageous to default to noopener. Then check that target is not null, and just return the rel. Then they can specify null if they need communication.

@tmorehouse
Copy link
Member

What about if target is not _self or _top, then don't set noopener?

@alexsasharegan
Copy link
Member Author

Maybe we should try to keep this on the PR. I'm getting dizzy!

@tmorehouse
Copy link
Member

PR #418 Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants