Page MenuHomePhabricator

Cursoring left next to an about-grouped alien leaves the cursor inside it
Closed, ResolvedPublic1 Estimate Story Points


Open the following test case in VE core:

<p>abc<span rel="ve:Alien" about="m1">ALIEN1</span><span rel="ve:Alien" about="m1">ALIEN2</span><span rel="ve:Alien" about="m1">ALIEN3</span>def</p>

Place the cursor before 'd' and press left, the cursor moves between ALIEN2 and ALIEN3, even though ALIEN1/2/3 is one node.


Related Gerrit Patches:

Event Timeline

Esanders created this task.Oct 2 2015, 11:07 PM
Esanders assigned this task to dchan.
Esanders raised the priority of this task from to Needs Triage.
Esanders updated the task description. (Show Details)
Esanders added a project: VisualEditor.
Esanders added a subscriber: Esanders.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 2 2015, 11:07 PM
Jdforrester-WMF triaged this task as Medium priority.Oct 3 2015, 12:54 AM
Jdforrester-WMF set Security to None.
Jdforrester-WMF moved this task from To Triage to TR3: Language support on the VisualEditor board.
DLynch claimed this task.Oct 19 2015, 5:20 PM
DLynch added a subscriber: dchan.

I think what we need to do is observe if the cursor ends up sandwiched between two DOM nodes which have .data('view') of the same CE node. If that happens we should select the CE node (focus it).

dchan added a comment.Oct 19 2015, 5:32 PM

I think the check and fixup should be in afterDocumentKeyDown, and it should check for isArrow.

The more obvious approach - manually jumping the right distance in onDocumentKeyDown and then doing preventDefault - isn't actually possible, because when you press (say) leftarrow, the correct cursor behaviour may in fact be to jump right, or even to some logically distant place, because of bidirectionality and bidi visual cursoring. We don't believe it's feasible to calculate and emulate this behaviour.

Change 247519 had a related patch set uploaded (by DLynch):
ce.Surface afterKeyDown to better cope with adjacent grouped nodes

Change 247519 merged by jenkins-bot:
ce.Surface afterKeyDown shouldn't assume .contenteditable is boolean