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

Text highlighting and setting marks is broken #3923

Closed
mr-remington opened this issue Oct 15, 2020 · 7 comments
Closed

Text highlighting and setting marks is broken #3923

mr-remington opened this issue Oct 15, 2020 · 7 comments
Assignees
Labels

Comments

@mr-remington
Copy link

Do you want to request a feature or report a bug?

Bug

What's the current behavior?

If you double click on a word, set "Bold", "Italic", or any other mark. Navigate away to another character and then go back to the marked word with double click, and undo the prior mark set, it will unset the mark but enable the mark for surrounding marks. Reproduce:

  1. Go to: https://www.slatejs.org/examples/richtext
  2. Double click "things" in the second paragraph; highlighting the word and click "B"(Bold)
  3. Arrow away clearing the highlight
  4. Double click "things" again.. highlighting it. The mark button is no longer highlighted despite it being bold.
  5. Click Bold and the entire sentence is highlighted bold.
    (Sorry I dont know how y'all are creating these fancy gif screens)

Slate: Whatever https://www.slatejs.org/examples/richtext is currently running
Browser Firefox Developer Edition (82.0b8 (64-bit))
OS: Mac 11.0 Beta

What's the expected behavior?

Highlighting the word should correctly set the mark, toggling the button should likewise work

@vitorrd
Copy link

vitorrd commented Oct 21, 2020

Couldn't reproduce it on Windows on Chrome 86.0.4240.75.

@IsaacMSchultz
Copy link
Contributor

IsaacMSchultz commented Oct 22, 2020

I was able to reproduce this issue. Digging into the code, it seems it is related to the way slate selects what DOM nodes the user is highlighting using window.getSelection().

The bug

I was able to discover that after making a mark change, the selection is properly updated by examining the anchor and focus nodes of window.getSelection():
image
Also note that the offsets count the correct number of characters after the anchor node.

But when the text is re-selected, the anchor node is set to the previous node (which I believe is correct), but the focus node is set to the node that comes after the actual selected node, and thus the offset of the focus node is 0, which is the case that is causing the bug.
image

I also notice that this does not cause the bug when there is a selection between nodes:
image
Which means there is simply a bug when selecting the entirety of a single node with siblings.

Solution

I believe this is fixable by checking for this case in the toSlateRange() function. I will start working on a solution for it.

@BrentFarese
Copy link
Collaborator

Is this issue caused by #3756? I am not sure this is a selection bug. Perhaps it is a slate-history bug with how the history resolves the selection (in this case, it might be incorrect which causes the formatting bug)?

@marsprince
Copy link

Only appeared in FireFox, Chrome and Safari are all OK

@BrentFarese
Copy link
Collaborator

Related to #4038

@BrentFarese BrentFarese added the mlh label Feb 5, 2021
@ridhambhat
Copy link
Contributor

Hey Brent, please assign this issue to me.

@BrentFarese
Copy link
Collaborator

Fixed by #4168.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
6 participants