8
0
Просмотр исходного кода

Merge pull request #1534 from ckeditor/t/1533

Internal: `MergeOperation#graveyardPosition` was not transformed due to typo. Closes #1533.
Piotr Jasiun 7 лет назад
Родитель
Сommit
73703392a5

+ 2 - 3
packages/ckeditor5-engine/src/model/differ.js

@@ -182,14 +182,13 @@ export default class Differ {
 			}
 			case 'split': {
 				const splitElement = operation.position.parent;
-				const howManyMoved = splitElement.maxOffset - operation.position.offset;
 
 				if ( !this._isInInsertedElement( splitElement ) ) {
-					this._markRemove( splitElement, operation.position.offset, howManyMoved );
+					this._markRemove( splitElement, operation.position.offset, operation.howMany );
 				}
 
 				if ( !this._isInInsertedElement( splitElement.parent ) ) {
-					this._markInsert( splitElement.parent, splitElement.startOffset + 1, 1 );
+					this._markInsert( splitElement.parent, operation.insertionPosition.offset, 1 );
 				}
 
 				break;

+ 14 - 2
packages/ckeditor5-engine/src/model/operation/splitoperation.js

@@ -87,7 +87,13 @@ export default class SplitOperation extends Operation {
 		const path = this.position.path.slice( 0, -1 );
 		path[ path.length - 1 ]++;
 
-		return new Position( this.position.root, path );
+		let pos = new Position( this.position.root, path );
+
+		if ( this.graveyardPosition && this.graveyardPosition.root == pos.root ) {
+			pos = pos._getTransformedByDeletion( this.graveyardPosition, 1 );
+		}
+
+		return pos;
 	}
 
 	/**
@@ -103,7 +109,13 @@ export default class SplitOperation extends Operation {
 		path[ path.length - 1 ]++;
 		path.push( 0 );
 
-		return new Position( this.position.root, path );
+		let pos = new Position( this.position.root, path );
+
+		if ( this.graveyardPosition && this.graveyardPosition.root == pos.root ) {
+			pos = pos._getTransformedByDeletion( this.graveyardPosition, 1 );
+		}
+
+		return pos;
 	}
 
 	/**

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

@@ -1241,7 +1241,7 @@ setTransformation( MergeOperation, MergeOperation, ( a, b, context ) => {
 	// Handle positions in graveyard.
 	// If graveyard positions are same and `a` operation is strong - do not transform.
 	if ( !a.graveyardPosition.isEqual( b.graveyardPosition ) || !context.aIsStrong ) {
-		a.graveyardPosition._getTransformedByInsertion( b.graveyardPosition, 1 );
+		a.graveyardPosition = a.graveyardPosition._getTransformedByMergeOperation( b );
 	}
 
 	return [ a ];
@@ -1475,7 +1475,7 @@ setTransformation( MergeOperation, UnwrapOperation, ( a, b, context ) => {
 	// Handle positions in graveyard.
 	// If graveyard positions are same and `a` operation is strong - do not transform.
 	if ( !a.graveyardPosition.isEqual( b.graveyardPosition ) || !context.aIsStrong ) {
-		a.graveyardPosition._getTransformedByInsertion( b.graveyardPosition, 1 );
+		a.graveyardPosition = a.graveyardPosition._getTransformedByUnwrapOperation( b );
 	}
 
 	return [ a ];
@@ -2205,7 +2205,7 @@ setTransformation( SplitOperation, WrapOperation, ( a, b ) => {
 	// was transformed by `UnwrapOperation` and it was left in the unwrapped node. Now the unwrapped node will be re-used
 	// and we need to fix `howMany` property in the `SplitOperation`.
 	//
-	if ( b.graveyardPosition && a.insertionPosition.isEqual( b.graveyardPosition.getShiftedBy( 1 ) ) ) {
+	if ( b.graveyardPosition && compareArrays( b.graveyardPosition.path, a.position.getParentPath() ) == 'same' ) {
 		a.position = a.position._getCombined( b.graveyardPosition, b.position );
 		a.howMany = a.howMany + b.howMany;
 

+ 16 - 0
packages/ckeditor5-engine/tests/model/operation/transform/merge.js

@@ -30,6 +30,22 @@ describe( 'transform', () => {
 				expectClients( '<paragraph>FooBarAbc</paragraph>' );
 			} );
 
+			it( 'elements into paragraph with undo', () => {
+				john.setData( '<paragraph>Foo</paragraph>[]<paragraph>Bar</paragraph><paragraph>Abc</paragraph>' );
+				kate.setData( '<paragraph>Foo</paragraph><paragraph>Bar</paragraph>[]<paragraph>Abc</paragraph>' );
+
+				john.merge();
+				kate.merge();
+
+				syncClients();
+
+				kate.undo();
+				john.undo();
+
+				syncClients();
+				expectClients( '<paragraph>Foo</paragraph><paragraph>Bar</paragraph><paragraph>Abc</paragraph>' );
+			} );
+
 			it.skip( 'same element with undo', () => {
 				// Unnecessary SplitOperation.
 				john.setData( '<paragraph>Foo</paragraph>[]<paragraph></paragraph>' );

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

@@ -66,6 +66,11 @@ export class Client {
 
 		model.document.selection._setTo( ranges );
 
+		// Purify graveyard so there are no artifact nodes there remaining after setting new data.
+		// Because of those old nodes some tests may pass even though they fail in real scenarios (those tests
+		// involve bringing back elements from graveyard, like wrap or split).
+		model.document.graveyard._removeChildren( 0, model.document.graveyard.childCount );
+
 		this.syncedVersion = this.document.version;
 	}