8
0
Pārlūkot izejas kodu

Changed: Operations that does not operate on document have `null` base version.

Szymon Cofalik 8 gadi atpakaļ
vecāks
revīzija
dd202e6760
26 mainītis faili ar 245 papildinājumiem un 355 dzēšanām
  1. 9 3
      packages/ckeditor5-engine/src/model/batch.js
  2. 2 6
      packages/ckeditor5-engine/src/model/operation/attributeoperation.js
  3. 2 8
      packages/ckeditor5-engine/src/model/operation/detachoperation.js
  4. 2 6
      packages/ckeditor5-engine/src/model/operation/insertoperation.js
  5. 2 24
      packages/ckeditor5-engine/src/model/operation/markeroperation.js
  6. 2 12
      packages/ckeditor5-engine/src/model/operation/moveoperation.js
  7. 0 12
      packages/ckeditor5-engine/src/model/operation/nooperation.js
  8. 11 8
      packages/ckeditor5-engine/src/model/operation/operation.js
  9. 0 15
      packages/ckeditor5-engine/src/model/operation/reinsertoperation.js
  10. 0 16
      packages/ckeditor5-engine/src/model/operation/removeoperation.js
  11. 2 6
      packages/ckeditor5-engine/src/model/operation/renameoperation.js
  12. 17 6
      packages/ckeditor5-engine/src/model/operation/rootattributeoperation.js
  13. 70 46
      packages/ckeditor5-engine/src/model/writer.js
  14. 6 6
      packages/ckeditor5-engine/tests/dev-utils/enableenginedebug.js
  15. 0 29
      packages/ckeditor5-engine/tests/model/operation/attributeoperation.js
  16. 6 6
      packages/ckeditor5-engine/tests/model/operation/detachoperation.js
  17. 0 25
      packages/ckeditor5-engine/tests/model/operation/insertoperation.js
  18. 0 20
      packages/ckeditor5-engine/tests/model/operation/markeroperation.js
  19. 0 27
      packages/ckeditor5-engine/tests/model/operation/moveoperation.js
  20. 0 4
      packages/ckeditor5-engine/tests/model/operation/nooperation.js
  21. 14 0
      packages/ckeditor5-engine/tests/model/operation/operation.js
  22. 0 4
      packages/ckeditor5-engine/tests/model/operation/reinsertoperation.js
  23. 0 11
      packages/ckeditor5-engine/tests/model/operation/removeoperation.js
  24. 0 16
      packages/ckeditor5-engine/tests/model/operation/renameoperation.js
  25. 35 30
      packages/ckeditor5-engine/tests/model/operation/rootattributeoperation.js
  26. 65 9
      packages/ckeditor5-engine/tests/model/writer.js

+ 9 - 3
packages/ckeditor5-engine/src/model/batch.js

