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

Fixes in MoveOperation x MoveOperation transformation.

Szymon Cofalik 9 лет назад
Родитель
Сommit
e2cd698e84

+ 11 - 11
packages/ckeditor5-engine/src/treemodel/operation/transform.js

@@ -252,8 +252,6 @@ const ot = {
 				isStrong = false;
 			}
 
-			let isSticky = a.isSticky && b.isSticky;
-
 			// Create ranges from MoveOperations properties.
 			const rangeA = Range.createFromPositionAndShift( a.sourcePosition, a.howMany );
 			const rangeB = Range.createFromPositionAndShift( b.sourcePosition, b.howMany );
@@ -268,8 +266,8 @@ const ot = {
 			let difference = joinRanges( rangeA.getDifference( rangeB ) );
 
 			if ( difference ) {
-				difference.start = difference.start.getTransformedByMove( b.sourcePosition, moveTargetPosition, b.howMany, !isSticky, false );
-				difference.end = difference.end.getTransformedByMove( b.sourcePosition, moveTargetPosition, b.howMany, isSticky, false );
+				difference.start = difference.start.getTransformedByMove( b.sourcePosition, moveTargetPosition, b.howMany, !a.isSticky, false );
+				difference.end = difference.end.getTransformedByMove( b.sourcePosition, moveTargetPosition, b.howMany, a.isSticky, false );
 
 				ranges.push( difference );
 			}
@@ -281,14 +279,16 @@ const ot = {
 			// * on the same tree level - it means that we move the same nodes into different places
 			// * on deeper tree level - it means that we move nodes that are inside moved nodes
 			// The operations are conflicting only if they try to move exactly same nodes, so only in the first case.
-			// So, we will handle common range if it is "deeper" or if transformed operation is more important.
-			let isDeeper = utils.compareArrays( b.sourcePosition.getParentPath(), a.sourcePosition.getParentPath() ) == 'PREFIX';
+			// That means that we transform common part in two cases:
+			// * `rangeA` is "deeper" than `rangeB` so it does not collide
+			// * `rangeA` is at the same level but is stronger than `rangeB`.
+			let aCompB = utils.compareArrays( a.sourcePosition.getParentPath(), b.sourcePosition.getParentPath() );
 
 			// If the `b` MoveOperation points inside the `a` MoveOperation range, the common part will be included in
 			// range(s) that (is) are results of processing `difference`. If that's the case, we cannot include it again.
-			let bIsIncluded = rangeA.containsPosition( b.targetPosition ) ||
-				( rangeA.start.isEqual( b.targetPosition ) && isSticky ) ||
-				( rangeA.end.isEqual( b.targetPosition ) && isSticky );
+			let bTargetsToA = rangeA.containsPosition( b.targetPosition ) ||
+				( rangeA.start.isEqual( b.targetPosition ) && a.isSticky ) ||
+				( rangeA.end.isEqual( b.targetPosition ) && a.isSticky );
 
 			// If the `b` MoveOperation range contains both whole `a` range and target position we do an exception and
 			// transform `a` operation. Normally, when same nodes are moved, we stick with stronger operation's target.
@@ -296,7 +296,7 @@ const ot = {
 			// smaller range will be moved to larger range target. The effect of this transformation feels natural.
 			let aIsInside = rangeB.containsRange( rangeA ) && rangeB.containsPosition( a.targetPosition );
 
-			if ( common !== null && ( isDeeper || isStrong || aIsInside ) && !bIsIncluded ) {
+			if ( common !== null && ( aCompB === 'EXTENSION' || ( aCompB === 'SAME' && isStrong ) || aIsInside ) && !bTargetsToA ) {
 				// Here we do not need to worry that newTargetPosition is inside moved range, because that
 				// would mean that the MoveOperation targets into itself, and that is incorrect operation.
 				// Instead, we calculate the new position of that part of original range.
@@ -318,7 +318,7 @@ const ot = {
 			}
 
 			// Target position also could be affected by the other MoveOperation. We will transform it.
-			let newTargetPosition = a.targetPosition.getTransformedByMove( b.sourcePosition, moveTargetPosition, b.howMany, !isStrong, isSticky );
+			let newTargetPosition = a.targetPosition.getTransformedByMove( b.sourcePosition, moveTargetPosition, b.howMany, !isStrong, b.isSticky );
 
 			// Map transformed range(s) to operations and return them.
 			return ranges.reverse().map( ( range ) => {

+ 53 - 3
packages/ckeditor5-engine/tests/treemodel/operation/transform.js

@@ -1713,7 +1713,7 @@ describe( 'transform', () => {
 				expectOperation( transOp[ 0 ], expected );
 			} );
 
-			it( 'target at offset same as range end boundary: expand range', () => {
+			it( 'target at offset same as range end boundary: no operation update', () => {
 				let transformBy = new InsertOperation(
 					new Position( root, [ 2, 2, 6 ] ),
 					[ nodeA, nodeB ],
@@ -1722,6 +1722,21 @@ describe( 'transform', () => {
 
 				let transOp = transform( op, transformBy );
 
+				expect( transOp.length ).to.equal( 1 );
+				expectOperation( transOp[ 0 ], expected );
+			} );
+
+			it( 'target at offset same as range end boundary (sticky move): expand range', () => {
+				let transformBy = new InsertOperation(
+					new Position( root, [ 2, 2, 6 ] ),
+					[ nodeA, nodeB ],
+					baseVersion
+				);
+
+				op.isSticky = true;
+
+				let transOp = transform( op, transformBy );
+
 				expected.howMany = 4;
 
 				expect( transOp.length ).to.equal( 1 );
@@ -2035,7 +2050,7 @@ describe( 'transform', () => {
 				expectOperation( transOp[ 0 ], expected );
 			} );
 
-			it( 'target at start boundary of transforming move range: expand move range', () => {
+			it( 'target at start boundary of transforming move range: increment source offset', () => {
 				let transformBy = new MoveOperation(
 					new Position( root, [ 4, 1, 0 ] ),
 					2,
@@ -2047,12 +2062,31 @@ describe( 'transform', () => {
 
 				expect( transOp.length ).to.equal( 1 );
 
+				expected.sourcePosition.offset = 6;
+
+				expectOperation( transOp[ 0 ], expected );
+			} );
+
+			it( 'target at start boundary of transforming move range (sticky move): expand move range', () => {
+				let transformBy = new MoveOperation(
+					new Position( root, [ 4, 1, 0 ] ),
+					2,
+					new Position( root, [ 2, 2, 4 ] ),
+					baseVersion
+				);
+
+				op.isSticky = true;
+
+				let transOp = transform( op, transformBy );
+
+				expect( transOp.length ).to.equal( 1 );
+
 				expected.howMany = 4;
 
 				expectOperation( transOp[ 0 ], expected );
 			} );
 
-			it( 'target at end boundary of transforming move range: expand move range', () => {
+			it( 'target at end boundary of transforming move range: no operation update', () => {
 				let transformBy = new MoveOperation(
 					new Position( root, [ 4, 1, 0 ] ),
 					2,
@@ -2062,6 +2096,22 @@ describe( 'transform', () => {
 
 				let transOp = transform( op, transformBy );
 
+				expect( transOp.length ).to.equal( 1 );
+				expectOperation( transOp[ 0 ], expected );
+			} );
+
+			it( 'target at end boundary of transforming move range (sticky move): expand move range', () => {
+				let transformBy = new MoveOperation(
+					new Position( root, [ 4, 1, 0 ] ),
+					2,
+					new Position( root, [ 2, 2, 6 ] ),
+					baseVersion
+				);
+
+				op.isSticky = true;
+
+				let transOp = transform( op, transformBy );
+
 				expect( transOp.length ).to.equal( 1 );
 
 				expected.howMany = 4;