Browse Source

setAttr and removeAttr of the node.

Piotr Jasiun 10 years ago
parent
commit
edb5ad4850

+ 49 - 12
packages/ckeditor5-utils/src/document/delta/changedelta.js

@@ -9,9 +9,11 @@ CKEDITOR.define( [
 	'document/delta/delta',
 	'document/delta/register',
 	'document/operation/changeoperation',
+	'document/position',
 	'document/range',
-	'document/attribute'
-], ( Delta, register, ChangeOperation, Range, Attribute ) => {
+	'document/attribute',
+	'document/element'
+], ( Delta, register, ChangeOperation, Position, Range, Attribute, Element ) => {
 	/**
 	 * To provide specific OT behavior and better collisions solving, change methods ({@link document.Transaction#setAttr}
 	 * and {@link document.Transaction#removeAttr}) use `ChangeDelta` class which inherit from `Delta` class and may
@@ -22,14 +24,14 @@ CKEDITOR.define( [
 	class ChangeDelta extends Delta {}
 
 	/**
-	 * Sets the value of the attribute on the range.
+	 * Sets the value of the attribute of the node or on the range.
 	 *
 	 * @chainable
 	 * @memberOf document.Transaction
 	 * @method setAttr
 	 * @param {String} key Attribute key.
 	 * @param {Mixed} value Attribute new value.
-	 * @param {document.Range} range Range on which the attribute will be set.
+	 * @param {document.Node|document.Range} range Node or range on which the attribute will be set.
 	 */
 	register( 'setAttr', change );
 
@@ -40,15 +42,54 @@ CKEDITOR.define( [
 	 * @memberOf document.Transaction
 	 * @method removeAttr
 	 * @param {String} key Attribute key.
-	 * @param {document.Range} range Range on which the attribute will be set.
+	 * @param {document.Node|document.Range} nodeOrRange Node or range on which the attribute will be removed.
 	 */
-	register( 'removeAttr', ( doc, transaction, key, range ) => {
-		change( doc, transaction, key, null, range );
+	register( 'removeAttr', ( doc, transaction, key, nodeOrRange ) => {
+		change( doc, transaction, key, null, nodeOrRange );
 	} );
 
+	function change( doc, transaction, key, value, nodeOrRange ) {
+		const delta = new ChangeDelta();
+
+		if ( nodeOrRange instanceof Range ) {
+			changeRange( doc, delta, key, value, nodeOrRange );
+		} else {
+			changeNode( doc, delta, key, value, nodeOrRange );
+		}
+
+		transaction.addDelta( delta );
+	}
+
+	function changeNode( doc, delta, key, value, node ) {
+		const previousValue = node.getAttr( key );
+		let range;
+
+		if ( previousValue != value ) {
+			if ( node instanceof Element ) {
+				// If we change the attribute of the element, we do not want to change attributes of its children, so
+				// the end on the range can not be put after the closing tag, it should be inside that element with the
+				// offset 0, so the range will contains only the opening tag...
+				range = new Range( Position.createBefore( node ), Position.createFromParentAndOffset( node, 0 ) );
+			} else {
+				// ...but for characters we can not put the range inside it, so we end the range after that character.
+				range = new Range( Position.createBefore( node ), Position.createAfter( node ) );
+			}
+
+			const operation = new ChangeOperation(
+					range,
+					previousValue ? new Attribute( key, previousValue ) : null,
+					value ? new Attribute( key, value ) : null,
+					doc.version
+				);
+
+			doc.applyOperation( operation );
+			delta.addOperation( operation );
+		}
+	}
+
 	// Because change operation needs to have the same attribute value on the whole range, this function split the range
 	// into smaller parts.
-	function change( doc, transaction, key, value, range ) {
+	function changeRange( doc, delta, key, value, range ) {
 		// Position of the last split, the beginning of the new range.
 		let lastSplitPosition = range.start;
 
@@ -65,8 +106,6 @@ CKEDITOR.define( [
 		// Iterator state.
 		let next = iterator.next();
 
-		const delta = new ChangeDelta();
-
 		while ( !next.done ) {
 			valueAfter = next.value.node.getAttr( key );
 
@@ -93,8 +132,6 @@ CKEDITOR.define( [
 			addOperation();
 		}
 
-		transaction.addDelta( delta );
-
 		function addOperation() {
 			const operation = new ChangeOperation(
 					new Range( lastSplitPosition, position ),

+ 186 - 118
packages/ckeditor5-utils/tests/document/deltas/changedelta.js

@@ -17,10 +17,12 @@ const modules = bender.amd.require(
 	'document/text',
 	'document/attribute',
 	'document/range',
-	'document/position' );
+	'document/position',
+	'document/element',
+	'document/character' );
 
 describe( 'Transaction', () => {
-	let Transaction, Document, Text, Attribute, Range, Position;
+	let Transaction, Document, Text, Attribute, Range, Position, Element, Character;
 
 	let doc, root, transaction;
 
@@ -31,31 +33,16 @@ describe( 'Transaction', () => {
 		Attribute = modules[ 'document/attribute' ];
 		Range = modules[ 'document/range' ];
 		Position = modules[ 'document/position' ];
+		Element = modules[ 'document/element' ];
+		Character = modules[ 'document/character' ];
 	} );
 
 	beforeEach( () => {
 		doc = new Document();
 		root = doc.createRoot( 'root' );
-
-		root.insertChildren( 0, [
-			new Text( 'xxx', [ new Attribute( 'a', 1 ) ] ),
-			'xxx',
-			new Text( 'xxx', [ new Attribute( 'a', 1 ) ] ),
-			new Text( 'xxx', [ new Attribute( 'a', 2 ) ] ),
-			'xxx',
-			new Text( 'xxx', [ new Attribute( 'a', 1 ) ] )
-		] );
-
 		transaction = doc.makeTransaction();
 	} );
 
-	function getRange( startIndex, endIndex ) {
-		return new Range(
-				Position.createFromParentAndOffset( root, startIndex ),
-				Position.createFromParentAndOffset( root, endIndex )
-			);
-	}
-
 	function getOperationsCount() {
 		let count = 0;
 
@@ -66,122 +53,203 @@ describe( 'Transaction', () => {
 		return count;
 	}
 
-	function getChangesAttrsCount() {
-		let count = 0;
-
-		for ( let delta of transaction ) {
-			for ( let operation of delta ) {
-				count += getIteratorCount( operation.range );
-			}
-		}
-
-		return count;
-	}
-
-	function getCompressedAttrs() {
-		// default: 111---111222---111
-		const range = Range.createFromElement( root );
-
-		return Array.from( range ).map( value => value.node.getAttr( 'a' ) || '-' ).join( '' );
-	}
+	describe( 'change attribute on node', () => {
+		let node, character;
 
-	describe( 'setAttr', () => {
-		it( 'should set the attribute on the range', () => {
-			transaction.setAttr( 'a', 3, getRange( 3, 6 ) );
-			expect( getOperationsCount() ).to.equal( 1 );
-			expect( getChangesAttrsCount() ).to.equal( 3 );
-			expect( getCompressedAttrs() ).to.equal( '111333111222---111' );
+		beforeEach( () => {
+			node = new Element( 'p', [ new Attribute( 'a', 1 ) ] );
+			character = new Character( 'c', [ new Attribute( 'a', 1 ) ] );
+			root.insertChildren( 0, [ node, character ] );
 		} );
 
-		it( 'should split the operations if parts of the range have different attributes', () => {
-			transaction.setAttr( 'a', 3, getRange( 4, 14 ) );
-			expect( getOperationsCount() ).to.equal( 4 );
-			expect( getChangesAttrsCount() ).to.equal( 10 );
-			expect( getCompressedAttrs() ).to.equal( '111-3333333333-111' );
+		describe( 'setAttr', () => {
+			it( 'should create the attribute on element', () => {
+				transaction.setAttr( 'b', 2, node );
+				expect( getOperationsCount() ).to.equal( 1 );
+				expect( node.getAttr( 'b' ) ).to.equal( 2 );
+			} );
+
+			it( 'should change the attribute of element', () => {
+				transaction.setAttr( 'a', 2, node );
+				expect( getOperationsCount() ).to.equal( 1 );
+				expect( node.getAttr( 'a' ) ).to.equal( 2 );
+			} );
+
+			it( 'should create the attribute on character', () => {
+				transaction.setAttr( 'b', 2, character );
+				expect( getOperationsCount() ).to.equal( 1 );
+				expect( character.getAttr( 'b' ) ).to.equal( 2 );
+			} );
+
+			it( 'should change the attribute of character', () => {
+				transaction.setAttr( 'a', 2, character );
+				expect( getOperationsCount() ).to.equal( 1 );
+				expect( character.getAttr( 'a' ) ).to.equal( 2 );
+			} );
+
+			it( 'should do nothing if the attribute value is the same', () => {
+				transaction.setAttr( 'a', 1, node );
+				expect( getOperationsCount() ).to.equal( 0 );
+				expect( node.getAttr( 'a' ) ).to.equal( 1 );
+			} );
 		} );
 
-		it( 'should split the operations if parts of the part of the range have the attribute', () => {
-			transaction.setAttr( 'a', 2, getRange( 4, 14 ) );
-			expect( getOperationsCount() ).to.equal( 3 );
-			expect( getChangesAttrsCount() ).to.equal( 7 );
-			expect( getCompressedAttrs() ).to.equal( '111-2222222222-111' );
-		} );
-
-		it( 'should strip the range if the beginning have the attribute', () => {
-			transaction.setAttr( 'a', 1, getRange( 1, 5 ) );
-			expect( getOperationsCount() ).to.equal( 1 );
-			expect( getChangesAttrsCount() ).to.equal( 2 );
-			expect( getCompressedAttrs() ).to.equal( '11111-111222---111' );
-		} );
-
-		it( 'should strip the range if the ending have the attribute', () => {
-			transaction.setAttr( 'a', 1, getRange( 13, 17 ) );
-			expect( getOperationsCount() ).to.equal( 1 );
-			expect( getChangesAttrsCount() ).to.equal( 2 );
-			expect( getCompressedAttrs() ).to.equal( '111---111222-11111' );
+		describe( 'removeAttr', () => {
+			it( 'should remove the attribute from element', () => {
+				transaction.removeAttr( 'a', node );
+				expect( getOperationsCount() ).to.equal( 1 );
+				expect( node.getAttr( 'a' ) ).to.be.null;
+			} );
+
+			it( 'should remove the attribute from character', () => {
+				transaction.removeAttr( 'a', character );
+				expect( getOperationsCount() ).to.equal( 1 );
+				expect( character.getAttr( 'a' ) ).to.be.null;
+			} );
+
+			it( 'should do nothing if the attribute is not set', () => {
+				transaction.removeAttr( 'b', node );
+				expect( getOperationsCount() ).to.equal( 0 );
+			} );
 		} );
+	} );
 
-		it( 'should do nothing if the range has attribute', () => {
-			transaction.setAttr( 'a', 1, getRange( 0, 3 ) );
-			expect( getOperationsCount() ).to.equal( 0 );
-			expect( getCompressedAttrs() ).to.equal( '111---111222---111' );
+	describe( 'change attribute on range', () => {
+		beforeEach( () => {
+			root.insertChildren( 0, [
+				new Text( 'xxx', [ new Attribute( 'a', 1 ) ] ),
+				'xxx',
+				new Text( 'xxx', [ new Attribute( 'a', 1 ) ] ),
+				new Text( 'xxx', [ new Attribute( 'a', 2 ) ] ),
+				'xxx',
+				new Text( 'xxx', [ new Attribute( 'a', 1 ) ] )
+			] );
 		} );
 
-		it( 'should create a proper operations for the mixed range', () => {
-			transaction.setAttr( 'a', 1, getRange( 0, 18 ) );
-			expect( getOperationsCount() ).to.equal( 3 );
-			expect( getChangesAttrsCount() ).to.equal( 9 );
-			expect( getCompressedAttrs() ).to.equal( '111111111111111111' );
-		} );
-	} );
+		function getRange( startIndex, endIndex ) {
+			return new Range(
+					Position.createFromParentAndOffset( root, startIndex ),
+					Position.createFromParentAndOffset( root, endIndex )
+				);
+		}
 
-	describe( 'removeAttr', () => {
-		it( 'should remove the attribute on the range', () => {
-			transaction.removeAttr( 'a', getRange( 0, 2 ) );
-			expect( getOperationsCount() ).to.equal( 1 );
-			expect( getChangesAttrsCount() ).to.equal( 2 );
-			expect( getCompressedAttrs() ).to.equal( '--1---111222---111' );
-		} );
+		function getChangesAttrsCount() {
+			let count = 0;
 
-		it( 'should split the operations if parts of the range have different attributes', () => {
-			transaction.removeAttr( 'a', getRange( 7, 11 ) );
-			expect( getOperationsCount() ).to.equal( 2 );
-			expect( getChangesAttrsCount() ).to.equal( 4 );
-			expect( getCompressedAttrs() ).to.equal( '111---1----2---111' );
-		} );
+			for ( let delta of transaction ) {
+				for ( let operation of delta ) {
+					count += getIteratorCount( operation.range );
+				}
+			}
 
-		it( 'should split the operations if parts of the part of the range have no attribute', () => {
-			transaction.removeAttr( 'a', getRange( 1, 7 ) );
-			expect( getOperationsCount() ).to.equal( 2 );
-			expect( getChangesAttrsCount() ).to.equal( 3 );
-			expect( getCompressedAttrs() ).to.equal( '1------11222---111' );
-		} );
+			return count;
+		}
 
-		it( 'should strip the range if the beginning have no attribute', () => {
-			transaction.removeAttr( 'a', getRange( 4, 12 ) );
-			expect( getOperationsCount() ).to.equal( 2 );
-			expect( getChangesAttrsCount() ).to.equal( 6 );
-			expect( getCompressedAttrs() ).to.equal( '111------------111' );
-		} );
+		function getCompressedAttrs() {
+			// default: 111---111222---111
+			const range = Range.createFromElement( root );
 
-		it( 'should strip the range if the ending have no attribute', () => {
-			transaction.removeAttr( 'a', getRange( 7, 15 ) );
-			expect( getOperationsCount() ).to.equal( 2 );
-			expect( getChangesAttrsCount() ).to.equal( 5 );
-			expect( getCompressedAttrs() ).to.equal( '111---1--------111' );
-		} );
+			return Array.from( range ).map( value => value.node.getAttr( 'a' ) || '-' ).join( '' );
+		}
 
-		it( 'should do nothing if the range has no attribute', () => {
-			transaction.removeAttr( 'a', getRange( 4, 5 ) );
-			expect( getOperationsCount() ).to.equal( 0 );
-			expect( getCompressedAttrs() ).to.equal( '111---111222---111' );
+		describe( 'setAttr', () => {
+			it( 'should set the attribute on the range', () => {
+				transaction.setAttr( 'a', 3, getRange( 3, 6 ) );
+				expect( getOperationsCount() ).to.equal( 1 );
+				expect( getChangesAttrsCount() ).to.equal( 3 );
+				expect( getCompressedAttrs() ).to.equal( '111333111222---111' );
+			} );
+
+			it( 'should split the operations if parts of the range have different attributes', () => {
+				transaction.setAttr( 'a', 3, getRange( 4, 14 ) );
+				expect( getOperationsCount() ).to.equal( 4 );
+				expect( getChangesAttrsCount() ).to.equal( 10 );
+				expect( getCompressedAttrs() ).to.equal( '111-3333333333-111' );
+			} );
+
+			it( 'should split the operations if parts of the part of the range have the attribute', () => {
+				transaction.setAttr( 'a', 2, getRange( 4, 14 ) );
+				expect( getOperationsCount() ).to.equal( 3 );
+				expect( getChangesAttrsCount() ).to.equal( 7 );
+				expect( getCompressedAttrs() ).to.equal( '111-2222222222-111' );
+			} );
+
+			it( 'should strip the range if the beginning have the attribute', () => {
+				transaction.setAttr( 'a', 1, getRange( 1, 5 ) );
+				expect( getOperationsCount() ).to.equal( 1 );
+				expect( getChangesAttrsCount() ).to.equal( 2 );
+				expect( getCompressedAttrs() ).to.equal( '11111-111222---111' );
+			} );
+
+			it( 'should strip the range if the ending have the attribute', () => {
+				transaction.setAttr( 'a', 1, getRange( 13, 17 ) );
+				expect( getOperationsCount() ).to.equal( 1 );
+				expect( getChangesAttrsCount() ).to.equal( 2 );
+				expect( getCompressedAttrs() ).to.equal( '111---111222-11111' );
+			} );
+
+			it( 'should do nothing if the range has attribute', () => {
+				transaction.setAttr( 'a', 1, getRange( 0, 3 ) );
+				expect( getOperationsCount() ).to.equal( 0 );
+				expect( getCompressedAttrs() ).to.equal( '111---111222---111' );
+			} );
+
+			it( 'should create a proper operations for the mixed range', () => {
+				transaction.setAttr( 'a', 1, getRange( 0, 18 ) );
+				expect( getOperationsCount() ).to.equal( 3 );
+				expect( getChangesAttrsCount() ).to.equal( 9 );
+				expect( getCompressedAttrs() ).to.equal( '111111111111111111' );
+			} );
 		} );
 
-		it( 'should create a proper operations for the mixed range', () => {
-			transaction.removeAttr( 'a', getRange( 3, 15 ) );
-			expect( getOperationsCount() ).to.equal( 2 );
-			expect( getChangesAttrsCount() ).to.equal( 6 );
-			expect( getCompressedAttrs() ).to.equal( '111------------111' );
+		describe( 'removeAttr', () => {
+			it( 'should remove the attribute on the range', () => {
+				transaction.removeAttr( 'a', getRange( 0, 2 ) );
+				expect( getOperationsCount() ).to.equal( 1 );
+				expect( getChangesAttrsCount() ).to.equal( 2 );
+				expect( getCompressedAttrs() ).to.equal( '--1---111222---111' );
+			} );
+
+			it( 'should split the operations if parts of the range have different attributes', () => {
+				transaction.removeAttr( 'a', getRange( 7, 11 ) );
+				expect( getOperationsCount() ).to.equal( 2 );
+				expect( getChangesAttrsCount() ).to.equal( 4 );
+				expect( getCompressedAttrs() ).to.equal( '111---1----2---111' );
+			} );
+
+			it( 'should split the operations if parts of the part of the range have no attribute', () => {
+				transaction.removeAttr( 'a', getRange( 1, 7 ) );
+				expect( getOperationsCount() ).to.equal( 2 );
+				expect( getChangesAttrsCount() ).to.equal( 3 );
+				expect( getCompressedAttrs() ).to.equal( '1------11222---111' );
+			} );
+
+			it( 'should strip the range if the beginning have no attribute', () => {
+				transaction.removeAttr( 'a', getRange( 4, 12 ) );
+				expect( getOperationsCount() ).to.equal( 2 );
+				expect( getChangesAttrsCount() ).to.equal( 6 );
+				expect( getCompressedAttrs() ).to.equal( '111------------111' );
+			} );
+
+			it( 'should strip the range if the ending have no attribute', () => {
+				transaction.removeAttr( 'a', getRange( 7, 15 ) );
+				expect( getOperationsCount() ).to.equal( 2 );
+				expect( getChangesAttrsCount() ).to.equal( 5 );
+				expect( getCompressedAttrs() ).to.equal( '111---1--------111' );
+			} );
+
+			it( 'should do nothing if the range has no attribute', () => {
+				transaction.removeAttr( 'a', getRange( 4, 5 ) );
+				expect( getOperationsCount() ).to.equal( 0 );
+				expect( getCompressedAttrs() ).to.equal( '111---111222---111' );
+			} );
+
+			it( 'should create a proper operations for the mixed range', () => {
+				transaction.removeAttr( 'a', getRange( 3, 15 ) );
+				expect( getOperationsCount() ).to.equal( 2 );
+				expect( getChangesAttrsCount() ).to.equal( 6 );
+				expect( getCompressedAttrs() ).to.equal( '111------------111' );
+			} );
 		} );
 	} );
 } );