Merge pull request #1163 from ckeditor/t/1161

Fix: The `deleteContent()` algorithm will use merging to "remove" empty element which will ensure a better conflict resolution on collaborative editing. Closes #1161.

src/controller/deletecontent.js

@@ -123,27 +123,20 @@ function mergeBranches( batch, startPos, endPos ) {
 	startPos = Position.createAfter( startParent );
 	endPos = Position.createBefore( endParent );
 
-	if ( endParent.isEmpty ) {
-		batch.remove( endParent );
-	} else {
-		// At the moment, next startPos is also the position to which the endParent
-		// needs to be moved:
+	if ( !endPos.isEqual( startPos ) ) {
+		// In this case, before we merge, we need to move `endParent` to the `startPos`:
 		// <a><b>x[]</b></a><c><d>{}y</d></c>
 		// becomes:
 		// <a><b>x</b>[]<d>y</d></a><c>{}</c>
-
-		// Move the end parent only if needed.
-		// E.g. not in this case: <p>ab</p>[]{}<p>cd</p>
-		if ( !endPos.isEqual( startPos ) ) {
-			batch.move( endParent, startPos );
-		}
-
-		// To then become:
-		// <a><b>xy</b>[]</a><c>{}</c>
-		batch.merge( startPos );
+		batch.move( endParent, startPos );
 	}
 
-	// Removes empty end ancestors:
+	// Merge two siblings:
+	// <a>x</a>[]<b>y</b> -> <a>xy</a> (the usual case)
+	// <a><b>x</b>[]<d>y</d></a><c></c> -> <a><b>xy</b>[]</a><c></c> (this is the "move parent" case shown above)
+	batch.merge( startPos );
+
+	// Remove empty end ancestors:
 	// <a>fo[o</a><b><a><c>bar]</c></a></b>
 	// becomes:
 	// <a>fo[]</a><b><a>{}</a></b>

tests/controller/deletecontent.js

@@ -233,19 +233,30 @@ describe( 'DataController', () => {
 				'<heading1>[]bar</heading1>'
 			);
 
-			it( 'uses remove delta instead of merge delta if merged element is empty', () => {
+			it( 'uses merge delta even if merged element is empty', () => {
 				setData( doc, '<paragraph>ab[cd</paragraph><paragraph>efgh]</paragraph>' );
 
 				const batch = doc.batch();
 				const spyMerge = sinon.spy( batch, 'merge' );
-				const spyRemove = sinon.spy( batch, 'remove' );
 
 				deleteContent( doc.selection, batch );
 
 				expect( getData( doc ) ).to.equal( '<paragraph>ab[]</paragraph>' );
 
-				expect( spyMerge.called ).to.be.false;
-				expect( spyRemove.called ).to.be.true;
+				expect( spyMerge.called ).to.be.true;
+			} );
+
+			it( 'uses merge delta even if merged element is empty #2', () => {
+				setData( doc, '<paragraph>ab[</paragraph><paragraph>]</paragraph>' );
+
+				const batch = doc.batch();
+				const spyMerge = sinon.spy( batch, 'merge' );
+
+				deleteContent( doc.selection, batch );
+
+				expect( getData( doc ) ).to.equal( '<paragraph>ab[]</paragraph>' );
+
+				expect( spyMerge.called ).to.be.true;
 			} );
 
 			it( 'does not try to move the second block if not needed', () => {