Kaynağa Gözat

ChangeOperation._execute() removed some throwed errors, ChangeOperation.clone() introduced.

Szymon Cofalik 10 yıl önce
ebeveyn
işleme
708ed546dc

+ 17 - 33
packages/ckeditor5-utils/src/document/operation/changeoperation.js

@@ -5,7 +5,14 @@
 
 'use strict';
 
-CKEDITOR.define( [ 'document/operation/operation', 'ckeditorerror' ], ( Operation, CKEditorError ) => {
+CKEDITOR.define( [
+	'document/operation/operation',
+	'document/range',
+	'document/operation/nooperation',
+	'ckeditorerror',
+	'document/operation/moveoperation',
+	'document/operation/insertoperation'
+], ( Operation, Range, NoOperation, CKEditorError ) => {
 	/**
 	 * Operation to change nodes' attribute. Using this class you can add, remove or change value of the attribute.
 	 *
@@ -77,45 +84,15 @@ CKEDITOR.define( [ 'document/operation/operation', 'ckeditorerror' ], ( Operatio
 			}
 
 			// Remove or change.
-			if ( oldAttr !== null ) {
+			if ( oldAttr !== null && newAttr === null ) {
 				for ( value of this.range ) {
-					if ( !value.node.hasAttr( oldAttr ) ) {
-						/**
-						 * The attribute which should be removed does not exists for the given node.
-						 *
-						 * @error operation-change-no-attr-to-remove
-						 * @param {document.Node} node
-						 * @param {document.Attribute} attr
-						 */
-						throw new CKEditorError(
-							'operation-change-no-attr-to-remove: The attribute which should be removed does not exists for given node.',
-							{ node: value.node, attr: oldAttr } );
-					}
-
-					// There is no use in removing attribute if we will overwrite it later.
-					// Still it is profitable to run through the loop to check if all nodes in the range has old attribute.
-					if ( newAttr === null ) {
-						value.node.removeAttr( oldAttr.key );
-					}
+					value.node.removeAttr( oldAttr.key );
 				}
 			}
 
 			// Insert or change.
 			if ( newAttr !== null ) {
 				for ( value of this.range ) {
-					if ( oldAttr === null && value.node.hasAttr( newAttr.key ) ) {
-						/**
-						 * The attribute with given key already exists for the given node.
-						 *
-						 * @error operation-change-attr-exists
-						 * @param {document.Node} node
-						 * @param {document.Attribute} attr
-						 */
-						throw new CKEditorError(
-							'operation-change-attr-exists: The attribute with given key already exists.',
-							{ node: value.node, attr: newAttr } );
-					}
-
 					value.node.setAttr( newAttr );
 				}
 			}
@@ -124,6 +101,13 @@ CKEDITOR.define( [ 'document/operation/operation', 'ckeditorerror' ], ( Operatio
 		getReversed() {
 			return new ChangeOperation( this.range, this.newAttr, this.oldAttr, this.baseVersion + 1 );
 		}
+		clone( baseVersion ) {
+			if ( !baseVersion ) {
+				baseVersion = this.baseVersion;
+			}
+
+			return new ChangeOperation( this.range.clone(), this.oldAttr, this.newAttr, baseVersion );
+		}
 	}
 
 	return ChangeOperation;

+ 20 - 34
packages/ckeditor5-utils/tests/document/operation/changeoperation.js

@@ -258,8 +258,9 @@ describe( 'ChangeOperation', () => {
 		expect( root.getChild( 2 ).hasAttr( fooAttr ) ).to.be.true;
 	} );
 
-	it( 'should throw an error when one try to remove and the attribute does not exists', () => {
+	it( 'should throw an error when one try to change and the new and old attributes have different keys', () => {
 		let fooAttr = new Attribute( 'foo', true );
+		let barAttr = new Attribute( 'bar', true );
 
 		root.insertChildren( 0, 'x' );
 
@@ -269,51 +270,36 @@ describe( 'ChangeOperation', () => {
 					new ChangeOperation(
 						new Range( new Position( [ 0 ], root ), new Position( [ 1 ], root ) ),
 						fooAttr,
-						null,
+						barAttr,
 						doc.version
 					)
 				);
 			}
-		).to.throw( CKEditorError, /operation-change-no-attr-to-remove/ );
+		).to.throw( CKEditorError, /operation-change-different-keys/ );
 	} );
 
-	it( 'should throw an error when one try to insert and the attribute already exists', () => {
-		let x1Attr = new Attribute( 'x', 1 );
-		let x2Attr = new Attribute( 'x', 2 );
+	it( 'should create operation with the same parameters when cloned', () => {
+		let range = new Range( new Position( [ 0 ], root ), new Position( [ 1 ], root ) );
+		let oldAttr = new Attribute( 'foo', 'old' );
+		let newAttr = new Attribute( 'foo', 'bar' );
+		let baseVersion = doc.version;
 
-		root.insertChildren( 0, new Character( 'x', [ x1Attr ] ) );
+		let op = new ChangeOperation( range, oldAttr, newAttr, baseVersion );
 
-		expect(
-			() => {
-				doc.applyOperation(
-					new ChangeOperation(
-						new Range( new Position( [ 0 ], root ), new Position( [ 1 ], root ) ),
-						null,
-						x2Attr,
-						doc.version
-					)
-				);
-			}
-		).to.throw( CKEditorError, /operation-change-attr-exists/ );
+		let clone = op.clone();
+
+		// New instance rather than a pointer to the old instance.
+		expect( clone ).not.to.be.equal( op );
+
+		expect( clone ).to.be.instanceof( ChangeOperation );
+		expect( clone.range.isEqual( range ) ).to.be.true;
+		expect( clone.oldAttr.isEqual( oldAttr ) ).to.be.true;
+		expect( clone.newAttr.isEqual( newAttr ) ).to.be.true;
+		expect( clone.baseVersion ).to.equal( baseVersion );
 	} );
 
-	it( 'should throw an error when one try to change and the new and old attributes have different keys', () => {
-		let fooAttr = new Attribute( 'foo', true );
-		let barAttr = new Attribute( 'bar', true );
 
-		root.insertChildren( 0, 'x' );
 
-		expect(
-			() => {
-				doc.applyOperation(
-					new ChangeOperation(
-						new Range( new Position( [ 0 ], root ), new Position( [ 1 ], root ) ),
-						fooAttr,
-						barAttr,
-						doc.version
-					)
-				);
 			}
-		).to.throw( CKEditorError, /operation-change-different-keys/ );
 	} );
 } );