Page MenuHomePhabricator

contextChange event not emitted when part of range goes from collapsed to not collapsed
Closed, ResolvedPublic

Description

In ve.dm.SurfaceFragment#getSelectedLeafNodes we collected only leaf nodes that have a non-empty selection. That means that methods like getSelectedLeafNodes will give a different result if we go from having a collapsed selection in one of the leaf nodes, to an uncollapsed selection. However ve.dm.Selection#setSelection does not emit a context change when this happens, so the previous result sticks.

Example:
Create a document:

* One
Two
  • Select from the middle of "One" to the left of the "T".
  • The only "selected" leaf node is "One", so the VE toolbar shows that the whole selection is a bullet list, and the bullet list button is selected
  • Now with shift held, click in the middle of the word "Two" to grow the selection.
  • Both "One" and "Two" are now "selected" leaf nodes, so VE should recognised the selection only partially contains a bullet list, and the bullet list button should not be selected

However because of this bug, the previous result is used, and the bullet list button remains selected. This means when it is clicked "One" is de-indented, instead of "Two" being indented.

Event Timeline

Change 785859 had a related patch set uploaded (by Esanders; author: Esanders):

[VisualEditor/VisualEditor@master] ve.dm.Surface: Emit context change when selection in node changes collapsed-ness

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

Change 785859 merged by jenkins-bot:

[VisualEditor/VisualEditor@master] ve.dm.Surface: Emit context change when selection in node changes collapsed-ness

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

Change 790452 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (e54ef8d1c)

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

Change 790452 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (e54ef8d1c)

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

matmarex moved this task from To Triage to Triaged on the VisualEditor board.
matmarex edited projects, added Editing QA; removed Patch-For-Review.
matmarex moved this task from Incoming to QA on the Editing-team (Kanban Board) board.

Event properly emitted as expected.

See https://photos.app.goo.gl/BXoHiExVbBY3pSyg9 for bullet list

Screenshot 2022-05-10 at 22.48.41.png (840×1 px, 118 KB)
for numbered list

ppelberg claimed this task.