Bug: Text sometimes not updated in Safari since 0.6.4 (regression from #3429)
- Dominant language
- TypeScript
- Stars
- 23.9k
- Forks
- 2.2k
- Avg merge
- 1d 14h
- Merged PRs (30d)
- 55
Description
Lexical version: 0.6.4
Given a playwright test such as the following:
```ts
test.describe('Suite', () => {
test.only('Edits stuff', async ({
page,
}) => {
await page.goto('/page/with/editor');
await page.
.locator('[data-rich-text-input-front] [data-lexical-editor]')
.fill('Front');
await pageOne
.locator('[data-rich-text-input-front] [data-lexical-editor]')
.fill('Front updated');
await expect(
pageOne.locator('[data-rich-text-input-front] [data-lexical-editor]')
).toHaveText('Front updated');
});
});
```
I get the following test failure after updating from 0.6.3 to 0.6.4 in **webkit** only.
```log
Error: expect(received).toHaveText(expected)
Expected string: "Front updated"
Received string: "Front"
```
Having debugged the difference between the two versions I see the following:
Firstly, we call `$shouldPreventDefaultAndInsertText` from `onBeforeInput.updateEditor` with inputType `'insertText'`.
In Chromium in 0.6.3 and 0.6.4 `$shouldPreventDefaultAndInsertText` will return `true` due to the following condition evaluating to `true`:
https://github.com/facebook/lexical/blob/a2f3b2fe157bcd20d69c1e85b9203d6c52d244ab/packages/lexical/src/LexicalEvents.ts#L216-L220
In Chromium, the `domAnchorNode` points to the wrapping `
In Webkit, however, in both 0.6.3 and 0.6.4, the above condition will return `false` since `domAnchorNode` points to text node child of `backingAnchorElement`. Hence none of the conditions in `$shouldPreventDefaultAndInsertText` evaluate to `true` and we _don't_ dispatch a `CONTROLLED_TEXT_INSERTION_COMMAND`.
Since we don't call `preventDefault()` we get a call to `onInput`.
In `onInput` we call `$shouldPreventDefaultAndInsertText` again but _this_ time, when we have an `'input'` event, `domAnchorNode` points to the the `Text` node on the `` (containing "Front updated") back the `backingAnchorElement` points to an orphaned `` whose `Text` node contains just "Front". Since these are different `Text` nodes, the backing anchor element check evaluates to `true` and `$shouldPreventDefaultAndInsertText` returns `true`.
From this point on the behavior differs between 0.6.3 and 0.6.4.
In 0.6.3, we unconditionally dispatch `CONTROLLED_TEXT_INSERTION_COMMAND`:
https://github.com/facebook/lexical/blob/1367743625e8ea79c08d881d4cecbe9195da2fcc/packages/lexical/src/LexicalEvents.ts#L616
However, in 0.6.4, as of #3429, we only dispatch a `CONTROLLED_TEXT_INSERTION_COMMAND` if the text in the DOM appears to differ from the result of splicing the event `data` with the anchor node's text:
https://github.com/facebook/lexical/blob/a2f3b2fe157bcd20d69c1e85b9203d6c52d244ab/packages/lexical/src/LexicalEvents.ts#L692-L706
In the case of this particular test, the combination of `anchorNode.getTextContent()` and `data` matches the text content of `domSelection.anchorNode` so we _don't_ dispatch a `CONTROLLED_TEXT_INSERTION_COMMAND`.
(Specifically, `anchorNode.getTextContent()` is `"Front"`, but the selection encompasses the whole string from 0 to 5 so we end up just considering `event.data` which is `"Front updated"`.)
As a result, `anchorNode` is never updated to match the DOM. And presumably at some later point we clobber the DOM with the value in `anchorNode`.
If I force the condition to evaluate to true and hence dispatch a `CONTROLLED_TEXT_INSERTION_COMMAND` the test passes again.
In Firefox, `beforeinput` uses `inputType` of `'insertCompositionText'` which is ignored by `onBeforeInput` so it also proceeds to `onInput`. However, it will fail the branch `$shouldPreventDefaultAndInsertText` in `onInput` (unlike Safari, `backingAnchorElement` appears to point to the live `` element in Firefox) and proceed to update the selected text from the DOM:
https://github.com/facebook/lexical/blob/a2f3b2fe157bcd20d69c1e85b9203d6c52d244ab/packages/lexical/src/LexicalEvents.ts#L726-L727
Contributor guide
Assessment
This issue has not been assessed yet.