Browse Source

Changed root property to isDocumentOperation.

Oskar Wróbel 8 years ago
parent
commit
aa93ed2f24

+ 2 - 2
packages/ckeditor5-engine/src/model/operation/attributeoperation.js

@@ -91,8 +91,8 @@ export default class AttributeOperation extends Operation {
 	/**
 	 * @inheritDoc
 	 */
-	get root() {
-		return this.range.root;
+	get isDocumentOperation() {
+		return !!this.range.root.document;
 	}
 
 	/**

+ 2 - 2
packages/ckeditor5-engine/src/model/operation/insertoperation.js

@@ -58,8 +58,8 @@ export default class InsertOperation extends Operation {
 	/**
 	 * @inheritDoc
 	 */
-	get root() {
-		return this.position.root;
+	get isDocumentOperation() {
+		return !!this.position.root.document;
 	}
 
 	/**

+ 4 - 2
packages/ckeditor5-engine/src/model/operation/markeroperation.js

@@ -67,8 +67,10 @@ export default class MarkerOperation extends Operation {
 	/**
 	 * @inheritDoc
 	 */
-	get root() {
-		return this.newRange ? this.newRange.root : this.oldRange.root;
+	get isDocumentOperation() {
+		const root = this.newRange ? this.newRange.root : this.oldRange.root;
+
+		return !!root.document;
 	}
 
 	/**

+ 2 - 2
packages/ckeditor5-engine/src/model/operation/moveoperation.js

@@ -76,10 +76,10 @@ export default class MoveOperation extends Operation {
 	/**
 	 * @inheritDoc
 	 */
-	get root() {
+	get isDocumentOperation() {
 		// Note that range cannot be moved within different documents e.g. from docFrag to document root so
 		// root of source and target positions will be always the same.
-		return this.targetPosition.root;
+		return !!this.targetPosition.root.document;
 	}
 
 	/**

+ 7 - 0
packages/ckeditor5-engine/src/model/operation/nooperation.js

@@ -45,6 +45,13 @@ export default class NoOperation extends Operation {
 	/**
 	 * @inheritDoc
 	 */
+	get isDocumentOperation() {
+		return true;
+	}
+
+	/**
+	 * @inheritDoc
+	 */
 	_execute() {
 		return {};
 	}

+ 2 - 4
packages/ckeditor5-engine/src/model/operation/operation.js

@@ -46,12 +46,10 @@ export default class Operation {
 		 */
 
 		/**
-		 * Root within operation is applied. It might be {@link module:engine/model/rootelement~RootElement RootElement}
-		 * when operation is applied on {@link module:engine/model/document~Document Document} or any
-		 * {module:engine/model/item~Item} when operation is applied on detached node.
+		 * Defines whether operation is executed on attached or detached {@link module:engine/model/item~Item items}.
 		 *
 		 * @readonly
-		 * @member {module:engine/model/item~Item} #root
+		 * @member {Boolean} #isDocumentOperation
 		 */
 
 		/**

+ 21 - 3
packages/ckeditor5-engine/src/model/operation/reinsertoperation.js

@@ -9,6 +9,7 @@
 
 import MoveOperation from './moveoperation';
 import RemoveOperation from './removeoperation';
+import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 
 /**
  * Operation to reinsert previously removed nodes back to the non-graveyard root. This operation acts like
@@ -41,10 +42,12 @@ export default class ReinsertOperation extends MoveOperation {
 	}
 
 	/**
-	 * @inheritDoc
+	 * Reinsert operation is always executed on attached items.
+	 *
+	 * @member {Boolean}
 	 */
-	get root() {
-		return this.targetPosition.root;
+	get isDocumentOperation() {
+		return true;
 	}
 
 	/**
@@ -61,6 +64,21 @@ export default class ReinsertOperation extends MoveOperation {
 	/**
 	 * @inheritDoc
 	 */
+	_execute() {
+		if ( !this.sourcePosition.root.document ) {
+			throw new CKEditorError( 'reinsert-operation-on-detached-item: Cannot reinsert detached item.' );
+		}
+
+		if ( !this.targetPosition.root.document ) {
+			throw new CKEditorError( 'reinsert-operation-to-detached-parent: Cannot reinsert item to detached parent.' );
+		}
+
+		return super._execute();
+	}
+
+	/**
+	 * @inheritDoc
+	 */
 	static get className() {
 		return 'engine.model.operation.ReinsertOperation';
 	}

+ 7 - 0
packages/ckeditor5-engine/src/model/operation/renameoperation.js

@@ -61,6 +61,13 @@ export default class RenameOperation extends Operation {
 	}
 
 	/**
+	 * @inheritDoc
+	 */
+	get isDocumentOperation() {
+		return !!this.position.root.document;
+	}
+
+	/**
 	 * Creates and returns an operation that has the same parameters as this operation.
 	 *
 	 * @returns {module:engine/model/operation/renameoperation~RenameOperation} Clone of this operation.

+ 10 - 0
packages/ckeditor5-engine/src/model/operation/rootattributeoperation.js

@@ -69,6 +69,9 @@ export default class RootAttributeOperation extends Operation {
 		this.newValue = newValue;
 	}
 
+	/**
+	 * @inheritDoc
+	 */
 	get type() {
 		if ( this.oldValue === null ) {
 			return 'addRootAttribute';
@@ -80,6 +83,13 @@ export default class RootAttributeOperation extends Operation {
 	}
 
 	/**
+	 * @inheritDoc
+	 */
+	get isDocumentOperation() {
+		return !!this.root.document;
+	}
+
+	/**
 	 * Creates and returns an operation that has the same parameters as this operation.
 	 *
 	 * @returns {module:engine/model/operation/rootattributeoperation~RootAttributeOperation} Clone of this operation.

+ 5 - 5
packages/ckeditor5-engine/tests/model/operation/attributeoperation.js

@@ -59,8 +59,8 @@ describe( 'AttributeOperation', () => {
 		} );
 	} );
 
-	describe( 'root', () => {
-		it( 'should return root of range when range is in document', () => {
+	describe( 'isDocumentOperation', () => {
+		it( 'should return true when attribute is applied on attached items', () => {
 			const op = new AttributeOperation(
 				new Range( new Position( root, [ 0 ] ), new Position( root, [ 2 ] ) ),
 				'key',
@@ -69,10 +69,10 @@ describe( 'AttributeOperation', () => {
 				doc.version
 			);
 
-			expect( op.root ).to.equal( root );
+			expect( op.isDocumentOperation ).to.true;
 		} );
 
-		it( 'should return root of range when range is in document fragment', () => {
+		it( 'should return false when attribute is applied on detached items', () => {
 			const docFrag = doc.batch().createDocumentFragment();
 			doc.batch().appendText( 'abc', null, docFrag );
 
@@ -84,7 +84,7 @@ describe( 'AttributeOperation', () => {
 				doc.version
 			);
 
-			expect( op.root ).to.equal( docFrag );
+			expect( op.isDocumentOperation ).to.false;
 		} );
 	} );
 

+ 5 - 5
packages/ckeditor5-engine/tests/model/operation/insertoperation.js

@@ -206,18 +206,18 @@ describe( 'InsertOperation', () => {
 		expect( op2.nodes.getNode( 0 ) ).not.to.equal( text );
 	} );
 
-	describe( 'root', () => {
-		it( 'should return operation root for document', () => {
+	describe( 'isDocumentOperation', () => {
+		it( 'should return true when element is inserted to the document', () => {
 			const op = new InsertOperation(
 				new Position( root, [ 0 ] ),
 				new Text( 'x' ),
 				doc.version
 			);
 
-			expect( op.root ).to.equal( root );
+			expect( op.isDocumentOperation ).to.true;
 		} );
 
-		it( 'should return operation root for document fragment', () => {
+		it( 'should return false when element is inserted to document fragment', () => {
 			const docFrag = doc.batch().createDocumentFragment();
 
 			const op = new InsertOperation(
@@ -226,7 +226,7 @@ describe( 'InsertOperation', () => {
 				doc.version
 			);
 
-			expect( op.root ).to.equal( docFrag );
+			expect( op.isDocumentOperation ).to.false;
 		} );
 	} );
 

+ 5 - 5
packages/ckeditor5-engine/tests/model/operation/markeroperation.js

@@ -161,17 +161,17 @@ describe( 'MarkerOperation', () => {
 		expect( clone ).to.deep.equal( op );
 	} );
 
-	describe( 'type', () => {
-		it( 'should return root of new marker range when new marker is added', () => {
+	describe( 'isDocumentOperation', () => {
+		it( 'should return true when new marker range is added to the document', () => {
 			const op = new MarkerOperation( 'name', null, range, doc.markers, doc.version );
 
-			expect( op.root ).to.equal( root );
+			expect( op.isDocumentOperation ).to.true;
 		} );
 
-		it( 'should return root of old marker range when marker is removed', () => {
+		it( 'should return false when marker range is removed from the document', () => {
 			const op = new MarkerOperation( 'name', range, null, doc.markers, doc.version );
 
-			expect( op.root ).to.equal( root );
+			expect( op.isDocumentOperation ).to.true;
 		} );
 	} );
 

+ 5 - 5
packages/ckeditor5-engine/tests/model/operation/moveoperation.js

@@ -267,8 +267,8 @@ describe( 'MoveOperation', () => {
 		expect( clone.baseVersion ).to.equal( baseVersion );
 	} );
 
-	describe( 'root', () => {
-		it( 'should return root for document', () => {
+	describe( 'isDocumentOperation', () => {
+		it( 'should return root when operation is executed on attached items', () => {
 			const op = new MoveOperation(
 				new Position( root, [ 0, 0 ] ),
 				1,
@@ -276,10 +276,10 @@ describe( 'MoveOperation', () => {
 				doc.version
 			);
 
-			expect( op.root ).to.equal( root );
+			expect( op.isDocumentOperation ).to.true;
 		} );
 
-		it( 'should return root for document fragment', () => {
+		it( 'should return false when operation is executed on detached items', () => {
 			const docFrag = doc.batch().createDocumentFragment();
 
 			doc.batch().appendText( 'abc', null, docFrag );
@@ -291,7 +291,7 @@ describe( 'MoveOperation', () => {
 				doc.version
 			);
 
-			expect( op.root ).to.equal( docFrag );
+			expect( op.isDocumentOperation ).to.false;
 		} );
 	} );
 

+ 4 - 0
packages/ckeditor5-engine/tests/model/operation/nooperation.js

@@ -37,6 +37,10 @@ describe( 'NoOperation', () => {
 		expect( clone.baseVersion ).to.equal( 0 );
 	} );
 
+	it( 'should be a document operation', () => {
+		expect( noop.isDocumentOperation ).to.true;
+	} );
+
 	describe( 'toJSON', () => {
 		it( 'should create proper json object', () => {
 			const serialized = jsonParseStringify( noop );

+ 33 - 2
packages/ckeditor5-engine/tests/model/operation/reinsertoperation.js

@@ -10,6 +10,7 @@ import MoveOperation from '../../../src/model/operation/moveoperation';
 import Position from '../../../src/model/position';
 import Element from '../../../src/model/element';
 import Text from '../../../src/model/text';
+import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 import { jsonParseStringify, wrapInDelta } from '../../../tests/model/_utils/utils';
 
 describe( 'ReinsertOperation', () => {
@@ -99,8 +100,38 @@ describe( 'ReinsertOperation', () => {
 		expect( graveyard.maxOffset ).to.equal( 2 );
 	} );
 
-	it( 'should return root of operation', () => {
-		expect( operation.root ).to.equal( root );
+	it( 'should be a document operation', () => {
+		expect( operation.isDocumentOperation ).to.true;
+	} );
+
+	it( 'should throw when target position is not in the document', () => {
+		const docFrag = doc.batch().createDocumentFragment();
+
+		operation = new ReinsertOperation(
+			graveyardPosition,
+			1,
+			Position.createAt( docFrag ),
+			doc.version
+		);
+
+		expect( () => {
+			operation._execute();
+		} ).to.throw( CKEditorError, /^reinsert-operation-to-detached-parent/ );
+	} );
+
+	it( 'should throw when source position is not in the document', () => {
+		const docFrag = doc.batch().createDocumentFragment();
+
+		operation = new ReinsertOperation(
+			Position.createAt( docFrag ),
+			1,
+			rootPosition,
+			doc.version
+		);
+
+		expect( () => {
+			operation._execute();
+		} ).to.throw( CKEditorError, /^reinsert-operation-on-detached-item/ );
 	} );
 
 	describe( 'toJSON', () => {

+ 19 - 0
packages/ckeditor5-engine/tests/model/operation/renameoperation.js

@@ -92,6 +92,25 @@ describe( 'RenameOperation', () => {
 		expect( clone.newName ).to.equal( newName );
 	} );
 
+	describe( 'isDocumentOperation', () => {
+		it( 'should be true when target item is in the document', () => {
+			const op = new RenameOperation( position, oldName, newName, doc.version );
+
+			expect( op.isDocumentOperation ).to.true;
+		} );
+
+		it( 'should be false when target item is not in the document', () => {
+			const batch = doc.batch();
+			const docFrag = batch.createDocumentFragment();
+
+			batch.appendElement( 'element', null, docFrag );
+
+			const op = new RenameOperation( Position.createAt( docFrag ), oldName, newName, doc.version );
+
+			expect( op.isDocumentOperation ).to.false;
+		} );
+	} );
+
 	describe( 'toJSON', () => {
 		it( 'should create proper serialized object', () => {
 			const op = new RenameOperation( Position.createAt( root, 'end' ), oldName, newName, doc.version );

+ 28 - 0
packages/ckeditor5-engine/tests/model/operation/rootattributeoperation.js

@@ -54,6 +54,34 @@ describe( 'RootAttributeOperation', () => {
 		} );
 	} );
 
+	describe( 'isDocumentOperation', () => {
+		it( 'should be true when root is in the document', () => {
+			const operation = new RootAttributeOperation(
+				root,
+				'isNew',
+				null,
+				true,
+				doc.version
+			);
+
+			expect( operation.isDocumentOperation ).to.true;
+		} );
+
+		it( 'should be false when root is not in the document', () => {
+			const element = doc.batch().createElement( 'element' );
+
+			const operation = new RootAttributeOperation(
+				element,
+				'isNew',
+				null,
+				true,
+				doc.version
+			);
+
+			expect( operation.isDocumentOperation ).to.false;
+		} );
+	} );
+
 	it( 'should add attribute on the root element', () => {
 		doc.applyOperation( wrapInDelta(
 			new RootAttributeOperation(