Linking selected text containing a colon is interpreted as a URL

[drewkerr]

While auto-linking text such as mailto:foo@bar and tel:0123456789 is useful, any text including a colon is also automatically turned into a link, even if it contains spaces, such as: this text. The user is then required to edit and replace the URL (AFAIK this cannot be done with a keyboard shortcut and so breaks my Cmd+K typing flow).

I think the selected text is tested using the following utility function:

// Does the href start with something that looks like a URL protocol?
if ( /^\S+:/.test( trimmedHref ) ) {

Perhaps the regex could be modified to exclude text containing a colon immediately followed by a whitespace character:

if ( /^\S+:\S/.test( trimmedHref ) ) {

[talldan]

Handling the space as you mention sounds like a good idea. Contributions are definitely welcome on this.

[tjcafferkey]

Just been looking into this issue as a means of learning the Gutenberg codebase.

It looks like the solution referenced in the description won't work as this function isn't being used, but if it was, this would still return true here as it wouldn't meet any of the conditions and just default to true (I don't entirely know why it returns true rather than false at the end either, so some context would be great so I could learn).

The function responsible for checking this is here, we could import the isValidHref function into here to use but again, it would just return true unless we update the function to catch these types of occurrences.

Hoping to open a PR for this on Monday.

[talldan]

I did write that isValidHref function, so I should know, but it was a little while ago. 😄

From memory, I think it was more about validating something that's already expected to be a URL, it should be currently used when the user tries to manually enter a URL after clicking the link button on some (non-link) text.

It looks like the solution referenced in the description won't work as this function isn't being used, but if it was, this would still return true here as it wouldn't meet any of the conditions and just default to true

I just tried adding a unit test:

expect( isValidHref( 'as: this text' ) ).toBe( false );

and it seems to return false correctly. It also checked that it correctly returns false without the space between 'this' and 'text'. Some of the existing unit tests also seem to check for the situation described in the description so I think the function might already work.

I'm not sure what part of the function catches the white space, perhaps that could be more explicit.

I don't entirely know why it returns true rather than false at the end either, so some context would be great so I could learn.

It returns false whenever any invalid part of the HREF is discovered, and then returns true once all those invalidations are ruled out. I think the alternative is to use a logical &&. But I don't think it'd be quite readable or performance as the function would have to call all the get functions before calling any is function.