protects against invalid Quill links

[mattoberle]

The default behavior when parsing a link in Quill:

1. Check if link matches a list of valid protocols.
2. If not, change link text to about:blank.

Kiln wraps the Link.sanitize function to prepend http:// in front
of any link missing a protocol.

As a result we can end up with invalid links like this:

"http://one two three"

This commit preserves the convenience factor of not requiring end
users to enter a protocol, but validates that the link produced is a
valid URL. Invalid URLs will revert to about:blank.

This commit also changes the default protocol to https:// which is a
reasonable expectation and more secure default.

[elgreg]

➕ 1 for this feature! We've had this come up a few times in the last couple of weeks.

[coveralls]

Coverage decreased (-0.01%) to 35.078% when pulling b76df32 on wysiwyg-link-validation into 211a95e on master.

[macgyver]

about:blank isn't really any more useful than "http://one two three" - deleting the link altogether would be better, at least that way the user would see that their text is not linked anymore and maybe realize there was a problem.

[mattoberle]

@macgyver I agree that it's not ideal, but it is more useful than a completely broken link (clicking it won't take take the end user away from the page to an unintended URL, including about:blank in templates like Apple News won't cause content to be rejected, etc).

This behavior is just the default behavior of Quill.

A more robust fix that deletes the link entirely would require some patching of Quill internals from what I could glean from a little experimentation.

Quill doesn't expose the create method for easy monkey-patching (unlike sanitize):

• https://github.com/quilljs/quill/blob/develop/formats/link.js

I'm open to suggestions / PRs for a full link deletion and will add an issue to the backlog so that an ideal fix stays on our radar.

This may be something better fixed in the Quill source itself, I'll see if there has been any discussion surrounding the about:blank behavior.

[macgyver]

duplicating the default behavior of Quill doesn't seem like a virtue in this case - also fwiw about:blank is considered invalid as far as AMP is concerned - try adding <a href='about:blank'>foo</a> to this page https://validator.ampproject.org/ - so this is an improvement solely in the case of Apple News, I guess.

I think a more comprehensive fix is to inform the CMS user, via a page or component validator, that the page contains invalid links so they can fix the issue before publishing. This isn't really something we can fix in code, we can't know what the CMS user was thinking when they entered a bad link, all we can do is inform them that it won't work and give them the ability to fix it themselves.

[mattoberle]

@macgyver I see what you mean about AMP.

In that case, <a href="http://one two three">foo</a> also fails validation so this change isn't introducing any new risk to AMP.

I'm working on creating an issue outlining the feature request.

It sounds like the lowest level of effort would be to skip creating the anchor tags for invalid links.
A better experience (although more complicated in implementation) would be to provide some sort of validation feedback for the end user.

I'll link the issue here to capture this discussion and provide a new location for further discussion.

Thanks for your input on this.