Преглед на файлове

Merge pull request #1544 from ckeditor/t/1543

Internal: `MergeOperation` transformed by an equal `MergeOperation` becomes a `NoOperation` if it is not going to be affected by undoing operation. Closes #1543.
Piotr Jasiun преди 7 години
родител
ревизия
0308d77eb9

+ 20 - 8
packages/ckeditor5-engine/src/model/operation/transform.js

@@ -1216,17 +1216,29 @@ setTransformation( MergeOperation, MergeOperation, ( a, b, context ) => {
 	// Both operations have same source and target positions. So the element already got merged and there is
 	// theoretically nothing to do.
 	//
-	// In this case, keep the source operation in the merged element - in the graveyard - and don't change target position.
-	// Doing this instead of returning `NoOperation` allows for a correct undo later.
-	//
 	if ( a.sourcePosition.isEqual( b.sourcePosition ) && a.targetPosition.isEqual( b.targetPosition ) ) {
-		const path = b.graveyardPosition.path.slice();
-		path.push( 0 );
+		// There are two ways that we can provide a do-nothing operation.
+		//
+		// First is simply a NoOperation instance. We will use it if `b` operation was not undone.
+		//
+		// Second is a merge operation that has the source operation in the merged element - in the graveyard -
+		// same target position and `howMany` equal to `0`. So it is basically merging an empty element from graveyard
+		// which is almost the same as NoOperation.
+		//
+		// This way the merge operation can be later transformed by split operation
+		// to provide correct undo. This will be used if `b` operation was undone (only then it is correct).
+		//
+		if ( !context.bWasUndone ) {
+			return [ new NoOperation( 0 ) ];
+		} else {
+			const path = b.graveyardPosition.path.slice();
+			path.push( 0 );
 
-		a.sourcePosition = new Position( b.graveyardPosition.root, path );
-		a.howMany = 0;
+			a.sourcePosition = new Position( b.graveyardPosition.root, path );
+			a.howMany = 0;
 
-		return [ a ];
+			return [ a ];
+		}
 	}
 
 	// The default case.

+ 19 - 0
packages/ckeditor5-engine/tests/model/operation/transform/split.js

@@ -317,6 +317,25 @@ describe( 'transform', () => {
 				expectClients( '<paragraph>Foo</paragraph>' );
 			} );
 
+			it( 'text in same position, then undo and redo', () => {
+				john.setData( '<paragraph>F[]oo</paragraph>' );
+				kate.setData( '<paragraph>F[]oo</paragraph>' );
+
+				john.split();
+				kate.split();
+
+				syncClients();
+
+				john.undo();
+
+				syncClients();
+
+				kate.undo();
+				kate.redo();
+
+				expectClients( '<paragraph>Foo</paragraph>' );
+			} );
+
 			it( 'text in different path', () => {
 				john.setData( '<paragraph>F[]oo</paragraph><paragraph>Bar</paragraph>' );
 				kate.setData( '<paragraph>Foo</paragraph><paragraph>B[]ar</paragraph>' );

+ 4 - 0
packages/ckeditor5-engine/tests/model/operation/transform/utils.js

@@ -189,6 +189,10 @@ export class Client {
 		this._processExecute( 'undo' );
 	}
 
+	redo() {
+		this._processExecute( 'redo' );
+	}
+
 	_processExecute( commandName, commandArgs ) {
 		const oldVersion = this.document.version;