Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Selecting text by triple clicking also selects the following top level nodes, up to and including the next text node. #2660

Closed
AaronGazzola opened this issue Jul 20, 2022 · 6 comments
Assignees
Labels
all-platforms-bug core Reconciler, DOM, Selection, Node, Events, Composition

Comments

@AaronGazzola
Copy link

Triple clicking a text node in the lexical playground selects the parent node and the following top level nodes up to and including the next text node.

The following image shows the resulting editor state after triple clicking the first paragraph:
Editor state after triple-clicking a paragraph node that is followed by a decorator node and a paragraph node

This causes a breaking bug where the page becomes unresponsive if the block format drop down menu is used to format content that was selected this way, if the content contains a decorator node.

The following image is the unresponsive playground after triple-clicking the text above a YouTube node and then using the dropdown block format menu to format the content as heading 3:
Unresponsive playground

Lexical version: 0.3.7

Steps To Reproduce

  1. Insert a top level decorator node into the lexical playground, such as a YouTube node
  2. Create paragraph nodes with text content above and below the top level decorator node
  3. Select the text in the first paragraph node by triple clicking (the tree view plugin should show that the bottom paragraph node is also selected, while the text in the editor is not highlighted)
  4. Use the dropdown block format menu to change the format to any of the options, eg. Heading 1

Link to code example: https://playground.lexical.dev/

The current behavior

Selecting text by triple clicking also selects the following top level nodes, up to and including the next text node.

The expected behavior

When text is selected by triple clicking, only the clicked node should be selected

@thegreatercurve
Copy link
Contributor

I was able to replicate this bug a few days ago, but since the latest release this seems to have been fixed now. I am able to apply block styles even when the selection contains a decorator node without crashing the editor.

@acywatson
Copy link
Contributor

acywatson commented Jul 22, 2022

I was able to replicate this bug a few days ago, but since the latest release this seems to have been fixed now. I am able to apply block styles even when the selection contains a decorator node without crashing the editor.

I'm still not sure this is behaving correctly - triple click behavior varies amongst browsers, but it doesn't seem like we should be selecting everything between the triple-clicked line and the next text node. For instance, if you do this and then hit delete, you also delete the DecoratorNode.

More info:

#753

I don't know if this can really be called broken, as the Decorator is indicated as selected. I just don't think most people expect triple-click to behave this way. Same issue I raised in #753. Take a look at the Chromium bug in that PR description.

@AaronGazzola
Copy link
Author

I was able to replicate this bug a few days ago, but since the latest release this seems to have been fixed now. I am able to apply block styles even when the selection contains a decorator node without crashing the editor.

In Chrome and Safari in the latest lexical playground, triple clicking a text node selects both the top level parent of the clicked text node and the top level parent of the next text node. However the selection is only highlighted for the text node that was clicked. Applying block level styling in this state results in both parent nodes being styled.

@AaronGazzola
Copy link
Author

I was able to replicate this bug a few days ago, but since the latest release this seems to have been fixed now. I am able to apply block styles even when the selection contains a decorator node without crashing the editor.

The editor no longer freezes but the steps described above still result in strange behavior. Below are screen shots from chrome in the lexical playground. You can see the highlighting/selection issue mentioned above in the first image, then applying block styling in that state results in all the following nodes nesting within the styled block.

image
image

@acywatson acywatson reopened this Jul 23, 2022
@acywatson
Copy link
Contributor

More reading on this chromium bug led me to the a w3c Selection API spec discussion:

w3c/selection-api#70

Which led me to CKEditor also dealing with this exact same issue a while back:

ckeditor/ckeditor4#484
ckeditor/ckeditor4#3118

It looks like they solved this problem by just normalizing selection so that, except for some special cases, it just can't be focused on an element node/on the zero offset of a text node. We might be able to take an approach like that.

In the past, we have taken the position that this is an "upstream issue" or a "browser quirk" and that we should fix this type of thing by changing how our library handles applying changes (formatting, specifically) to a selected range. This case sort of shows how that approach falls short. I think one of most useful things about Lexical is that we try to make weird quirks like this implementation details of the library, so I'm inclined to say we should fix this universally with a similar Selection normalization approach.

Trade-off will be that some different, but I think very uncommon edge case behaviors, may not work as expected.

@husseinraoouf
Copy link
Contributor

@acywatson checking on the status of this issue

@zurfyx zurfyx added all-platforms-bug core Reconciler, DOM, Selection, Node, Events, Composition labels Apr 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
all-platforms-bug core Reconciler, DOM, Selection, Node, Events, Composition
Projects
None yet
Development

No branches or pull requests

5 participants