Explorar o código

Fix: Fixed incorrect Position#_getTransformedByMove for moves that didn't really move any nodes.

Szymon Cofalik %!s(int64=7) %!d(string=hai) anos
pai
achega
02089a30f0

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

@@ -1540,7 +1540,7 @@ setTransformation( SplitOperation, MergeOperation, ( a, b ) => {
 	a.position = a.position._getTransformedByMergeOperation( b );
 	a.position = a.position._getTransformedByMergeOperation( b );
 
 
 	if ( a.graveyardPosition ) {
 	if ( a.graveyardPosition ) {
-		a.graveyardPosition = a.graveyardPosition._getTransformedByInsertion( b.graveyardPosition, 1 );
+		a.graveyardPosition = a.graveyardPosition._getTransformedByMergeOperation( b );
 	}
 	}
 
 
 	return [ a ];
 	return [ a ];

+ 12 - 8
packages/ckeditor5-engine/src/model/position.js

@@ -717,12 +717,17 @@ export default class Position {
 	 * @returns {module:engine/model/position~Position} Transformed position.
 	 * @returns {module:engine/model/position~Position} Transformed position.
 	 */
 	 */
 	_getTransformedByMove( sourcePosition, targetPosition, howMany ) {
 	_getTransformedByMove( sourcePosition, targetPosition, howMany ) {
-		// Moving a range removes nodes from their original position. We acknowledge this by proper transformation.
-		let transformed = this._getTransformedByDeletion( sourcePosition, howMany );
-
-		// Then we update target position, as it could be affected by nodes removal too.
+		// Update target position, as it could be affected by nodes removal.
 		targetPosition = targetPosition._getTransformedByDeletion( sourcePosition, howMany );
 		targetPosition = targetPosition._getTransformedByDeletion( sourcePosition, howMany );
 
 
+		if ( sourcePosition.isEqual( targetPosition ) ) {
+			// If `targetPosition` is equal to `sourcePosition` this isn't really any move. Just return position as it is.
+			return Position.createFromPosition( this );
+		}
+
+		// Moving a range removes nodes from their original position. We acknowledge this by proper transformation.
+		const transformed = this._getTransformedByDeletion( sourcePosition, howMany );
+
 		const isMoved = transformed === null ||
 		const isMoved = transformed === null ||
 			( sourcePosition.isEqual( this ) && this.stickiness == 'toNext' ) ||
 			( sourcePosition.isEqual( this ) && this.stickiness == 'toNext' ) ||
 			( sourcePosition.getShiftedBy( howMany ).isEqual( this ) && this.stickiness == 'toPrevious' );
 			( sourcePosition.getShiftedBy( howMany ).isEqual( this ) && this.stickiness == 'toPrevious' );
@@ -730,14 +735,13 @@ export default class Position {
 		if ( isMoved ) {
 		if ( isMoved ) {
 			// This position is inside moved range (or sticks to it).
 			// This position is inside moved range (or sticks to it).
 			// In this case, we calculate a combination of this position, move source position and target position.
 			// In this case, we calculate a combination of this position, move source position and target position.
-			transformed = this._getCombined( sourcePosition, targetPosition );
+			return this._getCombined( sourcePosition, targetPosition );
 		} else {
 		} else {
 			// This position is not inside a removed range.
 			// This position is not inside a removed range.
+			//
 			// In next step, we simply reflect inserting `howMany` nodes, which might further affect the position.
 			// In next step, we simply reflect inserting `howMany` nodes, which might further affect the position.
-			transformed = transformed._getTransformedByInsertion( targetPosition, howMany );
+			return transformed._getTransformedByInsertion( targetPosition, howMany );
 		}
 		}
-
-		return transformed;
 	}
 	}
 
 
 	/**
 	/**

+ 18 - 0
packages/ckeditor5-engine/tests/model/operation/transform/delete.js

@@ -39,6 +39,24 @@ describe( 'transform', () => {
 
 
 				expectClients( '<paragraph>Fobc</paragraph>' );
 				expectClients( '<paragraph>Fobc</paragraph>' );
 			} );
 			} );
+
+			// https://github.com/ckeditor/ckeditor5-engine/issues/1492
+			it( 'delete same content from a few elements', () => {
+				john.setData( '<paragraph>F[oo</paragraph><paragraph>Bar</paragraph><paragraph>Ab]c</paragraph>' );
+				kate.setData( '<paragraph>F[oo</paragraph><paragraph>Bar</paragraph><paragraph>Ab]c</paragraph>' );
+
+				john.delete();
+				kate.delete();
+
+				syncClients();
+				expectClients( '<paragraph>Fc</paragraph>' );
+
+				john.undo();
+				kate.undo();
+
+				syncClients();
+				expectClients( '<paragraph>Foo</paragraph><paragraph>Bar</paragraph><paragraph>Abc</paragraph>' );
+			} );
 		} );
 		} );
 	} );
 	} );
 } );
 } );