Introduce DocumentSelection#change:marker

[scofalik]

📝 Provide a description of the new feature

Hello and welcome to your daily issue with yours truly, me.

Today, I'd like to talk with you about DocumentSelection! At the moment, we have the possibility to listen to DocumentSelection#markers#event:add and event:remove. This looks great but may end up real bad!

If on add or remove we will do something that will re-trigger selection markers updating, we will once again end up in this part of the code: https://github.com/ckeditor/ckeditor5-engine/blob/master/src/model/documentselection.js#L850-L854. What's wrong with that? Well, here's what happens:

1. On initial _updateMarkers we cache markers in an array, let's say [ markerA, markerB ].
2. Then we remove markerA and fire remove event.
3. On remove event we do something, which prompts another _updateMarkers call.
4. In the second _updateMarkers call, since we already removed markerA, markers to remove are [ markerB ]. We remove markerB.
5. Now we return to initial _updateMarkers. We go to the next iteration in the loop, because we cached markerB to be deleted. But it was already deleted! Collection throws an error.

So, we could fix it just right there, instead of Array.from() just iterate through the collection. It should work, I think. However, I think that since we have change:range and change:attribute events, it is natural that we also should have change:marker.

Hence, I propose introducing it.

Now, if we introduce it, we will have to take a look where markers#event:add and markers#event:remove are used. It might be that only in cloud features. Then, we might consider changing markers to a Set instead of a collection and abandon those events. Of course this is a breaking change but hopefully our users either are not using it or could switch to the new way of handling marker changes.

If we don't change from collection to Set we can still fix that for loop.

One thing to note here is that if some piece of code listens to DocumentSelection#event:change (the general one) it will be fired also for marker changes now. I don't expect big problems here but this is something to keep in mind.

[Reinmar]

Hello and welcome to your daily issue with yours truly, me.

Kogo witam, kogo goszczę?