WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.52k stars 4.21k forks source link

Changing text in a paragraph block to bold doesn't work properly. #33712

Open richward111 opened 3 years ago

richward111 commented 3 years ago

Description

When I highlight text in a paragraph block and press B for bold, only part of the text goes bold. When I try again, a different part of the text goes bold. When I look at the HTML view, the 'strong' tags are randomly entered within the text rather than at the start and end of the text I highlighted.

Edit: this seems to have most consistently if, when you are making a selection, you trigger the visual "border" around the block. See here for instructions.

Step-by-step reproduction instructions

Note: consistent steps to reproduce are available here https://github.com/WordPress/gutenberg/issues/33712#issuecomment-901520115

Steps to reproduce error:

  1. Type a line of text in a paragraph block.
  2. Highlight the text with a cursor.
  3. Select B in the floating block editor to make the highlighted text bold.
  4. Only part of the highlighted text changes to bold. The number of characters changed to bold is not consistent.

Expected behaviour

The highlighted text should change to bold. In the HTML view, the 'strong' tags will be at the start and end of the highlighted text.

Actual behaviour

Only part of the highlighted text changes to bold. In the HTML view, the 'strong' tags are inserted within the highlighted text and not at the start and end.

Screenshots or screen recording (optional)

https://gph.is/g/EJXjnRV

Code snippet (optional)

WordPress information

Device information

richward111 commented 3 years ago

I have also tested by reverting to 5.7.2 (not a backup) and the problem disappears. I therefore believe this to be a bug in the 5.8 update.

skorasaurus commented 3 years ago

Thanks for your detailed report and gif.

I'm able to intermittently reproduce this on 5.8.0; with Gutenberg 11.1.0 activated... sometimes it worked as expected, other times, it did not.

Additionally, I was only able to reproduce this if you select all of the text in the paragraph before 'boldening' it.

ehti commented 3 years ago

Was able to reproduce it once - couldn't do it again in a small number of tries that I made.

Like mentioned above, it happens but intermittently.

richward111 commented 3 years ago

Thank you for acknowledging this issue. Is a fix likely to be scheduled in 5.8.1? The issue happens consistently for me on all installations, and not just when changing text to bold, but also to italic too. I would have thought changing text style was a common editing process and am surprised it hasn't been reported by others.

jerrysarcastic commented 3 years ago

I've been able to replicate this issue pretty predictably, but it does not happen every time. In my tests if I highlight text, but continue too far to the left and into the "whitespace" of the editor (e.g. selecting the block and some area outside the block) it will sometimes fail to format all the text, but instead only formats a portion. It takes a few tries but you can see it in this video at the 20 second mark:

https://d.pr/v/9xKqLj

If I select the text carefully, so my cursor does not leave the block, there is no issue.

WP: 5.8 Gutenberg plugin: 11.3.0 OS: Mac 10.15.4 Browser: Chrome 92.0.4515.131

Testing based on this report in the WordPress.com forums which is also running on WP 5.8. User in the posting there notes they have the issue in other browsers as well.

donalirl commented 3 years ago

I can recreate this with absolute consistency:

When you are highlighting text, if you trigger the border box around the paragraph block, it seems to confuse the selection and cause the wrong text to be bolded/linked. Video:

https://user-images.githubusercontent.com/30727666/129989367-2df9a656-5998-405d-a981-f54d66df2912.mp4

If you take the same action, being careful not to trigger the border box around the paragraph block, only the highlighted text is formatted (correct behavior):

https://user-images.githubusercontent.com/30727666/129989383-2a97e0e6-cb4b-4c21-bdd2-ad75e3b23a28.mp4

It's also worth noting that this happens with not just bold text, but italics, links, and other formatting options. The title of this issue could be updated to reflect this.

mreishus commented 3 years ago

In my testing, 1929e283c15af96e5cc94797fe04e4b232c04ca5 is the first commit with the bug. I am still unsure of the exact mechanism. Enough changes have happened since then that the commit no longer reverts cleanly on the current code.

mreishus commented 3 years ago

I've discovered the mechanism here, but I don't have a fix. I've been examining these two commits:

The main purpose of 1929e283c15a is to extract code from rich text's index.js out to a new file, use-input-and-selection.js. It doesn't aim to change behavior. It actually fixes a subtle bug which ends up causing this issue.

Of particular importance is the handleSelectionChange() function. It exists in both commits, and goes through a number of checks at the beginning of the function, which might cause it to return early. If it makes it to the bottom of the function, it runs onSelectionChange( start, end );, which is key. In the bug-free version, this is called as we drag out of the box to extend our selection to the end. In the bugged version, it is not called here.

I have videos illustrating this, by showing the selections happening with console logging. Here is the logging I've added: 2021-09-17_14-21

https://user-images.githubusercontent.com/937354/133842902-67202284-6496-467d-ab18-e63c3cafbfda.mp4

^ In the bug-free version, we see "It is important 0 216" as I drag to the bottom line, then as I drag out of the box, we see "It is important 0 245".

https://user-images.githubusercontent.com/937354/133843023-2010d11a-e1f4-4a98-be15-4b93712bcd3a.mp4

^ In the bug version, we see "It is important 0 220" as I drag to the bottom line, then as I drag out of the box, we do not see "It is important 0 245". The handleSelectionChange() function ran just as before, but this time it exits early, never makes it to the end of the function, and never calls onSelectionChange(0, 245) which is key.

The same order of operations is happening in both cases:

Here are videos trying to illustrate this, I hope they're not too confusing:

https://user-images.githubusercontent.com/937354/133844785-e2a80f45-0084-427a-b4d2-5215e336a90c.mp4

^ Bug Free - After RichText rerenders with props.disabled=true , handleSelectionChange() runs and makes it to the end where we see the "it is important" debug line

https://user-images.githubusercontent.com/937354/133845041-0016a6cb-88e3-4c2c-8394-23a424feadf9.mp4

^ With Bug - After RichText rerenders with props.disabled=true , handleSelectionChange() runs and exits early, since it finds that the component is disabled

The key difference is the disabled check across the two versions:

How to fix this, I'm not sure of. The version with the bug seems in the abstract to be more correct behavior, by reading the current value of disabled. And not updating the selection changes of disabled blocks seems like a useful optimization to keep around that shouldn't be removed. I'm currently not sure what to do. Maybe if we could update the props.disabled slightly later, after the event handler has run somehow - open to other approaches as well. I'll tag @ellatrix who is undoubtedly the most familiar with this area. :)

mreishus commented 3 years ago

Since it's a selection bug, it can occur with anything that operates on selections, not just bolding text. For example, links:

https://user-images.githubusercontent.com/937354/138152502-11e1d83b-528e-45a2-8483-0f0d55f1133c.mp4

HongPong commented 3 years ago

This bug has been happening when dragging text to create links, I believe it is probably the same bug. As well as Italics. Thank you for your attention to this serious issue.

DrizzlyOwl commented 2 years ago

Thanks for the thorough report here. I think #37730 is also related to this.