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

fix(extension-link): Avoid auto-linking partial text for invalid TLDs #4865

Merged
merged 1 commit into from
Mar 28, 2024

Conversation

rfgamaral
Copy link
Contributor

@rfgamaral rfgamaral commented Feb 7, 2024

Please describe your changes

This PR implements better autolink validation to ensure that only complete and valid text is hyperlinked, preventing cases where a valid top-level domain (TLD) is immediately followed by an invalid character, like a number.

For instance, with the find method from Linkify, typing the text example.com1 would result in example.com being linked and the trailing 1 left as plain text:

firefox_ybHTgnid0x

This behaviour is a bit unexpected, and this PR implements a more comprehensive validation on the input text.

How did you accomplish your changes

With the tokenize method instead of find, which allows us to perform a more comprehensive validation on the input text.

Note

The find method uses tokenize internally, but it does a much simpler validation.

How have you tested your changes

Used the Link extension demo to test multiple link formats to make sure that autolinking still worked as expected.

How can we verify your changes

Use the Link extension demo as I did.

Copy link

netlify bot commented Feb 7, 2024

Deploy Preview for tiptap-embed ready!

Name Link
🔨 Latest commit f5b39bf
🔍 Latest deploy log https://app.netlify.com/sites/tiptap-embed/deploys/65c4f4f9c46cc80008922cd8
😎 Deploy Preview https://deploy-preview-4865--tiptap-embed.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@rfgamaral rfgamaral force-pushed the ricardo/better-autolink-validation branch from 7df10ee to f5b39bf Compare February 8, 2024 15:36
@rfgamaral
Copy link
Contributor Author

There's one test failing, allows images to be added via URL, but it looks a flaky one, since I had this passing before in this PR, and fail in a different PR. Both PRs have changes unrelated to the failing test, as far as I can tell.

Copy link
Contributor

@bdbch bdbch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Just a quick question regarding the tokenize function. It says it's an external function only - but I still assume it's safe to use?

Also what does this do?

return ['()', '[]'].includes(tokens[0].value + tokens[2].value)

bit hard to understand why we check that () and [] array.

@rfgamaral
Copy link
Contributor Author

LGTM! Just a quick question regarding the tokenize function. It says it's an external function only - but I still assume it's safe to use?

Yes, it should be safe to use it. I mean, it's used by the test function itself.

Also what does this do?

return ['()', '[]'].includes(tokens[0].value + tokens[2].value)

It allows one to type a URL surrounded by parentheses or square brackets. So, if you were to type [example.com]<space>, pressing <space> would auto-link example.com inside the square brackets.

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

Successfully merging this pull request may close these issues.

2 participants