Nested marker highlight breaks mouse text selection

[niegowski]

📝 Provide detailed reproduction steps (if any)

• http://fake.ckeditor.com:8125/ckeditor5-engine/tests/manual/markers.html
• Set markers so that they overlap:

• try selecting text (with the mouse) starting inside the nested marker (do it quickly, don't keep the mouse button pressed in one place):

✔️ Expected result

Selection is not collapsed and where the mouse button was pressed and ends where it was released.

❌ Actual result

Selection is collapsed at the end of the expected selection range.

---

What I discovered while debugging it:

• The selectionchange DOM event is not providing a proper range, it's already collapsed at this point
• The DOM structure is split in a crazy way if the caret is inside the nested highlight:

While with caret outside the marker looks like this:

I suppose that the problem might be caused by the replacement of the DOM nodes on the first selection change that causes this issue (the original node is removed and replaced with other nodes).

📃 Other details

• Browser: Chrome

---

If you'd like to see this fixed sooner, add a 👍 reaction to this post.

[Reinmar]

In the past, we were hoping to stop rendering while the selection is being made. This would solve a range of problems when we change the DOM (e.g. when you start selecting from the end of a link).

[Reinmar]

The second option is to work on this specific case only – to make sure that converting markers applies fewer changes to the tree.

[niegowski]

After debugging the issue - there were 2 problems:

1. The DOM structure was changed so much that the browser couldn't find the proper selection range.
2. The DOM selection was updated while dragging to select a range and that caused a browser to stop expanding the selection.

Steps to fix it:

1. In the SelectionObserver:
   1. Fire a selectionChangeStart on domDocument selectstart event.
   2. Fire a selectionChangeEnd on domDocument mouseup event (and if it started before). This is safe because this event is dispatched even with the mouse on another screen (because the mouse button was kept pressed).
2. In the EditingController:
   1. Disable downcastDispatcher.convertSelection() calls while in the block indicated by the above events.
   2. Trigger downcastDispatcher.convertSelection() on selectionChangeEnd - it's safe now to update the DOM structure and the selection itself.
3. Disable renderer._updateSelection() while in the above selection block to not update DOM selection while changing it.

[niegowski]

After further debugging and PoCing:

1. DOM selection is very fragile to changes via API while selecting (it sometimes is able to recover it) so we should avoid changing it through API calls while selecting
2. DOM selection is very fragile to changes in DOM structure (it sometimes can recover it but in most cases it fails, probably because of non-visible characters)
3. The DOM structure mentioned in the issue (broken nested marker span) is a bug of ModelConsumable that normalizes the name of the event to consume and does it to eager and loses details about the exact marker change (addMarker:comment:some-id -> addMarker:comment) so only the first marker is downcasted while downcasting selection.
4. The FocusObserver is calling forceRender() after a timeout causing the DOM selection to get changes by API call in an unexpected moment and causing the selection process to break.

Case 1 is already addressed by avoiding updating the DOM selection if it's in the desired position (it sometimes is violated by forceRender() or a down-cast of the selection over the changed DOM tree).

Cases 3 and 4 can be easily fixed, but case 2 is difficult to fix. There are some ideas:

• disabling down-casting while selection is being made (with temporary short-circuiting the selection change to SelectionObserver -> view.Selection -> DOM (instead of SelectionObserver -> model.Selection -> view.Selection -> DOM) this solution seems very risky since there could be major changes in the model that are not down-casted while selection is being made and down-casted after the selection changing is done. PoC is here: https://github.com/ckeditor/ckeditor5/compare/ck/9513-disabled-downcast-while-selecting
• changing how nested markers are downcasted: (not verified if would fix the problem)

from:

<span data-comment="thread-1">
	123
	<span data-comment="thread-2">
		456
	</span>
	789
</span>

to:

<span data-comment="thread-1">
	123
</span>
<span data-comment="thread-1,thread-2">
	456
</span>
<span data-comment="thread-1">
	789
</span>

so the active marker (with higher priority won't change the DOM structure (only update the style classes).