WordPress.org

Make WordPress Core

#37690 closed defect (bug) (fixed)

Backspace causes jumping

Reported by: ModernNovel Owned by: azaozz
Milestone: 4.6.1 Priority: normal
Severity: normal Version: 4.6
Component: Editor Keywords: has-patch
Focuses: Cc:

Description

This is a follow-up to #37072. The backspace causing jumping problem did not occur in previous versions but now occurs in 4.6 using Firefox 48.0

Attachments (3)

37690.patch (2.2 KB) - added by azaozz 23 months ago.
37690.1.patch (3.7 KB) - added by azaozz 22 months ago.
37690.2.patch (3.2 KB) - added by azaozz 22 months ago.

Download all attachments as: .zip

Change History (20)

#1 @DrewAPicture
23 months ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 4.6.1

#2 @swissspidy
23 months ago

#37716 was marked as a duplicate.

#3 in reply to: ↑ description @Psychosopher
23 months ago

Replying to ModernNovel:

This is a follow-up to #37072. The backspace causing jumping problem did not occur in previous versions but now occurs in 4.6 using Firefox 48.0

This also happens with Chrome 52, Edge 25, and Opera 39.

#4 @swissspidy
23 months ago

Hmm I've tried to reproduce the problem based on the screencast in #37072 without luck, neither in FIrefox nor Chrome.

Is the problem exactly the same as in the previous ticket or does it behave a bit differently?

#5 @ModernNovel
23 months ago

What happens is either when I backspace or spell-correct, the screen focus jumps to the top of the screen. This is annoying when the text is more than a screen load.

#6 @azaozz
23 months ago

I can reproduce it too. Steps:

  1. Make sure the "Enable full-height editor and distraction-free functionality." in Screen Options is checked.
  2. In the Text editor type or paste about 40-50 lines of text so the editor extends well below the fold.
  3. Scroll down to the last line and press the Backspace or Delete key. At that point the browser always tries to "scroll into view" regardless if the caret is in the viewport or not.

Working on a patch.

@azaozz
23 months ago

#7 follow-up: @azaozz
23 months ago

  • Keywords has-patch needs-testing added; needs-patch removed

37690.patch fixes it here. It brings back cloning of the textarea but uses an actual off-screen textarea element instead of a div, this is fasted and safer. To additionally speed it up that clone is used only when deleting more than one character.

Some more testing will be appreciated :)

#8 in reply to: ↑ 7 ; follow-up: @Akeif
22 months ago

  • Keywords needs-testing removed

Replying to azaozz:

37690.patch fixes it here. It brings back cloning of the textarea but uses an actual off-screen textarea element instead of a div, this is fasted and safer. To additionally speed it up that clone is used only when deleting more than one character.

Some more testing will be appreciated :)

I can reproduce this behavior on Firefox 48.0 on Ubuntu 16.04. The patch fix the problem for me.

Reproduced on git://develop.git.wordpress.org/ on master (4.7-alpha-38178-src) and origin/4.6. Patch tested on origin/4.6.

Screencast pre patch: https://youtu.be/wiaYMZquA10 Screencast post patch: https://youtu.be/dd03WtS6h_M

@azaozz
22 months ago

#9 in reply to: ↑ 8 @azaozz
22 months ago

Replying to Akeif:

Great. Thanks for testing.

I tested with a larger posts too. Unfortunately it is still very slow with posts that have more than 5,000 words. At that point the textarea is starting to get slower anyways, but holding down Backspace makes the browser "freeze" for few seconds.

In 37690.1.patch attempting to speed this up by using Underscore's throttle(). Seems to make it quite faster but has a small trade-off: when deleting empty rows by holding down Backspace, it doesn't reduce the textarea height fast enough. Then it "catches up" at the end.

Alternatively we can turn off the shrinking of the textarea, only resize when switching Visual => Text editor. It looks a bit odd when you delete most of the post, but that is quite rare.

Last edited 22 months ago by azaozz (previous) (diff)

This ticket was mentioned in Slack in #core by jeremyfelt. View the logs.


22 months ago

#11 @jeremyfelt
22 months ago

  • Keywords needs-testing added
  • Owner set to azaozz
  • Status changed from new to assigned

#12 @iseulde
22 months ago

I think delaying reducing the size isn't that bad, especially if we'd consider not doing it at all. Also, I think debounce would work better here than throttle? It will now attempt to reduce the size every half second you're deleting text, and we probably only want to reduce when you're done deleting text, especially if you're continuously hitting backspace on the same line.

@azaozz
22 months ago

#13 @azaozz
22 months ago

In 37690.2.patch: do not use cloned element, "lock" the scrolling while resetting the textarea height instead. This seems fastest and most compatible.

Using debounce() instead of throttle() makes it worse when the user presses backspace or delete only once. We need to run shrinkTextarea() at the beginning and at the end of the delay, debounce() can do either beginning or end but not both.

#14 @azaozz
22 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 38426:

Editor: fix jumpiness on pressing backspace and delete in the Text editor.

Fixes #37690 for trunk.

#15 @azaozz
22 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopen for 4.6.1.

#16 @jeremyfelt
22 months ago

  • Keywords needs-testing removed

[38426] looks good and is working for me in testing. Merging this to the 4.6 branch for RC tomorrow.

#17 @jeremyfelt
22 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 38487:

Editor: fix jumpiness on pressing backspace and delete in the Text editor.

Merge of [38426] to the 4.6 branch.

Props azaozz.
Fixes #37690.

Note: See TracTickets for help on using tickets.