lexical: Bug: Text sometimes not updated in Safari since 0.6.4 (regression from #3429)

Lexical version: 0.6.4

Given a playwright test such as the following:

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.

    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 <div> element and hence will not match the text node for the backingAnchorElement (which is the child <span> element). As a result we will call preventDefault() and dispatch a CONTROLLED_TEXT_INSERTION_COMMAND.

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 <span> (containing “Front updated”) back the backingAnchorElement points to an orphaned <span> 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 <span> 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

About this issue

  • Original URL
  • State: open
  • Created 2 years ago
  • Comments: 16 (9 by maintainers)

Commits related to this issue

Most upvoted comments

Sure, I’ll have a look tomorrow.