浏览代码

operation/transform.js tests and bugfix.

Szymon Cofalik 10 年之前
父节点
当前提交
59f7fcc55e

+ 5 - 0
packages/ckeditor5-utils/src/document/operation/transform.js

@@ -88,6 +88,11 @@ CKEDITOR.define( [
 			return false;
 		}
 
+		if ( a.newAttr === null && b.newAttr === null ) {
+			// Both remove the attribute - 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 ) ||

+ 45 - 8
packages/ckeditor5-utils/tests/document/operation/transform.js

@@ -398,12 +398,15 @@ describe( 'transform', () => {
 	} );
 
 	describe( 'ChangeOperation', () => {
-		let start, end, range, oldAttr, newAttr;
+		let start, end, range, oldAttr, newAttr, anotherOldAttr, anotherNewAttr;
 
 		beforeEach( () => {
 			oldAttr = new Attribute( 'foo', 'abc' );
 			newAttr = new Attribute( 'foo', 'bar' );
 
+			anotherOldAttr = new Attribute( oldAttr.key, 'another' );
+			anotherNewAttr = new Attribute( oldAttr.key, 'anothernew' );
+
 			expected = {
 				type: ChangeOperation,
 				oldAttr: oldAttr,
@@ -534,7 +537,7 @@ describe( 'transform', () => {
 			} );
 
 			describe( 'by ChangeOperation', () => {
-				it( 'attributes not conflicting: no operation update', () => {
+				it( 'attributes have different key: no operation update', () => {
 					let transformBy = new ChangeOperation(
 						range.clone(),
 						new Attribute( 'abc', true ),
@@ -548,13 +551,39 @@ describe( 'transform', () => {
 					expectOperation( transOp[ 0 ], expected );
 				} );
 
-				describe( 'that is less important and', () => {
-					let anotherOldAttr;
+				it( 'attributes set same value: no operation update', () => {
+					let transformBy = new ChangeOperation(
+						range.clone(),
+						oldAttr,
+						newAttr,
+						baseVersion
+					);
 
-					beforeEach( () => {
-						anotherOldAttr = new Attribute( oldAttr.key, 'another' );
-					} );
+					let transOp = transform( op, transformBy );
 
+					expect( transOp.length ).to.equal( 1 );
+					expectOperation( transOp[ 0 ], expected );
+				} );
+
+				it( 'both operations removes attribute: no operation update', () => {
+					op.newAttr = null;
+
+					let transformBy = new ChangeOperation(
+						new Range( new Position( [ 1, 1, 4 ], root ), new Position( [ 3 ], root ) ),
+						anotherOldAttr,
+						null,
+						baseVersion
+					);
+
+					let transOp = transform( op, transformBy, true );
+
+					expected.newAttr = null;
+
+					expect( transOp.length ).to.equal( 1 );
+					expectOperation( transOp[ 0 ], expected );
+				} );
+
+				describe( 'that is less important and', () => {
 					it( 'range does not intersect original range: no operation update', () => {
 						let transformBy = new ChangeOperation(
 							new Range( new Position( [ 3, 0 ], root ), new Position( [ 4 ], root ) ),
@@ -587,10 +616,14 @@ describe( 'transform', () => {
 
 					// [ original range   <   ]   range transformed by >
 					it( 'range intersects on left: split into two operations, update oldAttr', () => {
+						// Get more test cases and better code coverage
+						op.newAttr = null;
+
 						let transformBy = new ChangeOperation(
 							new Range( new Position( [ 1, 4, 2 ], root ), new Position( [ 3 ], root ) ),
 							anotherOldAttr,
-							null,
+							// Get more test cases and better code coverage
+							anotherNewAttr,
 							baseVersion
 						);
 
@@ -598,6 +631,8 @@ describe( 'transform', () => {
 
 						expect( transOp.length ).to.equal( 2 );
 
+						expected.newAttr = null;
+
 						expected.range.end.path = [ 1, 4, 2 ];
 
 						expectOperation( transOp[ 0 ], expected );
@@ -1017,6 +1052,8 @@ describe( 'transform', () => {
 
 		// Some extra cases for a ChangeOperation that operates on single tree level range.
 		// This means that the change range start and end differs only on offset value.
+		// This test suite also have some modifications to the original operation
+		// to get more test cases covered and better code coverage.
 		describe( 'with single-level range', () => {
 			beforeEach( () => {
 				start = new Position( [ 0, 2, 1 ], root );