Page MenuHomePhabricator

On mobile, cursoring off a focusable node fails
Closed, ResolvedPublic8 Estimated Story Points

Description

Steps to reproduce:

  1. Use a mobile IME with cursor keys (or just use a desktop browser).
  2. In VE standalone, open demos/ve/mobile.html and edit <h1>foo</h1><div rel='ve:Alien'>bar</div> .
  3. Cursor down to select the alien 'bar'.
  4. Cursor up. An exception is thrown.

The root cause is ve.ce.Surface#onModelSelect calling ve.ce.Surface#preparePasteTargetForCopy, which on mobile deactivates the surface.

The exception is thrown because ve.ce.Surface#afterDocumentKeyDown assumes the selection state must be non-null. If this is fixed, then a second problem arises: after the cursor up completes, the browser is left with a fake (deactivated) cursor instead of what should be a native selection.

Event Timeline

Change 313997 had a related patch set uploaded (by Divec):
Check for null keyDownSelectionState in handlers

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

Change 313997 merged by jenkins-bot:
Check for null keyDownSelectionState in handlers

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

With 313997 merged, the second problem now presents: at step 4, the browser is left with a fake (deactivated) cursor where it should have a native selection. This means that further arrow key presses have no effect.

Change 314032 had a related patch set uploaded (by Divec):
Reactivate mobile surface in onModelSelect for native selections

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

Jdforrester-WMF moved this task from To Triage to Epics on the VisualEditor board.
Jdforrester-WMF set the point value for this task to 8.

This bug is probably OS dependent. I also note that we accidentally broke the code path that deactivates the surface at some point in the past. Somehow on Android the keyboard still correctly hides when I select a focusable node, even though the cursor is placed in the paste target.

If I fix it so that the surface is deactivated, cursoring up and down just causes the page to scroll. This may be an acceptable trade off, as we are trying to optimise the mobile experience for touch input, not keyboard accessibility.

Change 314032 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] Reactivate mobile surface in onModelSelect for native selections

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

Change 454168 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (510506739)

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

Change 454168 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (510506739)

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