@@ -50,14 +50,20 @@ export default class Batch {
 	}
 
 	/**
-	 * Returns this batch base version, which is equal to the base version of first delta in the batch.
-	 * If there are no deltas in the batch, it returns `null`.
+	 * Returns this batch base version, which is equal to the base version of first delta (which has base version set)
+	 * in the batch. If there are no deltas in the batch or neither delta has base version set, it returns `null`.
 	 *
 	 * @readonly
 	 * @type {Number|null}
 	 */
 	get baseVersion() {
-		return this.deltas.length > 0 ? this.deltas[ 0 ].baseVersion : null;
+		for ( const delta of this.deltas ) {
+			if ( delta.baseVersion !== null ) {
+				return delta.baseVersion;
+			}
+		}
+
+		return null;
 	}
 
 	/**

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

@@ -37,7 +37,8 @@ export default class AttributeOperation extends Operation {
 	 * @param {String} key Key of an attribute to change or remove.
 	 * @param {*} oldValue Old value of the attribute with given key or `null`, if attribute was not set before.
 	 * @param {*} newValue New value of the attribute with given key or `null`, if operation should remove attribute.
-	 * @param {Number} baseVersion {@link module:engine/model/document~Document#version} on which the operation can be applied.
+	 * @param {Number|null} baseVersion Document {@link module:engine/model/document~Document#version} on which operation
+	 * can be applied or `null` if the operation operates on detached (non-document) tree.
 	 */
 	constructor( range, key, oldValue, newValue, baseVersion ) {
 		super( baseVersion );
@@ -73,11 +74,6 @@ export default class AttributeOperation extends Operation {
 		 * @member {*}
 		 */
 		this.newValue = newValue === undefined ? null : newValue;
-
-		/**
-		 * @inheritDoc
-		 */
-		this.isDocumentOperation = !!this.range.root.document;
 	}
 
 	/**

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

@@ -27,10 +27,9 @@ export default class DetachOperation extends Operation {
 	 * Position before the first {@link module:engine/model/item~Item model item} to move.
 	 * @param {Number} howMany Offset size of moved range. Moved range will start from `sourcePosition` and end at
 	 * `sourcePosition` with offset shifted by `howMany`.
-	 * @param {Number} baseVersion {@link module:engine/model/document~Document#version} on which operation can be applied.
 	 */
-	constructor( sourcePosition, howMany, baseVersion ) {
-		super( baseVersion );
+	constructor( sourcePosition, howMany ) {
+		super( null );
 
 		/**
 		 * Position before the first {@link module:engine/model/item~Item model item} to detach.
@@ -45,11 +44,6 @@ export default class DetachOperation extends Operation {
 		 * @member {Number} #howMany
 		 */
 		this.howMany = howMany;
-
-		/**
-		 * @inheritDoc
-		 */
-		this.isDocumentOperation = false;
 	}
 
 	/**

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

@@ -27,7 +27,8 @@ export default class InsertOperation extends Operation {
 	 *
 	 * @param {module:engine/model/position~Position} position Position of insertion.
 	 * @param {module:engine/model/node~NodeSet} nodes The list of nodes to be inserted.
-	 * @param {Number} baseVersion {@link module:engine/model/document~Document#version} on which operation can be applied.
+	 * @param {Number|null} baseVersion Document {@link module:engine/model/document~Document#version} on which operation
+	 * can be applied or `null` if the operation operates on detached (non-document) tree.
 	 */
 	constructor( position, nodes, baseVersion ) {
 		super( baseVersion );
@@ -47,11 +48,6 @@ export default class InsertOperation extends Operation {
 		 * @member {module:engine/model/nodelist~NodeList} module:engine/model/operation/insertoperation~InsertOperation#nodeList
 		 */
 		this.nodes = new NodeList( _normalizeNodes( nodes ) );
-
-		/**
-		 * @inheritDoc
-		 */
-		this.isDocumentOperation = !!this.position.root.document;
 	}
 
 	/**

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

@@ -19,7 +19,8 @@ export default class MarkerOperation extends Operation {
 	 * @param {module:engine/model/range~Range} oldRange Marker range before the change.
 	 * @param {module:engine/model/range~Range} newRange Marker range after the change.
 	 * @param {module:engine/model/markercollection~MarkerCollection} markers Marker collection on which change should be executed.
-	 * @param {Number} baseVersion {@link module:engine/model/document~Document#version} on which the operation can be applied.
+	 * @param {Number|null} baseVersion Document {@link module:engine/model/document~Document#version} on which operation
+	 * can be applied or `null` if the operation operates on detached (non-document) tree.
 	 */
 	constructor( name, oldRange, newRange, markers, baseVersion ) {
 		super( baseVersion );
@@ -55,11 +56,6 @@ export default class MarkerOperation extends Operation {
 		 * @member {module:engine/model/markercollection~MarkerCollection}
 		 */
 		this._markers = markers;
-
-		/**
-		 * @inheritDoc
-		 */
-		this.isDocumentOperation = this._isDocumentOperation();
 	}
 
 	/**
@@ -70,24 +66,6 @@ export default class MarkerOperation extends Operation {
 	}
 
 	/**
-	 * Checks if operation is executed on document or document fragment nodes.
-	 *
-	 * @private
-	 */
-	_isDocumentOperation() {
-		if ( this.newRange ) {
-			return !!this.newRange.root.document;
-		}
-
-		if ( this.oldRange ) {
-			return !!this.oldRange.root.document;
-		}
-
-		// This is edge and might happen only on data from the server.
-		return true;
-	}
-
-	/**
 	 * Creates and returns an operation that has the same parameters as this operation.
 	 *
 	 * @returns {module:engine/model/operation/markeroperation~MarkerOperation} Clone of this operation.

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

@@ -29,7 +29,8 @@ export default class MoveOperation extends Operation {
 	 * @param {Number} howMany Offset size of moved range. Moved range will start from `sourcePosition` and end at
 	 * `sourcePosition` with offset shifted by `howMany`.
 	 * @param {module:engine/model/position~Position} targetPosition Position at which moved nodes will be inserted.
-	 * @param {Number} baseVersion {@link module:engine/model/document~Document#version} on which operation can be applied.
+	 * @param {Number|null} baseVersion Document {@link module:engine/model/document~Document#version} on which operation
+	 * can be applied or `null` if the operation operates on detached (non-document) tree.
 	 */
 	constructor( sourcePosition, howMany, targetPosition, baseVersion ) {
 		super( baseVersion );
@@ -64,17 +65,6 @@ export default class MoveOperation extends Operation {
 		 * @member {Boolean} module:engine/model/operation/moveoperation~MoveOperation#isSticky
 		 */
 		this.isSticky = false;
-
-		/**
-		 * Defines whether operation is executed on attached or detached {@link module:engine/model/item~Item items}.
-		 *
-		 * Note that range cannot be moved within different documents e.g. from docFrag to document root so
-		 * root of source and target positions is always the same.
-		 *
-		 * @readonly
-		 * @member {Boolean} #isDocumentOperation
-		 */
-		this.isDocumentOperation = !!this.targetPosition.root.document;
 	}
 
 	/**

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

@@ -20,18 +20,6 @@ import Operation from './operation';
  * @extends module:engine/model/operation/operation~Operation
  */
 export default class NoOperation extends Operation {
-	/**
-	 * @inheritDoc
-	 */
-	constructor( baseVersion ) {
-		super( baseVersion );
-
-		/**
-		 * @inheritDoc
-		 */
-		this.isDocumentOperation = true;
-	}
-
 	get type() {
 		return 'noop';
 	}

+ 11 - 8
packages/ckeditor5-engine/src/model/operation/operation.js

@@ -17,7 +17,9 @@ import clone from '@ckeditor/ckeditor5-utils/src/lib/lodash/clone';
 export default class Operation {
 	/**
 	 * Base operation constructor.
-	 * @param {Number} baseVersion {@link module:engine/model/document~Document#version} on which the operation can be applied.
+	 *
+	 * @param {Number|null} baseVersion Document {@link module:engine/model/document~Document#version} on which operation
+	 * can be applied or `null` if the operation operates on detached (non-document) tree.
 	 */
 	constructor( baseVersion ) {
 		/**
@@ -31,6 +33,14 @@ export default class Operation {
 		this.baseVersion = baseVersion;
 
 		/**
+		 * Defines whether operation is executed on attached or detached {@link module:engine/model/item~Item items}.
+		 *
+		 * @readonly
+		 * @member {Boolean} #isDocumentOperation
+		 */
+		this.isDocumentOperation = this.baseVersion !== null;
+
+		/**
 		 * Operation type.
 		 *
 		 * @readonly
@@ -46,13 +56,6 @@ export default class Operation {
 		 */
 
 		/**
-		 * Defines whether operation is executed on attached or detached {@link module:engine/model/item~Item items}.
-		 *
-		 * @readonly
-		 * @member {Boolean} #isDocumentOperation
-		 */
-
-		/**
 		 * Creates and returns an operation that has the same parameters as this operation.
 		 *
 		 * @method #clone

+ 0 - 15
packages/ckeditor5-engine/src/model/operation/reinsertoperation.js

@@ -19,21 +19,6 @@ import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
  */
 export default class ReinsertOperation extends MoveOperation {
 	/**
-	 * @inheritDocs
-	 */
-	constructor( sourcePosition, howMany, targetPosition, baseVersion ) {
-		super( sourcePosition, howMany, targetPosition, baseVersion );
-
-		/**
-		 * Reinsert operation is always executed on attached items.
-		 *
-		 * @readonly
-		 * @member {Boolean}
-		 */
-		this.isDocumentOperation = true;
-	}
-
-	/**
 	 * Position where nodes will be re-inserted.
 	 *
 	 * @type {module:engine/model/position~Position}

+ 0 - 16
packages/ckeditor5-engine/src/model/operation/removeoperation.js

@@ -16,22 +16,6 @@ import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
  */
 export default class RemoveOperation extends MoveOperation {
 	/**
-	 * @inheritDocs
-	 */
-	constructor( sourcePosition, howMany, targetPosition, baseVersion ) {
-		super( sourcePosition, howMany, targetPosition, baseVersion );
-
-		/**
-		 * Remove operation cannot be applied on element that is not inside the document
-		 * so this will always be a document operation.
-		 *
-		 * @readonly
-		 * @member {Boolean}
-		 */
-		this.isDocumentOperation = true;
-	}
-
-	/**
 	 * @inheritDoc
 	 */
 	get type() {

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

@@ -26,7 +26,8 @@ export default class RenameOperation extends Operation {
 	 * @param {module:engine/model/position~Position} position Position before an element to change.
 	 * @param {String} oldName Current name of the element.
 	 * @param {String} newName New name for the element.
-	 * @param {Number} baseVersion {@link module:engine/model/document~Document#version} on which the operation can be applied.
+	 * @param {Number|null} baseVersion Document {@link module:engine/model/document~Document#version} on which operation
+	 * can be applied or `null` if the operation operates on detached (non-document) tree.
 	 */
 	constructor( position, oldName, newName, baseVersion ) {
 		super( baseVersion );
@@ -51,11 +52,6 @@ export default class RenameOperation extends Operation {
 		 * @member {String} module:engine/model/operation/renameoperation~RenameOperation#newName
 		 */
 		this.newName = newName;
-
-		/**
-		 * @inheritDoc
-		 */
-		this.isDocumentOperation = !!this.position.root.document;
 	}
 
 	/**

+ 17 - 6
packages/ckeditor5-engine/src/model/operation/rootattributeoperation.js

@@ -31,7 +31,8 @@ export default class RootAttributeOperation extends Operation {
 	 * @param {String} key Key of an attribute to change or remove.
 	 * @param {*} oldValue Old value of the attribute with given key or `null` if adding a new attribute.
 	 * @param {*} newValue New value to set for the attribute. If `null`, then the operation just removes the attribute.
-	 * @param {Number} baseVersion {@link module:engine/model/document~Document#version} on which the operation can be applied.
+	 * @param {Number|null} baseVersion Document {@link module:engine/model/document~Document#version} on which operation
+	 * can be applied or `null` if the operation operates on detached (non-document) tree.
 	 */
 	constructor( root, key, oldValue, newValue, baseVersion ) {
 		super( baseVersion );
@@ -67,11 +68,6 @@ export default class RootAttributeOperation extends Operation {
 		 * @member {*}
 		 */
 		this.newValue = newValue;
-
-		/**
-		 * @inheritDoc
-		 */
-		this.isDocumentOperation = !!this.root.document;
 	}
 
 	/**
@@ -109,6 +105,21 @@ export default class RootAttributeOperation extends Operation {
 	 * @inheritDoc
 	 */
 	_validate() {
+		if ( this.root != this.root.root || this.root.is( 'documentFragment' ) ) {
+			/**
+			 * The element to change is not a root element.
+			 *
+			 * @error rootattribute-operation-not-a-root
+			 * @param {module:engine/model/rootelement~RootElement} root
+			 * @param {String} key
+			 * @param {*} value
+			 */
+			throw new CKEditorError(
+				'rootattribute-operation-not-a-root: The element to change is not a root element.',
+				{ root: this.root, key: this.key }
+			);
+		}
+
 		if ( this.oldValue !== null && this.root.getAttribute( this.key ) !== this.oldValue ) {
 			/**
 			 * The attribute which should be removed does not exists for the given node.

+ 70 - 46
packages/ckeditor5-engine/src/model/writer.js

@@ -175,7 +175,9 @@ export default class Writer {
 			}
 		}
 
-		const insert = new InsertOperation( position, item, this.model.document.version );
+		const version = position.root.document ? this.model.document.version : null;
+
+		const insert = new InsertOperation( position, item, version );
 
 		this.batch.addDelta( delta );
 		delta.addOperation( insert );
@@ -322,9 +324,9 @@ export default class Writer {
 		this._assertWriterUsageCorrectness();
 
 		if ( itemOrRange instanceof Range ) {
-			setAttributeToRange( this, key, value, itemOrRange );
+			setAttributeOnRange( this, key, value, itemOrRange );
 		} else {
-			setAttributeToItem( this, key, value, itemOrRange );
+			setAttributeOnItem( this, key, value, itemOrRange );
 		}
 	}
 
@@ -359,9 +361,9 @@ export default class Writer {
 		this._assertWriterUsageCorrectness();
 
 		if ( itemOrRange instanceof Range ) {
-			setAttributeToRange( this, key, null, itemOrRange );
+			setAttributeOnRange( this, key, null, itemOrRange );
 		} else {
-			setAttributeToItem( this, key, null, itemOrRange );
+			setAttributeOnItem( this, key, null, itemOrRange );
 		}
 	}
 
@@ -449,7 +451,9 @@ export default class Writer {
 		const delta = new MoveDelta();
 		this.batch.addDelta( delta );
 
-		const operation = new MoveOperation( range.start, range.end.offset - range.start.offset, position, this.model.document.version );
+		const version = range.root.document ? this.model.document.version : null;
+
+		const operation = new MoveOperation( range.start, range.end.offset - range.start.offset, position, version );
 		delta.addOperation( operation );
 		this.model.applyOperation( operation );
 	}
@@ -465,19 +469,8 @@ export default class Writer {
 		const addRemoveDelta = ( position, howMany ) => {
 			const delta = new RemoveDelta();
 			this.batch.addDelta( delta );
-			let operation;
-
-			if ( position.root.document ) {
-				const graveyard = this.model.document.graveyard;
-				const gyPosition = new Position( graveyard, [ 0 ] );
-
-				operation = new RemoveOperation( position, howMany, gyPosition, this.model.document.version );
-			} else {
-				operation = new DetachOperation( position, howMany, this.model.document.version );
-			}
 
-			delta.addOperation( operation );
-			this.model.applyOperation( operation );
+			addRemoveOperation( position, howMany, delta, this.model );
 		};
 
 		if ( itemOrRange instanceof Range ) {
@@ -532,23 +525,20 @@ export default class Writer {
 		const positionAfter = Position.createFromParentAndOffset( nodeAfter, 0 );
 		const positionBefore = Position.createFromParentAndOffset( nodeBefore, nodeBefore.maxOffset );
 
+		const moveVersion = position.root.document ? this.model.document.version : null;
+
 		const move = new MoveOperation(
 			positionAfter,
 			nodeAfter.maxOffset,
 			positionBefore,
-			this.model.document.version
+			moveVersion
 		);
 
 		move.isSticky = true;
 		delta.addOperation( move );
 		this.model.applyOperation( move );
 
-		const graveyard = this.model.document.graveyard;
-		const gyPosition = new Position( graveyard, [ 0 ] );
-
-		const remove = new RemoveOperation( position, 1, gyPosition, this.model.document.version );
-		delta.addOperation( remove );
-		this.model.applyOperation( remove );
+		addRemoveOperation( position, 1, delta, this.model );
 	}
 
 	/**
@@ -574,7 +564,9 @@ export default class Writer {
 		const delta = new RenameDelta();
 		this.batch.addDelta( delta );
 
-		const renameOperation = new RenameOperation( Position.createBefore( element ), element.name, newName, this.model.document.version );
+		const version = element.root.document ? this.model.document.version : null;
+
+		const renameOperation = new RenameOperation( Position.createBefore( element ), element.name, newName, version );
 		delta.addOperation( renameOperation );
 		this.model.applyOperation( renameOperation );
 	}
@@ -605,21 +597,24 @@ export default class Writer {
 		}
 
 		const copy = new Element( splitElement.name, splitElement.getAttributes() );
+		const insertVersion = splitElement.root.document ? this.model.document.version : null;
 
 		const insert = new InsertOperation(
 			Position.createAfter( splitElement ),
 			copy,
-			this.model.document.version
+			insertVersion
 		);
 
 		delta.addOperation( insert );
 		this.model.applyOperation( insert );
 
+		const moveVersion = insertVersion !== null ? insertVersion + 1 : null;
+
 		const move = new MoveOperation(
 			position,
 			splitElement.maxOffset - position.offset,
 			Position.createFromParentAndOffset( copy, 0 ),
-			this.model.document.version
+			moveVersion
 		);
 		move.isSticky = true;
 
@@ -670,16 +665,20 @@ export default class Writer {
 		const delta = new WrapDelta();
 		this.batch.addDelta( delta );
 
-		const insert = new InsertOperation( range.end, element, this.model.document.version );
+		const insertVersion = range.root.document ? this.model.document.version : null;
+
+		const insert = new InsertOperation( range.end, element, insertVersion );
 		delta.addOperation( insert );
 		this.model.applyOperation( insert );
 
+		const moveVersion = insertVersion !== null ? insertVersion + 1 : null;
+
 		const targetPosition = Position.createFromParentAndOffset( element, 0 );
 		const move = new MoveOperation(
 			range.start,
 			range.end.offset - range.start.offset,
 			targetPosition,
-			this.model.document.version
+			moveVersion
 		);
 		delta.addOperation( move );
 		this.model.applyOperation( move );
@@ -707,26 +706,20 @@ export default class Writer {
 		this.batch.addDelta( delta );
 
 		const sourcePosition = Position.createFromParentAndOffset( element, 0 );
+		const moveVersion = sourcePosition.root.document ? this.model.document.version : null;
 
 		const move = new MoveOperation(
 			sourcePosition,
 			element.maxOffset,
 			Position.createBefore( element ),
-			this.model.document.version
+			moveVersion
 		);
 
 		move.isSticky = true;
 		delta.addOperation( move );
 		this.model.applyOperation( move );
 
-		// Computing new position because we moved some nodes before `element`.
-		// If we would cache `Position.createBefore( element )` we remove wrong node.
-		const graveyard = this.model.document.graveyard;
-		const gyPosition = new Position( graveyard, [ 0 ] );
-
-		const remove = new RemoveOperation( Position.createBefore( element ), 1, gyPosition, this.model.document.version );
-		delta.addOperation( remove );
-		this.model.applyOperation( remove );
+		addRemoveOperation( Position.createBefore( element ), 1, delta, this.model );
 	}
 
 	/**
@@ -824,7 +817,7 @@ export default class Writer {
 // @param {String} key Attribute key.
 // @param {*} value Attribute new value.
 // @param {module:engine/model/range~Range} range Model range on which the attribute will be set.
-function setAttributeToRange( writer, key, value, range ) {
+function setAttributeOnRange( writer, key, value, range ) {
 	const delta = new AttributeDelta();
 	const model = writer.model;
 	const doc = model.document;
@@ -873,7 +866,8 @@ function setAttributeToRange( writer, key, value, range ) {
 		}
 
 		const range = new Range( lastSplitPosition, position );
-		const operation = new AttributeOperation( range, key, valueBefore, value, doc.version );
+		const version = range.root.document ? doc.version : null;
+		const operation = new AttributeOperation( range, key, valueBefore, value, version );
 
 		delta.addOperation( operation );
 		model.applyOperation( operation );
@@ -887,19 +881,23 @@ function setAttributeToRange( writer, key, value, range ) {
 // @param {String} key Attribute key.
 // @param {*} value Attribute new value.
 // @param {module:engine/model/item~Item} item Model item on which the attribute will be set.
-function setAttributeToItem( writer, key, value, item ) {
+function setAttributeOnItem( writer, key, value, item ) {
 	const model = writer.model;
 	const doc = model.document;
 	const previousValue = item.getAttribute( key );
 	let range, operation;
 
 	if ( previousValue != value ) {
-		const delta = item.root === item ? new RootAttributeDelta() : new AttributeDelta();
+		const isRootChanged = item.root === item;
+
+		const delta = isRootChanged ? new RootAttributeDelta() : new AttributeDelta();
 		writer.batch.addDelta( delta );
 
-		if ( item.root === item ) {
+		if ( isRootChanged ) {
 			// If we change attributes of root element, we have to use `RootAttributeOperation`.
-			operation = new RootAttributeOperation( item, key, previousValue, value, doc.version );
+			const version = item.document ? doc.version : null;
+
+			operation = new RootAttributeOperation( item, key, previousValue, value, version );
 		} else {
 			if ( item.is( 'element' ) ) {
 				// If we change the attribute of the element, we do not want to change attributes of its children, so
@@ -912,7 +910,9 @@ function setAttributeToItem( writer, key, value, item ) {
 				range = new Range( Position.createBefore( item ), Position.createAfter( item ) );
 			}
 
-			operation = new AttributeOperation( range, key, previousValue, value, doc.version );
+			const version = range.root.document ? doc.version : null;
+
+			operation = new AttributeOperation( range, key, previousValue, value, version );
 		}
 
 		delta.addOperation( operation );
@@ -939,6 +939,30 @@ function addMarkerOperation( writer, name, oldRange, newRange ) {
 	model.applyOperation( operation );
 }
 
+// Creates `RemoveOperation` or `DetachOperation` that removes `howMany` nodes starting from `position`.
+// The operation will be applied on given model instance and added to given delta instance.
+//
+// @private
+// @param {module:engine/model/position~Position} position Position from which nodes are removed.
+// @param {Number} howMany Number of nodes to remove.
+// @param {module:engine/model/delta~Delta} delta Delta to add new operation to.
+// @param {module:engine/model/model~Model} model Model instance on which operation will be applied.
+function addRemoveOperation( position, howMany, delta, model ) {
+	let operation;
+
+	if ( position.root.document ) {
+		const doc = model.document;
+		const gyPosition = new Position( doc.graveyard, [ 0 ] );
+
+		operation = new RemoveOperation( position, howMany, gyPosition, doc.version );
+	} else {
+		operation = new DetachOperation( position, howMany );
+	}
+
+	delta.addOperation( operation );
+	model.applyOperation( operation );
+}
+
 // Returns `true` if both root elements are the same element or both are documents root elements.
 //
 // Elements in the same tree can be moved (for instance you can move element form one documents root to another, or

+ 6 - 6
packages/ckeditor5-engine/tests/dev-utils/enableenginedebug.js

@@ -250,9 +250,9 @@ describe( 'debug tools', () => {
 			} );
 
 			it( 'DetachOperation (text node)', () => {
-				const op = new DetachOperation( ModelPosition.createAt( modelRoot, 0 ), 3, 0 );
+				const op = new DetachOperation( ModelPosition.createAt( modelRoot, 0 ), 3 );
 
-				expect( op.toString() ).to.equal( 'DetachOperation( 0 ): #foo -> main [ 0 ] - [ 3 ]' );
+				expect( op.toString() ).to.equal( 'DetachOperation( null ): #foo -> main [ 0 ] - [ 3 ]' );
 
 				op.log();
 				expect( log.calledWithExactly( op.toString() ) ).to.be.true;
@@ -262,9 +262,9 @@ describe( 'debug tools', () => {
 				const element = new ModelElement( 'element' );
 				modelRoot.insertChildren( 0, element );
 
-				const op = new DetachOperation( ModelPosition.createBefore( element ), 1, 0 );
+				const op = new DetachOperation( ModelPosition.createBefore( element ), 1 );
 
-				expect( op.toString() ).to.equal( 'DetachOperation( 0 ): <element> -> main [ 0 ] - [ 1 ]' );
+				expect( op.toString() ).to.equal( 'DetachOperation( null ): <element> -> main [ 0 ] - [ 1 ]' );
 
 				op.log();
 				expect( log.calledWithExactly( op.toString() ) ).to.be.true;
@@ -274,9 +274,9 @@ describe( 'debug tools', () => {
 				const element = new ModelElement( 'element' );
 				modelRoot.insertChildren( 0, element );
 
-				const op = new DetachOperation( ModelPosition.createBefore( element ), 2, 0 );
+				const op = new DetachOperation( ModelPosition.createBefore( element ), 2 );
 
-				expect( op.toString() ).to.equal( 'DetachOperation( 0 ): [ 2 ] -> main [ 0 ] - [ 2 ]' );
+				expect( op.toString() ).to.equal( 'DetachOperation( null ): [ 2 ] -> main [ 0 ] - [ 2 ]' );
 
 				op.log();
 				expect( log.calledWithExactly( op.toString() ) ).to.be.true;

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

@@ -4,7 +4,6 @@
  */
 
 import Model from '../../../src/model/model';
-import DocumentFragment from '../../../src/model/documentfragment';
 import Element from '../../../src/model/element';
 import Text from '../../../src/model/text';
 import AttributeOperation from '../../../src/model/operation/attributeoperation';
@@ -61,34 +60,6 @@ describe( 'AttributeOperation', () => {
 		} );
 	} );
 
-	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',
-				'oldValue',
-				'newValue',
-				doc.version
-			);
-
-			expect( op.isDocumentOperation ).to.true;
-		} );
-
-		it( 'should return false when attribute is applied on detached items', () => {
-			const docFrag = new DocumentFragment( [ new Text( 'abc' ) ] );
-
-			const op = new AttributeOperation(
-				Range.createIn( docFrag ),
-				'key',
-				'oldValue',
-				'newValue',
-				doc.version
-			);
-
-			expect( op.isDocumentOperation ).to.false;
-		} );
-	} );
-
 	it( 'should insert attribute to the set of nodes', () => {
 		root.insertChildren( 0, new Text( 'bar' ) );
 

+ 6 - 6
packages/ckeditor5-engine/tests/model/operation/detachoperation.js

@@ -22,13 +22,13 @@ describe( 'DetachOperation', () => {
 	} );
 
 	it( 'should have type equal to detach', () => {
-		const op = new DetachOperation( Position.createBefore( element ), 1, doc.version );
+		const op = new DetachOperation( Position.createBefore( element ), 1 );
 
 		expect( op.type ).to.equal( 'detach' );
 	} );
 
 	it( 'should remove given element from parent', () => {
-		const op = new DetachOperation( Position.createBefore( element ), 1, doc.version );
+		const op = new DetachOperation( Position.createBefore( element ), 1 );
 
 		model.applyOperation( wrapInDelta( op ) );
 
@@ -42,7 +42,7 @@ describe( 'DetachOperation', () => {
 
 			root.appendChildren( [ element ] );
 
-			const op = new DetachOperation( Position.createBefore( element ), 1, doc.version );
+			const op = new DetachOperation( Position.createBefore( element ), 1 );
 
 			expect( () => {
 				op._validate();
@@ -51,7 +51,7 @@ describe( 'DetachOperation', () => {
 	} );
 
 	it( 'should be not a document operation', () => {
-		const op = new DetachOperation( Position.createBefore( element ), 1, doc.version );
+		const op = new DetachOperation( Position.createBefore( element ), 1 );
 
 		expect( op.isDocumentOperation ).to.false;
 	} );
@@ -59,13 +59,13 @@ describe( 'DetachOperation', () => {
 	describe( 'toJSON', () => {
 		it( 'should create proper json object', () => {
 			const position = Position.createBefore( element );
-			const op = new DetachOperation( position, 1, doc.version );
+			const op = new DetachOperation( position, 1 );
 
 			const serialized = jsonParseStringify( op );
 
 			expect( serialized ).to.deep.equal( {
 				__className: 'engine.model.operation.DetachOperation',
-				baseVersion: 0,
+				baseVersion: null,
 				sourcePosition: jsonParseStringify( position ),
 				howMany: 1
 			} );

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

@@ -6,7 +6,6 @@
 import Model from '../../../src/model/model';
 import NodeList from '../../../src/model/nodelist';
 import Element from '../../../src/model/element';
-import DocumentFragment from '../../../src/model/documentfragment';
 import InsertOperation from '../../../src/model/operation/insertoperation';
 import RemoveOperation from '../../../src/model/operation/removeoperation';
 import Position from '../../../src/model/position';
@@ -209,30 +208,6 @@ describe( 'InsertOperation', () => {
 		expect( op2.nodes.getNode( 0 ) ).not.to.equal( text );
 	} );
 
-	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.isDocumentOperation ).to.true;
-		} );
-
-		it( 'should return false when element is inserted to document fragment', () => {
-			const docFrag = new DocumentFragment();
-
-			const op = new InsertOperation(
-				new Position( docFrag, [ 0 ] ),
-				new Text( 'x' ),
-				doc.version
-			);
-
-			expect( op.isDocumentOperation ).to.false;
-		} );
-	} );
-
 	describe( '_validate()', () => {
 		it( 'should throw an error if target position does not exist', () => {
 			const element = new Element( 'p' );

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

@@ -128,26 +128,6 @@ describe( 'MarkerOperation', () => {
 		expect( clone ).to.deep.equal( op );
 	} );
 
-	describe( 'isDocumentOperation', () => {
-		it( 'should return true when new marker range is added to the document', () => {
-			const op = new MarkerOperation( 'name', null, range, model.markers, doc.version );
-
-			expect( op.isDocumentOperation ).to.true;
-		} );
-
-		it( 'should return false when marker range is removed from the document', () => {
-			const op = new MarkerOperation( 'name', range, null, model.markers, doc.version );
-
-			expect( op.isDocumentOperation ).to.true;
-		} );
-
-		it( 'should return true when non-existing marker range is removed from the document', () => {
-			const op = new MarkerOperation( 'name', null, null, model.markers, doc.version );
-
-			expect( op.isDocumentOperation ).to.true;
-		} );
-	} );
-
 	describe( 'toJSON', () => {
 		it( 'should create proper serialized object', () => {
 			const op = new MarkerOperation( 'name', null, range, model.markers, doc.version );

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

@@ -6,7 +6,6 @@
 import Model from '../../../src/model/model';
 import MoveOperation from '../../../src/model/operation/moveoperation';
 import Position from '../../../src/model/position';
-import DocumentFragment from '../../../src/model/documentfragment';
 import Element from '../../../src/model/element';
 import Text from '../../../src/model/text';
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
@@ -259,32 +258,6 @@ describe( 'MoveOperation', () => {
 		expect( clone.baseVersion ).to.equal( baseVersion );
 	} );
 
-	describe( 'isDocumentOperation', () => {
-		it( 'should return root when operation is executed on attached items', () => {
-			const op = new MoveOperation(
-				new Position( root, [ 0, 0 ] ),
-				1,
-				new Position( root, [ 1, 0 ] ),
-				doc.version
-			);
-
-			expect( op.isDocumentOperation ).to.true;
-		} );
-
-		it( 'should return false when operation is executed on detached items', () => {
-			const docFrag = new DocumentFragment( [ new Text( 'abc' ) ] );
-
-			const op = new MoveOperation(
-				new Position( docFrag, [ 0 ] ),
-				1,
-				new Position( docFrag, [ 2 ] ),
-				doc.version
-			);
-
-			expect( op.isDocumentOperation ).to.false;
-		} );
-	} );
-
 	describe( 'getMovedRangeStart', () => {
 		it( 'should return move operation target position transformed by removing move operation source range', () => {
 			const sourcePosition = new Position( root, [ 0, 2 ] );

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

@@ -34,10 +34,6 @@ 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 );

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

@@ -26,6 +26,20 @@ describe( 'Operation', () => {
 		expect( parsedOutside.delta ).to.be.undefined;
 	} );
 
+	describe( 'isDocumentOperation', () => {
+		it( 'operation is a document operation if it has base version set', () => {
+			const op = new Operation( 0 );
+
+			expect( op.isDocumentOperation ).to.be.true;
+		} );
+
+		it( 'operation is not a document operation if base version is null', () => {
+			const op = new Operation( null );
+
+			expect( op.isDocumentOperation ).to.be.false;
+		} );
+	} );
+
 	describe( 'toJSON', () => {
 		it( 'should create proper json object', () => {
 			const op = new Operation( 4 );

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

@@ -102,10 +102,6 @@ describe( 'ReinsertOperation', () => {
 		expect( graveyard.maxOffset ).to.equal( 2 );
 	} );
 
-	it( 'should be a document operation', () => {
-		expect( operation.isDocumentOperation ).to.true;
-	} );
-
 	describe( '_validate()', () => {
 		it( 'should throw when target position is not in the document', () => {
 			const docFrag = new DocumentFragment();

+ 0 - 11
packages/ckeditor5-engine/tests/model/operation/removeoperation.js

@@ -163,17 +163,6 @@ describe( 'RemoveOperation', () => {
 		} );
 	} );
 
-	it( 'should always be a document operation', () => {
-		const op = new RemoveOperation(
-			new Position( root, [ 2 ] ),
-			2,
-			new Position( doc.graveyard, [ 0 ] ),
-			doc.version
-		);
-
-		expect( op.isDocumentOperation ).to.true;
-	} );
-
 	describe( 'toJSON', () => {
 		it( 'should create proper json object', () => {
 			const op = new RemoveOperation(

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

@@ -4,7 +4,6 @@
  */
 
 import Model from '../../../src/model/model';
-import DocumentFragment from '../../../src/model/documentfragment';
 import Element from '../../../src/model/element';
 import RenameOperation from '../../../src/model/operation/renameoperation';
 import Position from '../../../src/model/position';
@@ -104,21 +103,6 @@ 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 docFrag = new DocumentFragment( [ new Element( 'element' ) ] );
-			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 );

+ 35 - 30
packages/ckeditor5-engine/tests/model/operation/rootattributeoperation.js

@@ -4,6 +4,7 @@
  */
 
 import Model from '../../../src/model/model';
+import DocumentFragment from '../../../src/model/documentfragment';
 import Element from '../../../src/model/element';
 import RootAttributeOperation from '../../../src/model/operation/rootattributeoperation';
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
@@ -56,34 +57,6 @@ 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 = new Element( 'element' );
-
-			const operation = new RootAttributeOperation(
-				element,
-				'isNew',
-				null,
-				true,
-				doc.version
-			);
-
-			expect( operation.isDocumentOperation ).to.false;
-		} );
-	} );
-
 	it( 'should add attribute on the root element', () => {
 		model.applyOperation( wrapInDelta(
 			new RootAttributeOperation(
@@ -204,7 +177,39 @@ describe( 'RootAttributeOperation', () => {
 	} );
 
 	describe( '_validate()', () => {
-		it( 'should throw an error when one try to remove and the attribute does not exists', () => {
+		it( 'should throw an error when trying to change non-root element', () => {
+			const child = new Element( 'p' );
+			const parent = new Element( 'p' );
+			parent.appendChildren( child );
+
+			expect( () => {
+				const op = new RootAttributeOperation(
+					child,
+					'foo',
+					null,
+					'bar',
+					null
+				);
+
+				op._validate();
+			} ).to.throw( CKEditorError, /rootattribute-operation-not-a-root/ );
+		} );
+
+		it( 'should throw an error when trying to change document fragment', () => {
+			expect( () => {
+				const op = new RootAttributeOperation(
+					new DocumentFragment(),
+					'foo',
+					null,
+					'bar',
+					null
+				);
+
+				op._validate();
+			} ).to.throw( CKEditorError, /rootattribute-operation-not-a-root/ );
+		} );
+
+		it( 'should throw an error when trying to remove an attribute that does not exists', () => {
 			expect( () => {
 				const op = new RootAttributeOperation(
 					root,
@@ -218,7 +223,7 @@ describe( 'RootAttributeOperation', () => {
 			} ).to.throw( CKEditorError, /rootattribute-operation-wrong-old-value/ );
 		} );
 
-		it( 'should throw an error when one try to insert and the attribute already exists', () => {
+		it( 'should throw an error when trying to add an attribute that already exists', () => {
 			root.setAttribute( 'x', 1 );
 
 			expect( () => {

+ 65 - 9
packages/ckeditor5-engine/tests/model/writer.js

@@ -189,7 +189,7 @@ describe( 'Writer', () => {
 			expect( spy.lastCall.args[ 0 ].delta.batch ).to.equal( batch );
 		} );
 
-		it( 'should move element from one parent to the other within the same document (documentA -> documentA)', () => {
+		it( 'should move element from one parent to the other within the same document (rootA -> rootA)', () => {
 			const root = doc.createRoot();
 			const parent1 = createElement( 'parent' );
 			const parent2 = createElement( 'parent' );
@@ -213,7 +213,7 @@ describe( 'Writer', () => {
 			expect( spy.firstCall.args[ 0 ].delta.batch ).to.equal( batch );
 		} );
 
-		it( 'should move element from one parent to the other within the same document (documentA -> documentB)', () => {
+		it( 'should move element from one parent to the other within the same document (rootA -> rootB)', () => {
 			const rootA = doc.createRoot( '$root', 'A' );
 			const rootB = doc.createRoot( '$root', 'B' );
 			const node = createText( 'foo' );
@@ -1439,6 +1439,18 @@ describe( 'Writer', () => {
 			expect( root.getChild( 0 ).getChild( 0 ).data ).to.equal( 'foobar' );
 		} );
 
+		it( 'should correctly merge in document fragment', () => {
+			const docFrag = new DocumentFragment( [
+				new Element( 'p', null, 'foo' ),
+				new Element( 'p', null, 'bar' )
+			] );
+
+			writer.merge( new Position( docFrag, [ 1 ] ) );
+
+			expect( docFrag.getChild( 0 ).name ).to.equal( 'p' );
+			expect( docFrag.getChild( 0 ).getChild( 0 ).data ).to.equal( 'foobar' );
+		} );
+
 		it( 'should throw if there is no element after', () => {
 			expect( () => {
 				merge( new Position( root, [ 2 ] ) );
@@ -1653,22 +1665,30 @@ describe( 'Writer', () => {
 	} );
 
 	describe( 'rename()', () => {
-		let root;
-
-		beforeEach( () => {
-			root = doc.createRoot();
-
+		it( 'should rename given element', () => {
+			const root = doc.createRoot();
 			const p = new Element( 'p', null, new Text( 'abc' ) );
+
 			root.appendChildren( p );
 
 			rename( p, 'h' );
-		} );
 
-		it( 'should rename given element', () => {
 			expect( root.maxOffset ).to.equal( 1 );
 			expect( root.getChild( 0 ) ).to.have.property( 'name', 'h' );
 		} );
 
+		it( 'should rename in document fragment', () => {
+			const docFrag = new DocumentFragment();
+			const p = new Element( 'p' );
+
+			docFrag.appendChildren( p );
+
+			writer.rename( p, 'h' );
+
+			expect( docFrag.maxOffset ).to.equal( 1 );
+			expect( docFrag.getChild( 0 ) ).to.have.property( 'name', 'h' );
+		} );
+
 		it( 'should throw if not an Element instance is passed', () => {
 			expect( () => {
 				rename( new Text( 'abc' ), 'h' );
@@ -1714,6 +1734,23 @@ describe( 'Writer', () => {
 			expect( root.getChild( 1 ).getChild( 0 ).data ).to.equal( 'bar' );
 		} );
 
+		it( 'should split inside document fragment', () => {
+			const docFrag = new DocumentFragment();
+			docFrag.appendChildren( new Element( 'p', null, new Text( 'foobar' ) ) );
+
+			writer.split( new Position( docFrag, [ 0, 3 ] ) );
+
+			expect( docFrag.maxOffset ).to.equal( 2 );
+
+			expect( docFrag.getChild( 0 ).name ).to.equal( 'p' );
+			expect( docFrag.getChild( 0 ).maxOffset ).to.equal( 3 );
+			expect( docFrag.getChild( 0 ).getChild( 0 ).data ).to.equal( 'foo' );
+
+			expect( docFrag.getChild( 1 ).name ).to.equal( 'p' );
+			expect( docFrag.getChild( 1 ).maxOffset ).to.equal( 3 );
+			expect( docFrag.getChild( 1 ).getChild( 0 ).data ).to.equal( 'bar' );
+		} );
+
 		it( 'should create an empty paragraph if we split at the end', () => {
 			split( new Position( root, [ 0, 6 ] ) );
 
@@ -1794,6 +1831,16 @@ describe( 'Writer', () => {
 			expect( root.getChild( 2 ).data ).to.equal( 'ar' );
 		} );
 
+		it( 'should wrap inside document fragment', () => {
+			const docFrag = new DocumentFragment( new Text( 'foo' ) );
+
+			writer.wrap( Range.createIn( docFrag ), 'p' );
+
+			expect( docFrag.maxOffset ).to.equal( 1 );
+			expect( docFrag.getChild( 0 ).name ).to.equal( 'p' );
+			expect( docFrag.getChild( 0 ).getChild( 0 ).data ).to.equal( 'foo' );
+		} );
+
 		it( 'should throw if range to wrap is not flat', () => {
 			root.insertChildren( 1, [ new Element( 'p', [], new Text( 'xyz' ) ) ] );
 			const notFlatRange = new Range( new Position( root, [ 3 ] ), new Position( root, [ 6, 2 ] ) );
@@ -1846,6 +1893,15 @@ describe( 'Writer', () => {
 			expect( root.getChild( 0 ).data ).to.equal( 'axyzb' );
 		} );
 
+		it( 'should unwrap inside document fragment', () => {
+			const docFrag = new DocumentFragment( new Element( 'p', null, new Text( 'foo' ) ) );
+
+			writer.unwrap( docFrag.getChild( 0 ) );
+
+			expect( docFrag.maxOffset ).to.equal( 3 );
+			expect( docFrag.getChild( 0 ).data ).to.equal( 'foo' );
+		} );
+
 		it( 'should throw if element to unwrap has no parent', () => {
 			const element = new Element( 'p' );