Page MenuHomePhabricator

In long footnote's VESurfaceWidget, hitting backspace scrolls you up to the top of the text field
Closed, ResolvedPublic8 Estimated Story Points

Description

Steps to reproduce:

  1. Go to https://de.wikipedia.org/wiki/Panoramafreiheit?veaction=edit
  2. Click on footnote [59]
  3. Click on "Bearbeiten" ("Edit")
  4. Scroll to the bottom of the resulting text box, remove the whitespace character between "Abbildung" and "einhergehen" using the backspace key

Notice that VE scrolls you up all the way to the top of the text box. (Chrome 51, Win 8)

Event Timeline

Jdforrester-WMF renamed this task from In long footnote, hitting certain keys scrolls you up to the top of the text field to In long footnote's VESurfaceWidget, hitting backspace scrolls you up to the top of the text field.Jul 26 2016, 7:15 PM
Jdforrester-WMF triaged this task as Medium priority.
Jdforrester-WMF moved this task from To Triage to TR3: Language support on the VisualEditor board.

Specifically, I think this is triggered by long lines, and it trying to scroll so they're inside the viewport. You can see this by introducing linebreaks into that 3,492 character footnote, and observing that it scrolls when you edit a line and either its top or bottom is outside the current viewport.

Even more specifically, it's caused by 6d6647925c2c4b55401c7decc03e830c44c0f4e0, which removes and resets the selection on delete, to get rid of any browser pre-annotations. Setting the selection makes it scroll the element into view... and that means the top/bottom of the element needs to be visible, not just the place where the caret is.

Change 304575 had a related patch set uploaded (by DLynch):
ve.ce.Surface: when setting selection just focus if focusTarget == activeElement

https://gerrit.wikimedia.org/r/304575

That patch does the simple thing, and tells it to just set focus if the active element is the focus element, rather than deliberately scrolling to the top/bottom of the element.

Because it's the simple thing, there may be cases where this causes undesired behavior. A more complicated "exactly where is the caret, and should we scroll to that?" test could perhaps be written.

Change 304575 merged by jenkins-bot:
ve.ce.Surface: when setting selection just focus if focusTarget == activeElement

https://gerrit.wikimedia.org/r/304575