Using RemoveDelta in deleteContent leads to wrong results in collaboration

[scofalik]

In deleteContent we make an assumption, that if the merged element is empty, maybe it is better to use batch.remove() instead of merging it.

It is all fine, until you realise that it is incorrect to take action basing on current model state. Here's a counter example:

1. Editor data is: <p>foo</p><p></p><p>bar</p>.
2. Second (empty) paragraph is "merged" with first paragraph.
3. Simultaneously, third paragraph ("bar") is merged with second paragraph.

If second paragraph is removed (instead of merged), then the data from third (merged) paragraph will be "moved" to the second paragraph and removed with it.

[scofalik]

Alternatively this problem probably could be solved in MergeDelta x RemoveDelta special case transformation -> "if left-side merge element was removed, merge with next left-side element". Although I am not sure if this will make sense in all scenarios.

[scofalik]

Solving this in transformations may be even better, considering this code in deleteContent:

// Removes empty end ancestors:
// <a>fo[o</a><b><a><c>bar]</c></a></b>
// becomes:
// <a>fo[]</a><b><a>{}</a></b>
// So we can remove <a> and <b>.
while ( endPos.parent.isEmpty ) {
	const parentToRemove = endPos.parent;

	endPos = Position.createBefore( parentToRemove );

	batch.remove( parentToRemove );
}

🤔

[scofalik]

Because of https://github.com/ckeditor/ckeditor5-engine/issues/1162#issuecomment-335477136 we cannot introduce this fix in deltas transformations. So I will push the fix with the original idea. Unfortunately, it won't work if empty parents will get removed and I am not really sure whether we should use batch.merge there (see the code snippet in the post above).

We may revisit this problem once again if we drop the deltas idea, as described in the linked issue.