Page MenuHomePhabricator

Unicorn does not get destroyed when arrowing out of a paragraph, causes weird arrow key behavior
Closed, ResolvedPublic8 Story Points

Description

  1. Open <p>abc</p><p>123456789</p> in VE
  2. Put the cursor at the end of the first paragraph, press Ctrl+B and press d. A bolded d appears.
  3. Press right arrow. The cursor jumps to the start of the second paragraph.
  4. Press left arrow. The cursor should jump back to the end of the first paragraph, but it doesn't go anywhere.
  5. Press right arrow a few times. The cursor should move one position to the right for each arrow key press, but it moves two positions per key press instead.
  6. Press left arrow. The cursor should move back one position, but it doesn't move at all.

If you enable input debugging, you'll see at step 3 that the pair of unicorns around d is never destroyed.

Event Timeline

Catrope created this task.Feb 2 2017, 12:33 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 2 2017, 12:33 PM
dchan added a comment.EditedFeb 2 2017, 12:36 PM

It looks like this is due to the ordering in ve.ce.Surface#cleanupUnicorns -- it does this.moveModelCursor( 1 ) and then this.renderSelectedContentBranchNode() . The latter only removes the unicorns if the moveModelCursor didn't change branch node.

Change 335641 had a related patch set uploaded (by Divec):
Check correctly whether the cursor crossed a preunicorn

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

dchan added a comment.Feb 2 2017, 12:56 PM

Change 335641 makes backwards cursoring subject to this bug too, as it should be :-)

Change 335641 merged by jenkins-bot:
Check correctly whether the cursor crossed a preunicorn

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

dchan added a comment.Feb 2 2017, 1:16 PM

I420c14f56187bb881c4b636a746d4383d9ba3146 changed the SurfaceObserver logic so that branch node changes are not always signaled (see https://gerrit.wikimedia.org/r/#/c/241144/15/src/ce/ve.ce.SurfaceObserver.js,sidebyside ), causing this behaviour.

Change 335681 had a related patch set uploaded (by Jforrester):
Update VE core submodule to master (495524f)

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

Change 335681 merged by jenkins-bot:
Update VE core submodule to master (495524f)

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

Jdforrester-WMF triaged this task as High priority.Feb 2 2017, 6:59 PM
Jdforrester-WMF moved this task from To Triage to TR3: Language support on the VisualEditor board.
Jdforrester-WMF set the point value for this task to 8.
Jdforrester-WMF removed a project: Patch-For-Review.

Change 335962 had a related patch set uploaded (by Divec):
Destroy unicorns properly when cursoring to another branch node

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

Change 335962 merged by jenkins-bot:
Destroy unicorns properly when cursoring to another branch node

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

Change 336829 had a related patch set uploaded (by Jforrester):
Update VE core submodule to master (c1884680a)

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

Change 336829 merged by jenkins-bot:
Update VE core submodule to master (c1884680a)

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