ckeditor
Marker transformation is incorrect in some cases involving merging blocks
📝 Provide detailed reproduction steps (if any)
For such model state:
<blockQuote>
<paragraph>
Foo ba[r
</paragraph>
</blockQuote>
<blockQuote>
<paragraph>
A]bc {xyz
</paragraph>
</blockQuote>}
where [] is a selection and {} is a live range (e.g. suggestion marker), if you press backspace, the marker is gone.
What happens is that during merging, at one point the second <p> is moved to the first block quote. When that happens {} marker is broken into two pieces:
<blockQuote>
<paragraph>
bc {xyz
</paragraph>}{
</blockQuote>}
When evaluating new live range boundaries, we have to choose one piece. Since there can be different model states and operations, sometimes it may make sense to choose first piece, sometimes the second. Since there's no clear winner, we always choose the piece that "stays" (rather the one that "moves"). Unfortunately, in this case, this is {</blockQuote>}, which is empty so it is removed and as a result, the whole suggestion is removed.
This was discovered when using track changes feature. Fortunately, it is very difficult to create a suggestion that ends after a non-widget block element, so this is a rare case. But it may be easier in case of custom plugins, depending on how they insert stuff into the model.
To solve this problem, we could try to make a better decision and e.g. check whether one of the pieces is empty and choose the other one. But we didn't want to do it, because we can't check that during OT (when solving conflicts). Why? In OT you are operating on model states that are not currently existing at the moment. You only have "virtual" paths that you modify, you don't have a model state, so you don't know whether the range is empty or not. For example, in the above scenario, the empty range has boundaries [ 1, 1 ] - [ 2 ] but you cannot know if there are more paragraphs (on positions 1, 1, 1, 2, etc.). You can't figure it out from the paths alone.
We wanted to have same range transformation for OT and for the live range purposes, to make sure that the ranges will not end up differently.
One idea to pursue is that maybe we don't need to have strictly the same transformation for OT and locally. That was desired at some point, to make sure, that locally marker will be transformed exactly the same way as on the remote machine (after remote client gets operations).
But it may not be needed anymore. Why? Because model.Writer re-saves directly affected marker after some action it takes (see model.Writer#_addOperationForAffectedMarkers). This creates a new marker operation which will be sent to the remote client too. So even if the remote client transforms the marker incorrectly, it will still get the "fixing" marker operation. Right now, AFAICS, it is used only for move and merge (which are the "difficult" operations). If necessary, we could expand this to other operations as well.
This idea is I think worth pursuing and probably our only chance to fix this issue (in reasonable amount of time, without bigger overhaul).
This is problematic for GHS as well, as div elements behave same way as block quote in this regard. It is reasonably easy to recreate suggestions that end after the block and may unexpectedly disappear when the block is merged.