model.insertContent() does not insert markers (added in a document fragment)
📝 Provide a description of the improvement
When model.DocumentFragment is inserted to the model using model.insertContent(), its children are inserted but markers saved in model.DocumentFragment#markers are skipped. They should be inserted as well.
Most probably here: https://github.com/ckeditor/ckeditor5/blob/master/packages/ckeditor5-engine/src/model/utils/insertcontent.ts#L90
The difficulty in this task is that we need to re-calculate the markers ranges. Additional difficulty is introduced by the fact that model.insertContent() may heavily affect the model, so re-calculation would be different depending on how some elements are split, merged, etc. I don't think that just "using math" will be enough.
One idea to simplify this calculations that I have is to add temporary, fake elements to the document fragment at the positions were marker boundaries are. Then, those elements would be inserted to the model together with regular children. After this is done, fake elements positions would be saved, the elements would be removed and markers would be added instead. I am not sure this method would be 100% accurate in all cases (e.g. when markers are at the beginning/end of the document fragment, or when the insertion causes massive splitting and merging in the document model).
The other idea is to handle markers in insertion.handleNodes() (maybe change the method name). Markers could be passed as second parameter and then processed in Insertion class as the nodes are inserted to the model. This way we could very precisely calculate new positions for markers boundaries.
I think the second idea would be more precise but it is probably tougher to implement. We can do it in two steps, first use fake nodes and then implement better solution if needed.
Another thing is that I wonder whether we should put it behind some kind of a flag for backward compatibility but I am leaning towards "no". At the moment, the markers are "silently dropped", so, yeah, we will introduce backward incompatibility. But will it be problematic?
- In most cases you don't have any markers in the document fragment (or you don't even insert a document fragment). We noticed this problem only now, after long time and it was probably never raised by our users.
- In cut/copy+paste, markers are removed earlier (probably during copy) and they are not available inside
insertContent(). So this won't be a problem. - Internally, we use
insertContent()in a couple of places. We can check if there's a possibility that in those places document fragment with marker might be inserted. - If the user has problems, the fix is very easy (just remove markers from document fragment before inserting it).
- The fact that markers are skipped can be treated as a bug, actually. We can treat this as fixing incorrect behavior of
insertContent(). If your document fragment has markers and you don't want them, remove them.
So, I propose no flag and marking this as minor breaking change + adding simple migration guide.