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

Links broken if no protocol provided #714

Closed
filipkis opened this issue Jun 30, 2015 · 8 comments
Closed

Links broken if no protocol provided #714

filipkis opened this issue Jun 30, 2015 · 8 comments

Comments

@filipkis
Copy link
Contributor

When inserting links using the anchor toolbar button and typing for instance only www.google.com (instead of full path http://www.google.com) the href of the anchor will only be set to the typed value, which in turn will cause the links to be broken (directing to http://yourapp.com/www.google.com).

The protocol (http://) is nicely added to links that are created automatically (autoLink = true), but not for the ones created from the toolbar.

@j0k3r
Copy link
Contributor

j0k3r commented Jun 30, 2015

We should add http:// by default if the inserted link doesn't start with https?://.

@filipkis
Copy link
Contributor Author

On further inspection, the adding of protocol works if the anchor: { linkValidation: true } is set, so the question is should the protocol also be added when the validation option is turned off?

I'm guessing there should be some consistency between how autoLink and anchor work.

@j0k3r
Copy link
Contributor

j0k3r commented Jun 30, 2015

In fact, the linkValidation is actually doing what you asked for. It adds http:// on anchor link if it doesn't start with a protocol

See https://github.com/yabwe/medium-editor/blob/master/src/js/extensions/anchor.js#L208-L211

I think the question is more, should linkValidation be turned true by default ?

And you're right about consistency about these two extensions.

@nmielnik
Copy link
Member

The problem here is that linkValidation is an implicit requirement of auto-link. Auto-link can only do something if it actually detects a valid link, so there can't really be a concept of turning off link validation for auto-link.

So, I don't think these options can be synchronized, however I do think it could be valid for linkValidation to be true by default...as long as we can convince ourselves that this doesn't require a major release to do 😅

@j0k3r
Copy link
Contributor

j0k3r commented Jun 30, 2015

Thinking about it, why this kind of feature is disabled by default and why this is an option at all.

Since anchor is about to insert a link, why people should insert relative link instead of absolute one?

@filipkis
Copy link
Contributor Author

I'm guessing having it as an option would still be good. Maybe in some cases you want relative links because the content is supposed to run on different domains (e.g. local and production), or you might be just creating hash links.

@j0k3r
Copy link
Contributor

j0k3r commented Jun 30, 2015

So we'll switch linkValidation defaut value to true. But we just need to allow hash url to be valid too.

@filipkis you're more than welcome to submit another PR about that 😄

@nmielnik
Copy link
Member

nmielnik commented Jul 5, 2015

For now, I think it will be better to leave things as there are currently, and not have linkValidation turned on by default. I don't think the fact that auto-link requires valid links is a strong reason to make the link extension require protocols on urls.

We can definitely revisit this later when there's a better case to be made in favor of it, but for now I'm going to close this issue. Thanks a bunch for raising this, it was definitely worth discussing.

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