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

mailto links (weird behavior) #9

Closed
zslabs opened this issue Jan 3, 2019 · 11 comments
Closed

mailto links (weird behavior) #9

zslabs opened this issue Jan 3, 2019 · 11 comments
Labels
🗄 area/interface This affects the public interface 💪 phase/solved Post is done 🧒 semver/minor This is backwards-compatible change 🦋 type/enhancement This is great to have

Comments

@zslabs
Copy link
Contributor

zslabs commented Jan 3, 2019

Currently links that start with mailto: have the external link options applied; which is an odd behavior as when clicked the mail client is opened, but a blank tab is shown in the browser.

@wooorm
Copy link
Member

wooorm commented Jan 3, 2019

Currently, any URL that starts with a protocol is seen as external (through here and here).

We can go two ways: see every protocol except mailto as absolute, or see only http / https as absolute. What are your thoughts, and benefits / tradeoffs?

/cc @makenowjust (last person to touch that line)

@zslabs
Copy link
Contributor Author

zslabs commented Jan 3, 2019

Hey @wooorm appreciate the quick reply as always. I could see this being a compounding issue for protocols like tel:, skype:, etc. A quick google search for all of them brought up https://www.iana.org/assignments/uri-schemes/uri-schemes.xhtml

Do you think it'd be worth trying to "whitelist" those in that list? I can't personally think of a time you'd want to have any of those protocols open up a new tab, but I'm not familiar with a bunch of 'em.

@wooorm
Copy link
Member

wooorm commented Jan 3, 2019

I’m not really into adding a big list here 😕 So I’m up for allowing just http / https. But I can see potential exceptions in the future (about? blob? more?). Or just ignoring mailto for now and seeing where that goes?

@zslabs
Copy link
Contributor Author

zslabs commented Jan 3, 2019

Yeah maybe just checking for http/https would be best for now; as I could make the same argument for a number of those in that list.

@wooorm
Copy link
Member

wooorm commented Jan 4, 2019

/cc also maybe @xuopled or @fand have thoughts on this

@makenowjust
Copy link
Contributor

Only Chrome (and Chromium family browsers like Vivaldi) remains a blank tab after clicking a mailto link having target=_blank attribute. Other browsers (Safari, Firefox and Edge) doesn't produce a new tab.

I think it is Chrome's problem, however treating only http/https is good workaround.

@wooorm
Copy link
Member

wooorm commented Jan 4, 2019

Hmm, also like to add that people can set up mailto: to open in the browser (such as in Gmail or so). In which case the target=_blank definitely makes sense.

@zslabs
Copy link
Contributor Author

zslabs commented Jan 4, 2019

Hmm, also like to add that people can set up mailto: to open in the browser (such as in Gmail or so). In which case the target=_blank definitely makes sense.

While I do agree there can be different cases for protocol URLs, I myself have never seen a target="blank" attached to a mailto: link; and is something they can easily customize on their own by just writing out the link as regular HTML if needed.

@wooorm
Copy link
Member

wooorm commented Jan 4, 2019

How about the following pseudo-code changes?

  • var defaultProtocols = ['http', 'https']
  • var protocols = opts.protocols || defaultProtocols
  • if (ctx && isAbsoluteURL(ctx.url) && protocols.indexOf(ctx.url.slice(0, ctx.url.indexOf(':')) !== -1)) {

...that would allow people to still specify their own protocols.

@zslabs Want to write a PR for this, with tests?

@cedricdelpoux
Copy link

It sounds good to me @wooorm

@zslabs
Copy link
Contributor Author

zslabs commented Jan 4, 2019

Just created #10 let me know what you think. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🗄 area/interface This affects the public interface 💪 phase/solved Post is done 🧒 semver/minor This is backwards-compatible change 🦋 type/enhancement This is great to have
Development

No branches or pull requests

4 participants