Browse Source

Merge pull request #716 from ckeditor/t/715

Fixed: wrong sourcePosition in RemoveOperation when RemoveOperation removes nodes from graveyard.
Piotr Jasiun 9 years ago
parent
commit
fc521cdb17

+ 8 - 1
packages/ckeditor5-engine/src/model/operation/removeoperation.js

@@ -120,7 +120,14 @@ export default class RemoveOperation extends MoveOperation {
 			const graveyard = this.targetPosition.root;
 			const holderElement = new Element( '$graveyardHolder' );
 
-			graveyard.insertChildren( this.targetPosition.path[ 0 ], holderElement );
+			graveyard.insertChildren( this._holderElementOffset, holderElement );
+
+			// If the operation removes nodes that are already in graveyard, it may happen that
+			// the operation's source position is invalidated by inserting new holder element into the graveyard.
+			// If that's the case, we need to fix source position path.
+			if ( this.sourcePosition.root == graveyard && this.sourcePosition.path[ 0 ] >= this._holderElementOffset ) {
+				this.sourcePosition.path[ 0 ]++;
+			}
 		}
 
 		// Then, execute as a move operation.

+ 15 - 0
packages/ckeditor5-engine/tests/model/operation/removeoperation.js

@@ -176,6 +176,21 @@ describe( 'RemoveOperation', () => {
 		expect( root.getChild( 0 ).data ).to.equal( 'bar' );
 	} );
 
+	it( 'should properly remove a node that is already in a graveyard', () => {
+		doc.graveyard.appendChildren( new Element( '$graveyardHolder', {}, [ new Text( 'foo' ) ] ) );
+
+		let position = new Position( doc.graveyard, [ 0, 0 ] );
+		let operation = new RemoveOperation( position, 1, 0 );
+
+		operation.targetPosition.path = [ 0, 0 ];
+
+		doc.applyOperation( wrapInDelta( operation ) );
+
+		expect( doc.graveyard.childCount ).to.equal( 2 );
+		expect( doc.graveyard.getChild( 0 ).getChild( 0 ).data ).to.equal( 'f' );
+		expect( doc.graveyard.getChild( 1 ).getChild( 0 ).data ).to.equal( 'oo' );
+	} );
+
 	describe( 'toJSON', () => {
 		it( 'should create proper json object', () => {
 			const op = new RemoveOperation(