[lexical] Bug Fix: Fix Chrome on android deletion bugs

[waynetee]

Description

Fixes #4340, #5042 and #1965

1. On Chrome on Android, deletions are usually handled by the browser instead of the DELETE_CHARACTER_COMMAND, however the browser is unable to deleted decorator nodes, this PR detects this scenario and dispatches DELETE_CHARACTER_COMMAND accordingly
2. When deleting text across paragraphs, Chrome on Android shifts the selection rightwards, this PR restores the correct selection

Test plan

Before

screen-20250203-030555.mp4

After

screen-20250203-030819.mp4

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
lexical	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Feb 3, 2025 4:21am
lexical-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Feb 3, 2025 4:21am

[github-actions]

size-limit report 📦

Path	Size
lexical - cjs	29.12 KB (+0.18% 🔺)
lexical - esm	28.92 KB (+0.2% 🔺)
@lexical/rich-text - cjs	38.08 KB (-0.1% 🔽)
@lexical/rich-text - esm	30.98 KB (+0.22% 🔺)
@lexical/plain-text - cjs	36.63 KB (+0.17% 🔺)
@lexical/plain-text - esm	28.22 KB (-0.03% 🔽)
@lexical/react - cjs	39.93 KB (+0.08% 🔺)
@lexical/react - esm	32.36 KB (+0.26% 🔺)

[etrepum]

This looks "fine" to me, I think the setTimeout is setting ourselves up for some future clean-up. I don't have an android chrome device to do any testing with so I am not volunteering to come up with anything more elegant. It would be nice if someone with access to this kind of device could verify before merge.

[etrepum]

Would be nice if someone with access to an Android + Chrome device could take a look at this before merge to confirm the bug & fix

[potatowagon]

is the reason to wrap the workaround in setTimeout with no delay param to:

remove the function from the execution queue and it will only be invoked after JavaScript has finished with the current execution queue

https://stackoverflow.com/questions/3580068/is-settimeout-with-no-delay-the-same-as-executing-the-function-instantly

[etrepum]

The reason is that few people understand how lexical event handling works so they add hacks like this. It’s certainly not strictly necessary to use setTimeout here, but I don’t have access to android to trace exactly when/why this code needs to run.

Usually the root cause for hacks like this is that an event comes in from the browser and then lexical handles that event without preventing default but also needs to do something after the platform handles the default. Lexical doesn’t have good/obvious primitives for queuing that up so people write code like this which “works” but has subtle problems of its own.

[etrepum]

The update cascade happens because lexical’s reconciliation happens at the next microtask which will be before the setTimeout callback runs, so a second reconciliation will happen because of this workaround. If the code was queued up in precisely the right way only one lexical reconciliation would occur.

[waynetee]

@potatowagon Yep you are absolutely right.

Here's the current flow:

1. User presses backspace
2. beforeinput event is triggered, handling the deletion
3. Reconciliation happens
4. We call domSelection.setBaseAndExtent() , shifting the caret to the correct position
5. The browser emits selectionchange, incorrectly shifting the caret

Using setTimeout allows us to restore the selection after selectionchange has occurred.

@etrepum If you are against the use of setTimeout you are welcome to suggest a better fix. Perhaps you can try augmenting the selectionchange handler to detect this change and revert it there, but I personally don't think that would work better. If its performance you are worried about, I don't think it makes a big difference given that backspacing normally within the same line already requires two reconciliations per character, based on testing on my Pixel.

If you need access to an Android device, I suggest https://www.browserstack.com/

[etrepum]

Turns out that this indeed did cause problems, I am guessing because it is over-writing some other changes to the selection that are expected to happen due to a different update or event. For now it will be reverted (#7218) because it causes worse problems in certain situations.

[waynetee]

You are right. Turns out that subsequent browser changes to selection are sometimes desired. Apologies to all those affected by the bugs introduced.

[potatowagon]

Screen_Recording_20250206-153236_Samsung.Internet.mp4

im unable to repro the issue on my android, could it be a keyboard setting? not sure if i need to enable maybe Right to Left language in my android chrome browser?

[potatowagon]

testing changes from https://lexical-playground-my5rjurxj-fbopensource.vercel.app/

Screen_Recording_20250206-154331_Telegram_1.mp4

i dont see any issues so should be safe to merge, tho i havnt yet been able to repro the bug

[waynetee]

im unable to repro the issue on my android, could it be a keyboard setting? not sure if i need to enable maybe Right to Left language in my android chrome browser?

Looks like you are using the Samsung keyboard, which doesn't seem to have this issue. Try using GBoard? It's the default browser on my Pixel device.

[potatowagon]

you are using the Samsung keyboard, which doesn't seem to have this issue. Try using GBoard? It's the default browser on my Pixel device.

thanks for this info. i have access to a google pixel too so let me try it there

[potatowagon]

you are using the Samsung keyboard, which doesn't seem to have this issue. Try using GBoard? It's the default browser on my Pixel device.

thanks for this info. i have access to a google pixel too so let me try it there

screen-20250207-004517.mp4

still unable to repro the bug on GBoard on a google pixel. Gboard About shown towards end of screen capture, has version info. could it be a particular keyboard setting? like language?

tested the changes in this PR on the google pixel and no issues either

[waynetee]

The bug occurs when deleting across lines:

video_2025-02-07_01-39-17.mp4

[etrepum]

Using a "Real" Google Pixel device on a Sauce Labs trial I was able to reproduce the bug and confirm that this PR fixes it. The issue with setTimeout is less about performance and more about how complicated things get when these events are not happening in the order you expect them to which often leads to different bugs. I think this is fine for now, it clearly fixes an issue and probably doesn't yet cause other ones, but should be cleaned up in a more systematic way that doesn't require the setTimeout later. I don't have a specific suggestion for exactly what the best fix is without doing a much deeper dive into this part of the code while having a live android device accessible for debugging.

[ThenTech]

I just tested this change on my current setup, where I have DecoratorNodes that render emojis (mentioned in #5042), but the issue still persists on mobile, specifically with the GBoard keyboard. When pressing backspace, the cursor jumps to the front of the node and then back behind the node, but the node itself does not get deleted.

[etrepum]

Please file a new detailed issue with a full repro if you’d like anyone to look at this bug, comments on closed PRs will get lost.