Browse Source

ChangeOperation.conflictsAttributesWith moved to transformOperation module.

Szymon Cofalik 10 years ago
parent
commit
425937287f

+ 0 - 25
packages/ckeditor5-utils/src/document/operation/changeoperation.js

@@ -101,31 +101,6 @@ CKEDITOR.define( [
 		clone() {
 			return new ChangeOperation( this.range.clone(), this.oldAttr, this.newAttr, this.baseVersion );
 		}
-
-		/**
-		 * Checks whether this operation has conflicting attributes with given {@link document.operation.ChangeOperation}.
-		 * This happens when both operations changes an attribute with the same key and they either set different
-		 * values for this attribute or one of them removes it while the other one sets it.
-		 *
-		 * @param {document.operation.ChangeOperation} otherOperation Operation to check against.
-		 * @returns {boolean} True if operations have conflicting attributes.
-		 */
-		conflictsAttributesWith( otherOperation ) {
-			// Keeping in mind that newAttr or oldAttr might be null.
-			// We will retrieve the key from whichever parameter is set.
-			const thisKey = ( this.newAttr || this.oldAttr ).key;
-			const otherKey = ( otherOperation.newAttr || otherOperation.oldAttr ).key;
-
-			if ( thisKey != otherKey ) {
-				// Different keys - not conflicting.
-				return false;
-			}
-
-			// Check if they set different value or one of them removes the attribute.
-			return ( this.newAttr === null && otherOperation.newAttr !== null ) ||
-				( this.newAttr !== null && otherOperation.newAttr === null ) ||
-				( !this.newAttr.isEqual( otherOperation.newAttr ) );
-		}
 	}
 
 	return ChangeOperation;

+ 22 - 1
packages/ckeditor5-utils/src/document/transformoperation.js

@@ -67,6 +67,27 @@ CKEDITOR.define( [
 		return a.targetPosition.getTransformedByDeletion( b.sourcePosition, b.howMany ) === null;
 	}
 
+	// Takes two ChangeOperations and checks whether their attributes are in conflict.
+	// This happens when both operations changes an attribute with the same key and they either set different
+	// values for this attribute or one of them removes it while the other one sets it.
+	// Returns true if attributes are in conflict.
+	function haveConflictingAttributes( a, b ) {
+		// Keeping in mind that newAttr or oldAttr might be null.
+		// We will retrieve the key from whichever parameter is set.
+		const keyA = ( a.newAttr || a.oldAttr ).key;
+		const keyB = ( b.newAttr || b.oldAttr ).key;
+
+		if ( keyA != keyB ) {
+			// Different keys - not conflicting.
+			return false;
+		}
+
+		// Check if they set different value or one of them removes the attribute.
+		return ( a.newAttr === null && b.newAttr !== null ) ||
+			( a.newAttr !== null && b.newAttr === null ) ||
+			( !a.newAttr.isEqual( b.newAttr ) );
+	}
+
 	const ot = {
 		InsertOperation: {
 			/**
@@ -157,7 +178,7 @@ CKEDITOR.define( [
 			 * @returns {Array.<document.operation.ChangeOperation>} Result of the transformation.
 			 */
 			ChangeOperation( a, b, isStrong ) {
-				if ( !isStrong && a.conflictsAttributesWith( b ) ) {
+				if ( !isStrong && haveConflictingAttributes( a, b ) ) {
 					// If operations' attributes are in conflict and this operation is less important
 					// we have to check if operations' ranges intersect and manage them properly.
 

+ 0 - 82
packages/ckeditor5-utils/tests/document/operation/changeoperation.js

@@ -296,86 +296,4 @@ describe( 'ChangeOperation', () => {
 		expect( clone.newAttr.isEqual( newAttr ) ).to.be.true;
 		expect( clone.baseVersion ).to.equal( baseVersion );
 	} );
-
-	describe( 'have conflicting attributes', () => {
-		it( 'if it sets an attribute that is removed by the other operation', () => {
-			let op = new ChangeOperation(
-				new Range( new Position( [ 0 ], root ), new Position( [ 1 ], root ) ),
-				new Attribute( 'foo', 'old' ),
-				new Attribute( 'foo', 'bar' ),
-				doc.version
-			);
-
-			let op2 = new ChangeOperation(
-				new Range( new Position( [ 0 ], root ), new Position( [ 1 ], root ) ),
-				new Attribute( 'foo', 'old' ),
-				null,
-				doc.version
-			);
-
-			let conflicts = op.conflictsAttributesWith( op2 );
-
-			expect( conflicts ).to.be.true;
-		} );
-
-		it( 'if it removes an attribute that is set by the other operation', () => {
-			let op = new ChangeOperation(
-				new Range( new Position( [ 0 ], root ), new Position( [ 1 ], root ) ),
-				new Attribute( 'foo', 'old' ),
-				null,
-				doc.version
-			);
-
-			let op2 = new ChangeOperation(
-				new Range( new Position( [ 0 ], root ), new Position( [ 1 ], root ) ),
-				new Attribute( 'foo', 'old' ),
-				new Attribute( 'foo', 'bar' ),
-				doc.version
-			);
-
-			let conflicts = op.conflictsAttributesWith( op2 );
-
-			expect( conflicts ).to.be.true;
-		} );
-
-		it( 'if it sets different value than the other operation', () => {
-			let op = new ChangeOperation(
-				new Range( new Position( [ 0 ], root ), new Position( [ 1 ], root ) ),
-				new Attribute( 'foo', 'old' ),
-				new Attribute( 'foo', 'bar' ),
-				doc.version
-			);
-
-			let op2 = new ChangeOperation(
-				new Range( new Position( [ 0 ], root ), new Position( [ 1 ], root ) ),
-				new Attribute( 'foo', 'old' ),
-				new Attribute( 'foo', 'xyz' ),
-				doc.version
-			);
-
-			let conflicts = op.conflictsAttributesWith( op2 );
-
-			expect( conflicts ).to.be.true;
-		} );
-
-		it( 'if it sets a value for an attribute that is being removed by the operation', () => {
-			let op = new ChangeOperation(
-				new Range( new Position( [ 0 ], root ), new Position( [ 1 ], root ) ),
-				null,
-				new Attribute( 'foo', 'bar' ),
-				doc.version
-			);
-
-			let op2 = new ChangeOperation(
-				new Range( new Position( [ 0 ], root ), new Position( [ 1 ], root ) ),
-				new Attribute( 'foo', 'old' ),
-				null,
-				doc.version
-			);
-
-			let conflicts = op.conflictsAttributesWith( op2 );
-
-			expect( conflicts ).to.be.true;
-		} );
-	} );
 } );