Parcourir la source

Fixed: NoOpDeltas now have NoOperation + fixed errors in deltas OT.

Szymon Cofalik il y a 9 ans
Parent
commit
e9d4c5f1e4

+ 44 - 15
packages/ckeditor5-engine/src/model/delta/basic-transformations.js

@@ -10,7 +10,9 @@ import { addTransformationCase, defaultTransform } from './transform.js';
 import Range from '../range.js';
 import Position from '../position.js';
 
+import NoOperation from '../operation/nooperation.js';
 import AttributeOperation from '../operation/attributeoperation.js';
+import ReinsertOperation from '../operation/reinsertoperation.js';
 
 import Delta from './delta.js';
 import AttributeDelta from './attributedelta.js';
@@ -60,7 +62,10 @@ addTransformationCase( MoveDelta, MergeDelta, ( a, b, isStrong ) => {
 	// didn't happen) and then apply the original move operation. This is "mirrored" in MergeDelta x MoveDelta
 	// transformation below, where we simply do not apply MergeDelta.
 
-	const operateInSameParent = compareArrays( a.sourcePosition.getParentPath(), b.position.getParentPath() ) === 'SAME';
+	const operateInSameParent =
+		a.sourcePosition.root == b.position.root &&
+		compareArrays( a.sourcePosition.getParentPath(), b.position.getParentPath() ) === 'SAME';
+
 	const mergeInsideMoveRange = a.sourcePosition.offset <= b.position.offset && a.sourcePosition.offset + a.howMany > b.position.offset;
 
 	if ( operateInSameParent && mergeInsideMoveRange ) {
@@ -78,9 +83,7 @@ addTransformationCase( MergeDelta, InsertDelta, ( a, b, isStrong ) => {
 	// If merge is applied at the same position where we inserted a range of nodes we cancel the merge as it's results
 	// may be unexpected and very weird. Even if we do some "magic" we don't know what really are users' expectations.
 	if ( a.position.isEqual( b.position ) ) {
-		// This is "no-op" delta, it has no type and no operations, it basically does nothing.
-		// It is used when we don't want to apply changes but still we need to return a delta.
-		return [ new Delta() ];
+		return [ noDelta() ];
 	}
 
 	return defaultTransform( a, b, isStrong );
@@ -91,13 +94,14 @@ addTransformationCase( MergeDelta, MoveDelta, ( a, b, isStrong ) => {
 	// If merge is applied at the position between moved nodes we cancel the merge as it's results may be unexpected and
 	// very weird. Even if we do some "magic" we don't know what really are users' expectations.
 
-	const operateInSameParent = compareArrays( a.position.getParentPath(), b.sourcePosition.getParentPath() ) === 'SAME';
+	const operateInSameParent =
+		a.position.root == b.sourcePosition.root &&
+		compareArrays( a.position.getParentPath(), b.sourcePosition.getParentPath() ) === 'SAME';
+
 	const mergeInsideMoveRange = b.sourcePosition.offset <= a.position.offset && b.sourcePosition.offset + b.howMany > a.position.offset;
 
 	if ( operateInSameParent && mergeInsideMoveRange ) {
-		// This is "no-op" delta, it has no type and no operations, it basically does nothing.
-		// It is used when we don't want to apply changes but still we need to return a delta.
-		return [ new Delta() ];
+		return [ noDelta() ];
 	}
 
 	return defaultTransform( a, b, isStrong );
@@ -112,7 +116,7 @@ addTransformationCase( SplitDelta, SplitDelta, ( a, b, isStrong ) => {
 	if ( compareArrays( pathA, pathB ) == 'SAME' ) {
 		if ( a.position.offset == b.position.offset ) {
 			// We are applying split at the position where split already happened. Additional split is not needed.
-			return [ new Delta() ];
+			return [ noDelta() ];
 		} else if ( a.position.offset < b.position.offset ) {
 			// Incoming split delta splits at closer offset. So we simply have to once again split the same node,
 			// but since it was already split (at further offset) there are less child nodes in the split node.
@@ -121,6 +125,15 @@ addTransformationCase( SplitDelta, SplitDelta, ( a, b, isStrong ) => {
 			const delta = a.clone();
 			delta._moveOperation.howMany = b.position.offset - a.position.offset;
 
+			// If both SplitDeltas are taking their nodes from graveyard, we have to transform their ReinsertOperations.
+			if (
+				a._cloneOperation instanceof ReinsertOperation &&
+				b._cloneOperation instanceof ReinsertOperation &&
+				a._cloneOperation.sourcePosition.offset > b._cloneOperation.sourcePosition.offset
+			) {
+				delta._cloneOperation.sourcePosition.offset--;
+			}
+
 			return [ delta ];
 		} else {
 			// Incoming split delta splits at further offset. We have to simulate that we are not splitting the
@@ -134,6 +147,15 @@ addTransformationCase( SplitDelta, SplitDelta, ( a, b, isStrong ) => {
 			delta._moveOperation.targetPosition.path[ delta._moveOperation.targetPosition.path.length - 2 ]++;
 			delta._moveOperation.sourcePosition.offset = a.position.offset - b.position.offset;
 
+			// If both SplitDeltas are taking their nodes from graveyard, we have to transform their ReinsertOperations.
+			if (
+				a._cloneOperation instanceof ReinsertOperation &&
+				b._cloneOperation instanceof ReinsertOperation &&
+				a._cloneOperation.sourcePosition.offset > b._cloneOperation.sourcePosition.offset
+			) {
+				delta._cloneOperation.sourcePosition.offset--;
+			}
+
 			return [ delta ];
 		}
 	}
@@ -146,9 +168,7 @@ addTransformationCase( SplitDelta, UnwrapDelta, ( a, b, isStrong ) => {
 	// If incoming split delta tries to split a node that just got unwrapped, there is actually nothing to split,
 	// so we discard that delta.
 	if ( compareArrays( b.position.path, a.position.getParentPath() ) === 'SAME' ) {
-		// This is "no-op" delta, it has no type and no operations, it basically does nothing.
-		// It is used when we don't want to apply changes but still we need to return a delta.
-		return [ new Delta() ];
+		return [ noDelta() ];
 	}
 
 	return defaultTransform( a, b, isStrong );
@@ -163,9 +183,7 @@ addTransformationCase( SplitDelta, WrapDelta, ( a, b, isStrong ) => {
 	const splitInsideWrapRange = b.range.start.offset < a.position.offset && b.range.end.offset >= a.position.offset;
 
 	if ( operateInSameParent && splitInsideWrapRange ) {
-		// This is "no-op" delta, it has no type and no operations, it basically does nothing.
-		// It is used when we don't want to apply changes but still we need to return a delta.
-		return [ new Delta() ];
+		return [ noDelta() ];
 	} else if ( compareArrays( a.position.getParentPath(), b.range.end.getShiftedBy( -1 ).path ) === 'SAME' ) {
 		// Split position is directly inside the last node from wrap range.
 		// If that's the case, we manually change split delta so it will "target" inside the wrapping element.
@@ -310,3 +328,14 @@ function _getComplementaryAttrDelta( weakInsertDelta, attributeDelta ) {
 
 	return complementaryAttrDelta;
 }
+
+// This is "no-op" delta, it has no type and only no-operation, it basically does nothing.
+// It is used when we don't want to apply changes but still we need to return a delta.
+function noDelta() {
+	let noDelta = new Delta();
+
+	// BaseVersion will be fixed later anyway.
+	noDelta.addOperation( new NoOperation( 0 ) );
+
+	return noDelta;
+}

+ 17 - 2
packages/ckeditor5-engine/tests/model/delta/transform/mergedelta.js

@@ -21,6 +21,7 @@ import MergeDelta from '/ckeditor5/engine/model/delta/mergedelta.js';
 
 import MoveOperation from '/ckeditor5/engine/model/operation/moveoperation.js';
 import RemoveOperation from '/ckeditor5/engine/model/operation/removeoperation.js';
+import NoOperation from '/ckeditor5/engine/model/operation/nooperation.js';
 
 import { getNodesAndText, jsonParseStringify } from '/tests/engine/model/_utils/utils.js';
 
@@ -65,11 +66,18 @@ describe( 'transform', () => {
 
 				// Expected: MergeDelta gets ignored and is not applied.
 
+				baseVersion = insertDelta.operations.length;
+
 				expect( transformed.length ).to.equal( 1 );
 
 				expectDelta( transformed[ 0 ], {
 					type: Delta,
-					operations: []
+					operations: [
+						{
+							type: NoOperation,
+							baseVersion: baseVersion
+						}
+					]
 				} );
 
 				// Test if deltas do what they should after applying transformed delta.
@@ -134,11 +142,18 @@ describe( 'transform', () => {
 				let moveDelta = getMoveDelta( new Position( root, [ 3, 3, 3 ] ), 1, new Position( root, [ 3, 3, 0 ] ), baseVersion );
 				let transformed = transform( mergeDelta, moveDelta );
 
+				baseVersion = moveDelta.operations.length;
+
 				expect( transformed.length ).to.equal( 1 );
 
 				expectDelta( transformed[ 0 ], {
 					type: Delta,
-					operations: []
+					operations: [
+						{
+							type: NoOperation,
+							baseVersion: baseVersion
+						}
+					]
 				} );
 
 				// Test if deltas do what they should after applying transformed delta.

+ 116 - 3
packages/ckeditor5-engine/tests/model/delta/transform/splitdelta.js

@@ -20,7 +20,9 @@ import Delta from '/ckeditor5/engine/model/delta/delta.js';
 import SplitDelta from '/ckeditor5/engine/model/delta/splitdelta.js';
 
 import InsertOperation from '/ckeditor5/engine/model/operation/insertoperation.js';
+import ReinsertOperation from '/ckeditor5/engine/model/operation/reinsertoperation.js';
 import MoveOperation from '/ckeditor5/engine/model/operation/moveoperation.js';
+import NoOperation from '/ckeditor5/engine/model/operation/nooperation.js';
 
 import { getNodesAndText, jsonParseStringify } from '/tests/engine/model/_utils/utils.js';
 
@@ -56,11 +58,18 @@ describe( 'transform', () => {
 				let splitDeltaB = getSplitDelta( splitPosition, new Element( 'p' ), 9, baseVersion );
 				let transformed = transform( splitDelta, splitDeltaB );
 
+				baseVersion = splitDeltaB.operations.length;
+
 				expect( transformed.length ).to.equal( 1 );
 
 				expectDelta( transformed[ 0 ], {
 					type: Delta,
-					operations: []
+					operations: [
+						{
+							type: NoOperation,
+							baseVersion: baseVersion
+						}
+					]
 				} );
 
 				// Test if deltas do what they should after applying transformed delta.
@@ -112,6 +121,51 @@ describe( 'transform', () => {
 				expect( nodesAndText ).to.equal( 'XXXXXabcdXPabcPPfoPPobarxyzP' );
 			} );
 
+			it( 'split in same parent, incoming delta splits closer, split deltas have reinsert operations', () => {
+				let reOp = new ReinsertOperation(
+					new Position( gy, [ 1 ] ),
+					1,
+					Position.createFromPosition( splitDelta.operations[ 0 ].position ),
+					splitDelta.operations[ 0 ].baseVersion
+				);
+				splitDelta.operations[ 0 ] = reOp;
+
+				let splitDeltaB = getSplitDelta( new Position( root, [ 3, 3, 3, 5 ] ), new Element( 'p' ), 7, baseVersion );
+				reOp = new ReinsertOperation(
+					new Position( gy, [ 0 ] ),
+					1,
+					Position.createFromPosition( splitDeltaB.operations[ 0 ].position ),
+					splitDeltaB.operations[ 0 ].baseVersion
+				);
+				splitDeltaB.operations[ 0 ] = reOp;
+
+				let transformed = transform( splitDelta, splitDeltaB );
+
+				baseVersion = splitDeltaB.operations.length;
+
+				expect( transformed.length ).to.equal( 1 );
+
+				expectDelta( transformed[ 0 ], {
+					type: SplitDelta,
+					operations: [
+						{
+							type: ReinsertOperation,
+							sourcePosition: new Position( gy, [ 0 ] ),
+							howMany: 1,
+							targetPosition: new Position( root, [ 3, 3, 4 ] ),
+							baseVersion: baseVersion
+						},
+						{
+							type: MoveOperation,
+							sourcePosition: new Position( root, [ 3, 3, 3, 3 ] ),
+							howMany: 2,
+							targetPosition: new Position( root, [ 3, 3, 4, 0 ] ),
+							baseVersion: baseVersion + 1
+						}
+					]
+				} );
+			} );
+
 			it( 'split in same parent, incoming delta splits further', () => {
 				let splitDeltaB = getSplitDelta( new Position( root, [ 3, 3, 3, 1 ] ), new Element( 'p' ), 11, baseVersion );
 				let transformed = transform( splitDelta, splitDeltaB );
@@ -149,6 +203,51 @@ describe( 'transform', () => {
 				expect( nodesAndText ).to.equal( 'XXXXXabcdXPaPPbcPPfoobarxyzP' );
 			} );
 
+			it( 'split in same parent, incoming delta splits further, split deltas have reinsert operations', () => {
+				let reOp = new ReinsertOperation(
+					new Position( gy, [ 1 ] ),
+					1,
+					Position.createFromPosition( splitDelta.operations[ 0 ].position ),
+					splitDelta.operations[ 0 ].baseVersion
+				);
+				splitDelta.operations[ 0 ] = reOp;
+
+				let splitDeltaB = getSplitDelta( new Position( root, [ 3, 3, 3, 1 ] ), new Element( 'p' ), 11, baseVersion );
+				reOp = new ReinsertOperation(
+					new Position( gy, [ 0 ] ),
+					1,
+					Position.createFromPosition( splitDeltaB.operations[ 0 ].position ),
+					splitDeltaB.operations[ 0 ].baseVersion
+				);
+				splitDeltaB.operations[ 0 ] = reOp;
+
+				let transformed = transform( splitDelta, splitDeltaB );
+
+				baseVersion = splitDeltaB.operations.length;
+
+				expect( transformed.length ).to.equal( 1 );
+
+				expectDelta( transformed[ 0 ], {
+					type: SplitDelta,
+					operations: [
+						{
+							type: ReinsertOperation,
+							sourcePosition: new Position( gy, [ 0 ] ),
+							howMany: 1,
+							targetPosition: new Position( root, [ 3, 3, 5 ] ),
+							baseVersion: baseVersion
+						},
+						{
+							type: MoveOperation,
+							sourcePosition: new Position( root, [ 3, 3, 4, 2 ] ),
+							howMany: 9,
+							targetPosition: new Position( root, [ 3, 3, 5, 0 ] ),
+							baseVersion: baseVersion + 1
+						}
+					]
+				} );
+			} );
+
 			it( 'split in split parent', () => {
 				let splitDeltaB = getSplitDelta( new Position( root, [ 3, 3, 3 ] ), new Element( 'div' ), 1, baseVersion );
 				let transformed = transform( splitDelta, splitDeltaB );
@@ -192,11 +291,18 @@ describe( 'transform', () => {
 				let unwrapDelta = getUnwrapDelta( new Position( root, [ 3, 3, 3 ] ), 12, baseVersion );
 				let transformed = transform( splitDelta, unwrapDelta );
 
+				baseVersion = unwrapDelta.operations.length;
+
 				expect( transformed.length ).to.equal( 1 );
 
 				expectDelta( transformed[ 0 ], {
 					type: Delta,
-					operations: []
+					operations: [
+						{
+							type: NoOperation,
+							baseVersion: baseVersion
+						}
+					]
 				} );
 
 				// Test if deltas do what they should after applying transformed delta.
@@ -255,11 +361,18 @@ describe( 'transform', () => {
 
 				let transformed = transform( splitDelta, wrapDelta );
 
+				baseVersion = wrapDelta.operations.length;
+
 				expect( transformed.length ).to.equal( 1 );
 
 				expectDelta( transformed[ 0 ], {
 					type: Delta,
-					operations: []
+					operations: [
+						{
+							type: NoOperation,
+							baseVersion: baseVersion
+						}
+					]
 				} );
 
 				// Test if deltas do what they should after applying transformed delta.