facebook / lexical

Lexical is an extensible text editor framework that provides excellent reliability, accessibility and performance.
https://lexical.dev
MIT License
19.6k stars 1.66k forks source link

Bug: [Playground] autolink escapes on second `.` when typing #3546

Open zeitstein opened 1 year ago

zeitstein commented 1 year ago

Steps To Reproduce

Type in e.g. https://www.foo.com in Playground. Works as expected on paste.

The current behavior

image

prestonbrown-me commented 1 year ago

some urls with subdomains do actually work. for example, typing https://playground.lexical.dev works. I believe this has to do with the autolink plugin itself, rather than the URL matcher used.

NikolajLeischner commented 1 year ago

I ran into the same issue and could track it to this change: https://github.com/facebook/lexical/commit/f29ab08dbf9c0b14b89ebef44fa243e712d817ad#diff-d62fd402ee470b9082046b4a435f031dd8539650efc92ac2ea645f8ded2a4c57R55

If you remove the "." from the PUNCTUATION_OR_SPACE regex, then the issue disappears.

thegreatercurve commented 1 year ago

Closing this as I'm not sure there's much we can do here, and this is technically behaving correctly.

The regex is accounting for valid URLs which omit their subdomains (i.e. https://github.com), but it doesn't make any distinction when such a URL uses "www" as it's actual domain.

This is technically valid behaviour. URLS like https://www.org are a valid, and we want them to be recognised by the regex.

beefancohen commented 1 year ago

this is technically behaving correctly.

imo it's most certainly a bug that https://www.google.com is not registering as a url

NikolajLeischner commented 1 year ago

Closing this doesn't seem very helpful. As is, the autolink-plugin is useless. It doesn't recognize most URLs with sub-domains correctly, and no URLs if you add a protocol. Note that this behavior is independent of the matchers you provide the plugin with.

acywatson commented 1 year ago

Closing this doesn't seem very helpful. As is, the autolink-plugin is useless. It doesn't recognize most URLs with sub-domains correctly, and no URLs if you add a protocol. Note that this behavior is independent of the matchers you provide the plugin with.

Agreed, this was closed prematurely, I think. We need to see what we can do to fix this.

Karibash commented 1 year ago

I ran into the same issue and could track it to this change: f29ab08#diff-d62fd402ee470b9082046b4a435f031dd8539650efc92ac2ea645f8ded2a4c57R55

If you remove the "." from the PUNCTUATION_OR_SPACE regex, then the issue disappears.

If the "." is removed from the regex, the following string will also be recognized as a URL, so it is necessary to consider another modification method. https://github.com...

NikolajLeischner commented 1 year ago

I ran into the same issue and could track it to this change: f29ab08#diff-d62fd402ee470b9082046b4a435f031dd8539650efc92ac2ea645f8ded2a4c57R55 If you remove the "." from the PUNCTUATION_OR_SPACE regex, then the issue disappears.

If the "." is removed from the regex, the following string will also be recognized as a URL, so it is necessary to consider another modification method. https://github.com...

Gotcha, I didn't test that case.. :)

NozomiSugiyama commented 1 year ago

It seems that the issue with https://github.com/... being allowed might be due to a problem with the URL REGEX in packages/lexical-playground/src/plugins/AutoLinkPlugin/index.tsx:

/((https?:\/\/(www\.)?)|(www\.))[-a-zA-Z0-9@:%._+~#=]{1,256}\.[a-zA-Z0-9()]{1,6}\b([-a-zA-Z0-9()@:%_+.~#?&//=]*)/.test("https://github.com...")
> true

Additionally, having the "." in PUNCTUATION_OR_SPACE causes issues with URLs that have extensions like .png functioning correctly.

I've submitted a PR to address this, and I would appreciate it if you could review it to ensure that the recognition works as intended.

4430

intrnl commented 1 year ago

is there another way we could do autolinks? I meant something closer to ProseMirror's decorations where these decorations aren't actual nodes on the document, they're just text ranges that tells the view to treat differently, it's much more robust compared to node manipulation

https://prosemirror.net/docs/ref/#view.Decorations

thorn0 commented 7 months ago

Can't PUNCTUATION_OR_SPACE be made configurable?

redstar504 commented 5 months ago

For anyone struggling with this issue, I wrote a very simple replacement plugin. I urgently needed a solution to unblock my project. Since I wanted to take a different approach, it seemed easier to start from scratch instead of modifying the existing plugin. Full disclosure, it's my first time releasing an npm package, so I have no idea how it will work for you, but it's worth a shot for those having trouble. The demo is available here. I would be happy for anyone to test it further.

MaxPlav commented 4 months ago

hi, it should be fixed now https://github.com/facebook/lexical/pull/6146