[lexical-link] Bug Fix: Enable autolink matching when it unlinked#8165
[lexical-link] Bug Fix: Enable autolink matching when it unlinked#8165etrepum merged 7 commits intofacebook:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
d0261f8 to
b0d32d4
Compare
etrepum
left a comment
There was a problem hiding this comment.
I'm not sure this fixes the whole problem, e.g. if you have an unlinked AutoLinkNode and add a decorator like date or emoji it will split the AutoLinkNode instead of destroying any of them
If during transformation one of the child nodes is not a TextNode or is not simple, the link will be destroyed. I added a test with emoji |
|
The tests are failing for the second time in a specific job |
|
I'm not sure either without looking a lot more closely, which is hard for me since I don't have a windows computer to test with. unit tests would be fine, it's unclear if marking them as flaky would be a sufficient workaround. Really depends on what kind of timing issue it is. |
c792aae to
475c316
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
Well, it took some time to figure out that the The truth turned out to be that even in the current version of lexical, without changes from this PR, there is a problem that this test revealed, and it also occurs on MacOS _Screen.Recording.2026-02-27.at.01.23.01.mov |
|
I also see that in I checked locally that the node transform handler is called once in both cases |
| test('Unlinked the autolink should not destruct if add non-spacing text in front or right after it', async ({ | ||
| page, | ||
| isPlainText, | ||
| }) => { | ||
| test.skip(isPlainText || LEGACY_EVENTS); |
There was a problem hiding this comment.
As a result, the current changes in this PR do not bring anything new to this scenario. It already fails in prod in legacy_events mode, so I added a condition to skip the test #8165 (comment)
Tests directly related to the new behavior are written below.
There was a problem hiding this comment.
I think legacy events are probably vestigial since we don't support browser versions that old anymore. AFAICT Chrome 60+ does not need it, and we don't support anything older than Chrome 86.
There was a problem hiding this comment.
Sounds like a good reason to remove legacy-events tests jobs. At the same time, I could refactor the tests and remove all stubs for legacy-events mode.
I can work on this and submit a separate PR
etrepum
left a comment
There was a problem hiding this comment.
Just adding some suggestions to the test names to make them a bit easier to understand, I'll go ahead and apply these changes directly
Description
Previously, it was possible to enter invalid characters in the text of an unlinked AutoLinkNode, which made the link invalid. The fix activates parsing of valid links in AutoLinkNode if they are marked as unlinked
Test plan
Before
Screen.Recording.2026-02-24.at.02.14.48.mov
After
Screen.Recording.2026-02-24.at.02.16.35.mov