Sfoglia il codice sorgente

Merge pull request #499 from ckeditor/t/undo-2

T/undo 2
Piotr Jasiun 9 anni fa
parent
commit
2e96d7539a

+ 2 - 1
packages/ckeditor5-engine/src/datacontroller.js

@@ -171,7 +171,8 @@ export default class DataController {
 		const modelRoot = this.model.getRoot( rootName );
 
 		this.model.enqueueChanges( () => {
-			this.model.batch()
+			// Initial batch should be ignored by features like undo, etc.
+			this.model.batch( 'transparent' )
 				.remove( ModelRange.createFromElement( modelRoot ) )
 				.insert( ModelPosition.createAt( modelRoot, 0 ), this.parse( data ) );
 		} );

+ 31 - 7
packages/ckeditor5-engine/src/model/batch.js

@@ -35,27 +35,51 @@ export default class Batch {
 	 * Creates Batch instance. Not recommended to use directly, use {@link engine.model.Document#batch} instead.
 	 *
 	 * @param {engine.model.Document} doc Document which this Batch changes.
+	 * @param {'transparent'|'default'} [type='default'] Type of the batch.
 	 */
-	constructor( doc ) {
+	constructor( doc, type = 'default' ) {
 		/**
-		 * Document which this Batch changes.
+		 * Document which this batch changes.
 		 *
-		 * @member {engine.model.Document} engine.model.Batch#doc
 		 * @readonly
+		 * @member {engine.model.Document} engine.model.Batch#doc
 		 */
 		this.doc = doc;
 
 		/**
-		 * Array of deltas which compose Batch.
+		 * Array of deltas which compose this batch.
 		 *
-		 * @member {Array.<engine.model.delta.Delta>} engine.model.Batch#deltas
 		 * @readonly
+		 * @member {Array.<engine.model.delta.Delta>} engine.model.Batch#deltas
 		 */
 		this.deltas = [];
+
+		/**
+		 * Type of the batch.
+		 *
+		 * Can be one of the following values:
+		 * * `'default'` - all "normal" batches, most commonly used type.
+		 * * `'transparent'` - batch that should be ignored by other features, i.e. initial batch or collaborative editing changes.
+		 *
+		 * @readonly
+		 * @member {'transparent'|'default'} engine.model.Batch#type
+		 */
+		this.type = type;
+	}
+
+	/**
+	 * 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`.
+	 *
+	 * @readonly
+	 * @type {Number|null}
+	 */
+	get baseVersion() {
+		return this.deltas.length > 0 ? this.deltas[ 0 ].baseVersion : null;
 	}
 
 	/**
-	 * Adds delta to the Batch instance. All modification methods (insert, remove, split, etc.) use this method
+	 * Adds delta to the batch instance. All modification methods (insert, remove, split, etc.) use this method
 	 * to add created deltas.
 	 *
 	 * @param {engine.model.delta.Delta} delta Delta to add.
@@ -81,7 +105,7 @@ export default class Batch {
 }
 
 /**
- * Function to register Batch methods. To make code scalable Batch do not have modification
+ * Function to register batch methods. To make code scalable Batch do not have modification
  * methods built in. They can be registered using this method.
  *
  * This method checks if there is no naming collision and throws `batch-register-taken` if the method name

+ 10 - 1
packages/ckeditor5-engine/src/model/delta/basic-transformations.js

@@ -230,10 +230,19 @@ addTransformationCase( UnwrapDelta, SplitDelta, ( a, b, isStrong ) => {
 	// If incoming unwrap delta tries to unwrap node that got split we should unwrap the original node and the split copy.
 	// This can be achieved either by reverting split and applying unwrap to singular node, or creating additional unwrap delta.
 	if ( compareArrays( a.position.path, b.position.getParentPath() ) === 'SAME' ) {
-		return [
+		const transformed = [
 			b.getReversed(),
 			a.clone()
 		];
+
+		// It's a kind of magic-magic-magic-maaaaagiiic!
+		transformed[ 1 ].operations[ 1 ].targetPosition.path[ 0 ]++;
+		// But seriously, we have to fix RemoveOperation in the second delta because reversed UnwrapDelta creates
+		// MergeDelta which also has RemoveOperation. Those two operations cannot point to the same "holder" element
+		// in the graveyard, so we fix it by hand. This is the only case where it happens in "special" transformation
+		// cases, and it won't happen for "default" transformation apart of RemoveDelta, where it is okay.
+
+		return transformed;
 	}
 
 	return defaultTransform( a, b, isStrong );

+ 10 - 4
packages/ckeditor5-engine/src/model/document.js

@@ -108,10 +108,12 @@ export default class Document {
 		/**
 		 * Document's history.
 		 *
+		 * **Note:** Be aware that deltas applied to the stored deltas might be removed or changed.
+		 *
 		 * @readonly
 		 * @member {engine.model.History} engine.model.Document#history
 		 */
-		this.history = new History();
+		this.history = new History( this );
 	}
 
 	/**
@@ -159,7 +161,10 @@ export default class Document {
 
 		this.version++;
 
-		this.history.addOperation( operation );
+		if ( operation.delta ) {
+			// Right now I can't imagine operations without deltas, but let's be safe.
+			this.history.addDelta( operation.delta );
+		}
 
 		const batch = operation.delta && operation.delta.batch;
 
@@ -172,10 +177,11 @@ export default class Document {
 	/**
 	 * Creates a {@link engine.model.Batch} instance which allows to change the document.
 	 *
+	 * @param {String} [type] Batch type. See {@link engine.model.Batch#type}.
 	 * @returns {engine.model.Batch} Batch instance.
 	 */
-	batch() {
-		return new Batch( this );
+	batch( type ) {
+		return new Batch( this, type );
 	}
 
 	/**

+ 135 - 69
packages/ckeditor5-engine/src/model/history.js

@@ -5,17 +5,16 @@
 
 'use strict';
 
-// Load all basic deltas and transformations, they register themselves, but they need to be imported somewhere.
-import deltas from './delta/basic-deltas.js'; // jshint ignore:line
-import transformations from './delta/basic-transformations.js'; // jshint ignore:line
-
-import transform from './delta/transform.js';
 import CKEditorError from '../../utils/ckeditorerror.js';
 
 /**
- * History keeps the track of all the deltas applied to the {@link engine.model.Document document} and provides
- * utility tools to operate on the history. Most of times history is needed to transform a delta that has wrong
- * {@link engine.model.delta.Delta#baseVersion} to a state where it can be applied to the document.
+ * `History` keeps the track of all the deltas applied to the {@link engine.model.Document document}. Deltas stored in
+ * `History` might get updated, split into more deltas or even removed. This is used mostly to compress history, instead
+ * of keeping all deltas in a state in which they were applied.
+ *
+ * **Note:** deltas kept in `History` should be used only to transform deltas. It's not advised to use `History` to get
+ * original delta basing on it's {@link engine.model.delta.Delta#baseVersion baseVersion}. Also, after transforming a
+ * delta by deltas from `History`, fix it's base version accordingly (set to {@link engine.model.Document#version}.
  *
  * @memberOf engine.model
  */
@@ -27,7 +26,7 @@ export default class History {
 		/**
 		 * Deltas added to the history.
 		 *
-		 * @private
+		 * @protected
 		 * @member {Array.<engine.model.delta.Delta>} engine.model.History#_deltas
 		 */
 		this._deltas = [];
@@ -36,109 +35,176 @@ export default class History {
 		 * Helper structure that maps added delta's base version to the index in {@link engine.model.History#_deltas}
 		 * at which the delta was added.
 		 *
-		 * @private
+		 * @protected
 		 * @member {Map} engine.model.History#_historyPoints
 		 */
 		this._historyPoints = new Map();
 	}
 
 	/**
-	 * Gets the number of base version which an up-to-date operation should have.
+	 * Adds delta to the history.
 	 *
-	 * @private
-	 * @type {Number}
+	 * @param {engine.model.delta.Delta} delta Delta to add.
 	 */
-	get _nextHistoryPoint() {
-		const lastDelta = this._deltas[ this._deltas.length - 1 ];
+	addDelta( delta ) {
+		if ( delta.operations.length > 0 && !this._historyPoints.has( delta.baseVersion ) ) {
+			const index = this._deltas.length;
 
-		return lastDelta.baseVersion + lastDelta.operations.length;
+			this._deltas[ index ] = delta;
+			this._historyPoints.set( delta.baseVersion, index );
+		}
 	}
 
 	/**
-	 * Adds an operation to the history.
+	 * Returns deltas added to the history.
 	 *
-	 * @param {engine.model.operation.Operation} operation Operation to add.
+	 * @param {Number} [from=0] Base version from which deltas should be returned (inclusive). Defaults to `0` which means
+	 * that deltas from the first one will be returned.
+	 * @param {Number} [to=Number.POSITIVE_INFINITY] Base version up to which deltas should be returned (exclusive).
+	 * Defaults to `Number.POSITIVE_INFINITY` which means that deltas up to the last one will be returned.
+	 * @returns {Iterator.<engine.model.delta.Delta>} Deltas added to the history.
 	 */
-	addOperation( operation ) {
-		const delta = operation.delta;
-
-		// History cares about deltas not singular operations.
-		// Operations from a delta are added one by one, from first to last.
-		// Operations from one delta cannot be mixed with operations from other deltas.
-		// This all leads us to the conclusion that we could just save deltas history.
-		// What is more, we need to check only the last position in history to check if delta is already in the history.
-		if ( delta && this._deltas[ this._deltas.length - 1 ] !== delta ) {
-			const index = this._deltas.length;
+	*getDeltas( from = 0, to = Number.POSITIVE_INFINITY ) {
+		// No deltas added, nothing to yield.
+		if ( this._deltas.length === 0 ) {
+			return;
+		}
 
-			this._deltas[ index ] = delta;
-			this._historyPoints.set( delta.baseVersion, index );
+		// Will throw if base version is incorrect.
+		let fromIndex = this._getIndex( from );
+
+		// Base version is too low or too high and is not found in history.
+		if ( fromIndex == -1 ) {
+			return;
+		}
+
+		// We have correct `fromIndex` so let's iterate starting from it.
+		while ( fromIndex < this._deltas.length ) {
+			const delta = this._deltas[ fromIndex++ ];
+
+			if ( delta.baseVersion >= to ) {
+				break;
+			}
+
+			yield delta;
 		}
 	}
 
 	/**
-	 * Transforms out-dated delta by all deltas that were added to the history since the given delta's base version. In other
-	 * words, it makes the delta up-to-date with the history. The transformed delta(s) is (are) ready to be applied
-	 * to the {@link engine.model.Document document}.
+	 * Returns one or more deltas from history that bases on given `baseVersion`. Most often it will be just
+	 * one delta, but if that delta got updated by multiple deltas, all of those updated deltas will be returned.
 	 *
-	 * @param {engine.model.delta.Delta} delta Delta to update.
-	 * @returns {Array.<engine.model.delta.Delta>} Result of transformation which is an array containing one or more deltas.
+	 * @see engine.model.History#updateDelta
+	 * @param {Number} baseVersion Base version of the delta to retrieve.
+	 * @returns {Array.<engine.model.delta.Delta>|null} Delta with given base version or null if no such delta is in history.
 	 */
-	getTransformedDelta( delta ) {
-		if ( delta.baseVersion === this._nextHistoryPoint ) {
-			return [ delta ];
+	getDelta( baseVersion ) {
+		let index = this._historyPoints.get( baseVersion );
+
+		if ( index === undefined ) {
+			return null;
 		}
 
-		let transformed = [ delta ];
+		const deltas = [];
 
-		for ( let historyDelta of this.getDeltas( delta.baseVersion ) ) {
-			let allResults = [];
+		for ( index; index < this._deltas.length; index++ ) {
+			const delta = this._deltas[ index ];
 
-			for ( let deltaToTransform of transformed ) {
-				const transformedDelta = History._transform( deltaToTransform, historyDelta );
-				allResults = allResults.concat( transformedDelta );
+			if ( delta.baseVersion != baseVersion ) {
+				break;
 			}
 
-			transformed = allResults;
+			deltas.push( delta );
 		}
 
-		// Fix base versions.
-		let baseVersion = transformed[ 0 ].operations[ 0 ].baseVersion;
-
-		for ( let i = 0; i < transformed.length; i++ ) {
-			transformed[ i ].baseVersion = baseVersion;
-			baseVersion += transformed[ i ].operations.length;
-		}
+		return deltas.length === 0 ? null : deltas;
+	}
 
-		return transformed;
+	/**
+	 * Removes delta from the history. This happens i.e., when a delta is undone by another delta. Both undone delta and
+	 * undoing delta should be removed so they won't have an impact on transforming other deltas.
+	 *
+	 * **Note:** using this method does not change the state of {@link engine.model.Document model}. It just affects
+	 * the state of `History`.
+	 *
+	 * **Note:** when some deltas are removed, deltas between them should probably get updated. See
+	 * {@link engine.model.History#updateDelta}.
+	 *
+	 * **Note:** if delta with `baseVersion` got {@link engine.model.History#updateDelta updated} by multiple
+	 * deltas, all updated deltas will be removed.
+	 *
+	 * @param {Number} baseVersion Base version of a delta to be removed.
+	 */
+	removeDelta( baseVersion ) {
+		this.updateDelta( baseVersion, [] );
 	}
 
 	/**
-	 * Returns all deltas from history, starting from given history point (if passed).
+	 * Substitutes delta in history by one or more given deltas.
+	 *
+	 * **Note:** if delta with `baseVersion` was already updated by multiple deltas, all updated deltas will be removed
+	 * and new deltas will be inserted at their position.
 	 *
-	 * @param {Number} from History point.
-	 * @returns {Iterator.<engine.model.delta.Delta>} Deltas from given history point to the end of history.
+	 * **Note:** delta marked as reversed won't get updated.
+	 *
+	 * @param {Number} baseVersion Base version of a delta to update.
+	 * @param {Iterable.<engine.model.delta.Delta>} updatedDeltas Deltas to be inserted in place of updated delta.
 	 */
-	*getDeltas( from = 0 ) {
-		let i = this._historyPoints.get( from );
+	updateDelta( baseVersion, updatedDeltas ) {
+		const deltas = this.getDelta( baseVersion );
 
-		if ( i === undefined ) {
-			throw new CKEditorError( 'history-wrong-version: Cannot retrieve given point in the history.' );
+		// If there are no deltas, stop executing function as there is nothing to update.
+		if ( deltas === null ) {
+			return;
 		}
 
-		for ( ; i < this._deltas.length; i++ ) {
-			yield this._deltas[ i ];
+		// Make sure that every updated delta has correct `baseVersion`.
+		// This is crucial for algorithms in `History` and algorithms using `History`.
+		for ( let delta of updatedDeltas ) {
+			delta.baseVersion = baseVersion;
+		}
+
+		// Put updated deltas in place of old deltas.
+		this._deltas.splice( this._getIndex( baseVersion ), deltas.length, ...updatedDeltas );
+
+		// Update history points.
+		const changeBy = updatedDeltas.length - deltas.length;
+
+		for ( let key of this._historyPoints.keys() ) {
+			if ( key > baseVersion ) {
+				this._historyPoints.set( key, this._historyPoints.get( key ) + changeBy );
+			}
 		}
 	}
 
 	/**
-	 * Transforms given delta by another given delta. Exposed for testing purposes.
+	 * Gets an index in {@link engine.model.History#_deltas} where delta with given `baseVersion` is added.
 	 *
-	 * @protected
-	 * @param {engine.model.delta.Delta} toTransform Delta to be transformed.
-	 * @param {engine.model.delta.Delta} transformBy Delta to transform by.
-	 * @returns {Array.<engine.model.delta.Delta>} Result of the transformation.
+	 * @private
+	 * @param {Number} baseVersion Base version of delta.
 	 */
-	static _transform( toTransform, transformBy ) {
-		return transform( toTransform, transformBy, true );
+	_getIndex( baseVersion ) {
+		let index = this._historyPoints.get( baseVersion );
+
+		// Base version not found - it is either too high or too low, or is in the middle of delta.
+		if ( index === undefined ) {
+			const lastDelta = this._deltas[ this._deltas.length - 1 ];
+			const nextBaseVersion = lastDelta.baseVersion + lastDelta.operations.length;
+
+			if ( baseVersion < 0 || baseVersion >= nextBaseVersion ) {
+				// Base version is too high or too low - it's acceptable situation.
+				// Return -1 because `baseVersion` was correct.
+				return -1;
+			}
+
+			/**
+			 * Given base version points to the middle of a delta.
+			 *
+			 * @error history-wrong-version
+			 */
+			throw new CKEditorError( 'history-wrong-version: Given base version points to the middle of a delta.' );
+		}
+
+		return index;
 	}
 }

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

@@ -7,6 +7,7 @@
 
 import MoveOperation from './moveoperation.js';
 import Position from '../position.js';
+import Element from '../element.js';
 import ReinsertOperation from './reinsertoperation.js';
 
 /**
@@ -25,16 +26,69 @@ export default class RemoveOperation extends MoveOperation {
 	 * @param {Number} baseVersion {@link engine.model.Document#version} on which operation can be applied.
 	 */
 	constructor( position, howMany, baseVersion ) {
-		// Position in a graveyard where nodes were moved.
-		const graveyardPosition = Position.createFromParentAndOffset( position.root.document.graveyard, 0 );
+		const graveyard = position.root.document.graveyard;
 
-		super( position, howMany, graveyardPosition, baseVersion );
+		super( position, howMany, new Position( graveyard, [ graveyard.getChildCount(), 0 ] ), baseVersion );
 	}
 
+	/**
+	 * @inheritDoc
+	 */
 	get type() {
 		return 'remove';
 	}
 
+	/**
+	 * Offset of the graveyard "holder" element, in which nodes removed by this operation are stored.
+	 *
+	 * @protected
+	 * @type {Number}
+	 */
+	get _holderElementOffset() {
+		return this.targetPosition.path[ 0 ];
+	}
+
+	/**
+	 * Sets {@link engine.model.operation.RemoveOperation#_holderElementOffset}.
+	 *
+	 * @protected
+	 * @param {Number} offset
+	 */
+	set _holderElementOffset( offset ) {
+		this.targetPosition.path[ 0 ] = offset;
+	}
+
+	/**
+	 * Flag informing whether this operation should insert "holder" element (`true`) or should remove nodes
+	 * into existing "holder" element (`false`). It is `true` for each `RemoveOperation` that is the first `RemoveOperation`
+	 * in it's delta which points to given holder element.
+	 *
+	 * @protected
+	 * @type {Boolean}
+	 */
+	get _needsHolderElement() {
+		if ( this.delta ) {
+			// Let's look up all operations from this delta in the same order as they are in the delta.
+			for ( let operation of this.delta.operations ) {
+				// We are interested only in `RemoveOperation`s.
+				if ( operation instanceof RemoveOperation ) {
+					// If the first `RemoveOperation` in the delta is this operation, this operation
+					// needs to insert holder element in the graveyard.
+					if ( operation == this ) {
+						return true;
+					} else if ( operation._holderElementOffset == this._holderElementOffset ) {
+						// If there is a `RemoveOperation` in this delta that "points" to the same holder element offset,
+						// that operation will already insert holder element at that offset. We should not create another holder.
+						return false;
+					}
+				}
+			}
+		}
+
+		// By default `RemoveOperation` needs holder element, so set it so, if the operation does not have delta.
+		return true;
+	}
+
 	/**
 	 * @returns {engine.model.operation.ReinsertOperation}
 	 */
@@ -46,7 +100,25 @@ export default class RemoveOperation extends MoveOperation {
 	 * @returns {engine.model.operation.RemoveOperation}
 	 */
 	clone() {
-		return new RemoveOperation( this.sourcePosition, this.howMany, this.baseVersion );
+		let removeOperation = new RemoveOperation( this.sourcePosition, this.howMany, this.baseVersion );
+		removeOperation.targetPosition = Position.createFromPosition( this.targetPosition );
+		removeOperation.movedRangeStart = Position.createFromPosition( this.movedRangeStart );
+
+		return removeOperation;
+	}
+
+	/**
+	 * @inheritDoc
+	 */
+	_execute() {
+		if ( this._needsHolderElement ) {
+			const graveyard = this.targetPosition.root;
+			const holderElement = new Element( '$graveyardHolder' );
+
+			graveyard.insertChildren( this.targetPosition.path[ 0 ], holderElement );
+		}
+
+		return super._execute();
 	}
 
 	/**

+ 40 - 3
packages/ckeditor5-engine/src/model/operation/transform.js

@@ -146,6 +146,18 @@ const ot = {
 			// This will aggregate transformed ranges.
 			let ranges = [];
 
+			// Special case when MoveOperation is in fact a RemoveOperation. RemoveOperation not only moves nodes but also
+			// creates a "holder" element for them in graveyard. If there was a RemoveOperation pointing to an offset
+			// before this AttributeOperation, we have to increment AttributeOperation's offset.
+			if ( b instanceof RemoveOperation && b._needsHolderElement &&
+				a.range.root == b.targetPosition.root && a.range.start.path[ 0 ] >= b._holderElementOffset
+			) {
+				// Do not change original operation!
+				a = a.clone();
+				a.range.start.path[ 0 ]++;
+				a.range.end.path[ 0 ]++;
+			}
+
 			// Difference is a part of changed range that is modified by AttributeOperation but is not affected
 			// by MoveOperation. This can be zero, one or two ranges (if moved range is inside changed range).
 			// Right now we will make a simplification and join difference ranges and transform them as one. We will cover rangeB later.
@@ -243,7 +255,22 @@ const ot = {
 				return [ b.getReversed() ];
 			}
 
-			// If one of operations is actually a remove operation, we force remove operation to be the "stronger" one
+			// Special case when both operations are RemoveOperations. RemoveOperation not only moves nodes but also
+			// (usually) creates a "holder" element for them in graveyard. Each RemoveOperation should move nodes to different
+			// "holder" element. If `a` operation points after `b` operation, we move `a` offset to acknowledge
+			// "holder" element insertion.
+			if ( a instanceof RemoveOperation && b instanceof RemoveOperation && b._needsHolderElement ) {
+				const aTarget = a.targetPosition.path[ 0 ];
+				const bTarget = b.targetPosition.path[ 0 ];
+
+				if ( aTarget > bTarget || ( aTarget == bTarget && isStrong ) ) {
+					// Do not change original operation!
+					a = a.clone();
+					a.targetPosition.path[ 0 ]++;
+				}
+			}
+
+			// If only one of operations is a remove operation, we force remove operation to be the "stronger" one
 			// to provide more expected results.
 			if ( a instanceof RemoveOperation && !( b instanceof RemoveOperation ) ) {
 				isStrong = true;
@@ -290,7 +317,10 @@ const ot = {
 			// transform `a` operation. Normally, when same nodes are moved, we stick with stronger operation's target.
 			// Here it is a move inside larger range so there is no conflict because after all, all nodes from
 			// smaller range will be moved to larger range target. The effect of this transformation feels natural.
-			let aIsInside = rangeB.containsRange( rangeA ) && rangeB.containsPosition( a.targetPosition );
+			// Also if we wouldn't do that, we would get different results on both sides of transformation (i.e. in
+			// collaborative editing).
+			let aIsInside = rangeB.containsRange( rangeA ) &&
+				( rangeB.containsPosition( a.targetPosition ) || rangeB.start.isEqual( a.targetPosition ) || rangeB.end.isEqual( a.targetPosition ) );
 
 			if ( common !== null && ( aCompB === 'EXTENSION' || ( aCompB === 'SAME' && isStrong ) || aIsInside ) && !bTargetsToA ) {
 				// Here we do not need to worry that newTargetPosition is inside moved range, because that
@@ -314,7 +344,13 @@ const ot = {
 			}
 
 			// Target position also could be affected by the other MoveOperation. We will transform it.
-			let newTargetPosition = a.targetPosition.getTransformedByMove( b.sourcePosition, b.targetPosition, b.howMany, !isStrong, b.isSticky );
+			let newTargetPosition = a.targetPosition.getTransformedByMove(
+				b.sourcePosition,
+				b.targetPosition,
+				b.howMany,
+				!isStrong,
+				b.isSticky || aIsInside
+			);
 
 			// Map transformed range(s) to operations and return them.
 			return ranges.reverse().map( ( range ) => {
@@ -327,6 +363,7 @@ const ot = {
 				);
 
 				result.isSticky = a.isSticky;
+				result._holderElementOffset = a._holderElementOffset;
 
 				return result;
 			} );

+ 32 - 0
packages/ckeditor5-engine/tests/model/batch.js

@@ -24,6 +24,20 @@ describe( 'Batch', () => {
 		expect( batch.removeAttr ).to.be.a( 'function' );
 	} );
 
+	describe( 'type', () => {
+		it( 'should default to "default"', () => {
+			const batch = new Batch( new Document() );
+
+			expect( batch.type ).to.equal( 'default' );
+		} );
+
+		it( 'should be set to the value set in constructor', () => {
+			const batch = new Batch( new Document(), 'ignore' );
+
+			expect( batch.type ).to.equal( 'ignore' );
+		} );
+	} );
+
 	describe( 'register', () => {
 		afterEach( () => {
 			delete Batch.prototype.foo;
@@ -86,4 +100,22 @@ describe( 'Batch', () => {
 			expect( batch.getOperations() ).to.have.property( 'next' );
 		} );
 	} );
+
+	describe( 'baseVersion', () => {
+		it( 'should return base version of first delta from the batch', () => {
+			const batch = new Batch( new Document() );
+			const delta = new Delta();
+			const operation = new Operation( 2 );
+			delta.addOperation( operation );
+			batch.addDelta( delta );
+
+			expect( batch.baseVersion ).to.equal( 2 );
+		} );
+
+		it( 'should return null if there are no deltas in batch', () => {
+			const batch = new Batch( new Document() );
+
+			expect( batch.baseVersion ).to.be.null;
+		} );
+	} );
 } );

+ 1 - 1
packages/ckeditor5-engine/tests/model/delta/transform/insertdelta.js

@@ -94,7 +94,7 @@ describe( 'transform', () => {
 					operations: [
 						{
 							type: ReinsertOperation,
-							sourcePosition: new Position( gy, [ 0 ] ),
+							sourcePosition: new Position( gy, [ 0, 0 ] ),
 							howMany: 1,
 							targetPosition: new Position( root, [ 3, 3, 3 ] ),
 							baseVersion: baseVersion

+ 2 - 6
packages/ckeditor5-engine/tests/model/delta/transform/mergedelta.js

@@ -112,14 +112,10 @@ describe( 'transform', () => {
 							baseVersion: baseVersion
 						},
 						{
-							// This is `MoveOperation` instead of `RemoveOperation` because during OT,
-							// `RemoveOperation` may get converted to `MoveOperation`. Still, this expectation is
-							// correct because `RemoveOperation` is deriving from `MoveOperation`. So we can expect
-							// that something that was `RemoveOperation` is a `MoveOperation`.
-							type: MoveOperation,
+							type: RemoveOperation,
 							sourcePosition: new Position( root, [ 3, 3, 3 ] ),
 							howMany: 1,
-							targetPosition: new Position( gy, [ 0 ] ),
+							targetPosition: new Position( gy, [ 0, 0 ] ),
 							baseVersion: baseVersion + 1
 						}
 					]

+ 1 - 1
packages/ckeditor5-engine/tests/model/delta/transform/movedelta.js

@@ -72,7 +72,7 @@ describe( 'transform', () => {
 							// is treated in OT as `MoveOperation` and might be converted to it. This is why we have to
 							// check whether the operation type is `MoveOperation`. This is all perfectly valid.
 							type: MoveOperation,
-							sourcePosition: new Position( gy, [ 0 ] ),
+							sourcePosition: new Position( gy, [ 0, 0 ] ),
 							howMany: 1,
 							targetPosition: new Position( root, [ 3, 3, 3 ] ),
 							baseVersion: baseVersion

+ 1 - 1
packages/ckeditor5-engine/tests/model/delta/transform/removedelta.js

@@ -68,7 +68,7 @@ describe( 'transform', () => {
 					operations: [
 						{
 							type: MoveOperation,
-							sourcePosition: new Position( gy, [ 0 ] ),
+							sourcePosition: new Position( gy, [ 0, 0 ] ),
 							howMany: 1,
 							targetPosition: new Position( root, [ 3, 3, 3 ] ),
 							baseVersion: baseVersion

+ 3 - 4
packages/ckeditor5-engine/tests/model/delta/transform/unwrapdelta.js

@@ -73,7 +73,7 @@ describe( 'transform', () => {
 							type: MoveOperation,
 							sourcePosition: new Position( root, [ 3, 3, 4 ] ),
 							howMany: 1,
-							targetPosition: new Position( gy, [ 0 ] ),
+							targetPosition: new Position( gy, [ 0, 0 ] ),
 							baseVersion: baseVersion + 1
 						}
 					]
@@ -90,11 +90,10 @@ describe( 'transform', () => {
 							baseVersion: baseVersion + 2
 						},
 						{
-							// `RemoveOperation` as `MoveOperation`
 							type: MoveOperation,
 							sourcePosition: new Position( root, [ 3, 3, 15 ] ),
 							howMany: 1,
-							targetPosition: new Position( gy, [ 0 ] ),
+							targetPosition: new Position( gy, [ 1, 0 ] ),
 							baseVersion: baseVersion + 3
 						}
 					]
@@ -135,7 +134,7 @@ describe( 'transform', () => {
 							type: MoveOperation,
 							sourcePosition: new Position( root, [ 3, 4, 12 ] ),
 							howMany: 1,
-							targetPosition: new Position( gy, [ 0 ] ),
+							targetPosition: new Position( gy, [ 0, 0 ] ),
 							baseVersion: baseVersion + 1
 						}
 					]

+ 1 - 1
packages/ckeditor5-engine/tests/model/delta/transform/wrapdelta.js

@@ -77,7 +77,7 @@ describe( 'transform', () => {
 							type: MoveOperation,
 							sourcePosition: new Position( root, [ 3, 3, 4 ] ),
 							howMany: 1,
-							targetPosition: new Position( gy, [ 0 ] ),
+							targetPosition: new Position( gy, [ 0, 0 ] ),
 							baseVersion: baseVersion + 1
 						}
 					]

+ 4 - 2
packages/ckeditor5-engine/tests/model/document/change-event.js

@@ -93,13 +93,15 @@ describe( 'Document change event', () => {
 
 		expect( changes ).to.have.length( 2 );
 
+		const holderElement = graveyard.getChild( 0 );
+
 		expect( types[ 0 ] ).to.equal( 'remove' );
-		expect( changes[ 0 ].range ).to.deep.equal( Range.createFromParentsAndOffsets( graveyard, 0, graveyard, 3 ) );
+		expect( changes[ 0 ].range ).to.deep.equal( Range.createFromParentsAndOffsets( holderElement, 0, holderElement, 3 ) );
 		expect( changes[ 0 ].sourcePosition ).to.deep.equal( Position.createFromParentAndOffset( root, 0 ) );
 
 		expect( types[ 1 ] ).to.equal( 'reinsert' );
 		expect( changes[ 1 ].range ).to.deep.equal( Range.createFromParentsAndOffsets( root, 0, root, 3 ) );
-		expect( changes[ 1 ].sourcePosition ).to.deep.equal( Position.createFromParentAndOffset( graveyard, 0 ) );
+		expect( changes[ 1 ].sourcePosition ).to.deep.equal( Position.createFromParentAndOffset( holderElement, 0 ) );
 	} );
 
 	it( 'should be fired when attribute is inserted', () => {

+ 12 - 2
packages/ckeditor5-engine/tests/model/document/document.js

@@ -12,6 +12,7 @@ import Schema from '/ckeditor5/engine/model/schema.js';
 import Composer from '/ckeditor5/engine/model/composer/composer.js';
 import RootElement from '/ckeditor5/engine/model/rootelement.js';
 import Batch from '/ckeditor5/engine/model/batch.js';
+import Delta from '/ckeditor5/engine/model/delta/delta.js';
 import CKEditorError from '/ckeditor5/utils/ckeditorerror.js';
 import count from '/ckeditor5/utils/count.js';
 
@@ -114,15 +115,18 @@ describe( 'Document', () => {
 			const changeCallback = sinon.spy();
 			const type = 't';
 			const data = { data: 'x' };
-			const batch = 'batch';
+			const batch = new Batch();
+			const delta = new Delta();
 
 			let operation = {
 				type: type,
-				delta: { batch: batch },
 				baseVersion: 0,
 				_execute: sinon.stub().returns( data )
 			};
 
+			delta.addOperation( operation );
+			batch.addDelta( delta );
+
 			doc.on( 'change', changeCallback );
 			doc.applyOperation( operation );
 
@@ -155,6 +159,12 @@ describe( 'Document', () => {
 			expect( batch ).to.be.instanceof( Batch );
 			expect( batch ).to.have.property( 'doc' ).that.equals( doc );
 		} );
+
+		it( 'should set given batch type', () => {
+			const batch = doc.batch( 'ignore' );
+
+			expect( batch ).to.have.property( 'type' ).that.equals( 'ignore' );
+		} );
 	} );
 
 	describe( 'enqueue', () => {

+ 158 - 108
packages/ckeditor5-engine/tests/model/history.js

@@ -7,7 +7,8 @@
 
 import History from '/ckeditor5/engine/model/history.js';
 import Delta from '/ckeditor5/engine/model/delta/delta.js';
-import NoOperation from '/ckeditor5/engine/model/operation/nooperation.js';
+import Operation from '/ckeditor5/engine/model/operation/operation.js';
+
 import CKEditorError from '/ckeditor5/utils/ckeditorerror.js';
 
 describe( 'History', () => {
@@ -19,166 +20,215 @@ describe( 'History', () => {
 
 	describe( 'constructor', () => {
 		it( 'should create an empty History instance', () => {
-			expect( history._deltas.length ).to.equal( 0 );
-			expect( history._historyPoints.size ).to.equal( 0 );
+			expect( Array.from( history.getDeltas() ).length ).to.equal( 0 );
 		} );
 	} );
 
-	describe( 'addOperation', () => {
-		it( 'should save delta containing passed operation in the history', () => {
+	describe( 'addDelta', () => {
+		it( 'should save delta in the history', () => {
 			let delta = new Delta();
-			let operation = new NoOperation( 0 );
+			delta.addOperation( new Operation( 0 ) );
 
-			delta.addOperation( operation );
-			history.addOperation( operation );
+			history.addDelta( delta );
 
-			expect( history._deltas.length ).to.equal( 1 );
-			expect( history._deltas[ 0 ] ).to.equal( delta );
+			const deltas = Array.from( history.getDeltas() );
+			expect( deltas.length ).to.equal( 1 );
+			expect( deltas[ 0 ] ).to.equal( delta );
 		} );
 
 		it( 'should save each delta only once', () => {
 			let delta = new Delta();
+			delta.addOperation( new Operation( 0 ) );
+
+			history.addDelta( delta );
+			history.addDelta( delta );
+
+			const deltas = Array.from( history.getDeltas() );
+			expect( deltas.length ).to.equal( 1 );
+			expect( deltas[ 0 ] ).to.equal( delta );
+		} );
 
-			delta.addOperation( new NoOperation( 0 ) );
-			delta.addOperation( new NoOperation( 1 ) );
-			delta.addOperation( new NoOperation( 2 ) );
+		it( 'should save multiple deltas and keep their order', () => {
+			let deltas = getDeltaSet();
 
-			for ( let operation of delta.operations ) {
-				history.addOperation( operation );
+			for ( let delta of deltas ) {
+				history.addDelta( delta );
 			}
 
-			expect( history._deltas.length ).to.equal( 1 );
-			expect( history._deltas[ 0 ] ).to.equal( delta );
+			const historyDeltas = Array.from( history.getDeltas() );
+			expect( historyDeltas ).to.deep.equal( deltas );
 		} );
 
-		it( 'should save multiple deltas and keep their order', () => {
-			let deltaA = new Delta();
-			let deltaB = new Delta();
-			let deltaC = new Delta();
+		it( 'should skip deltas that does not have operations', () => {
+			let delta = new Delta();
 
-			let deltas = [ deltaA, deltaB, deltaC ];
+			history.addDelta( delta );
 
-			let i = 0;
+			expect( Array.from( history.getDeltas() ).length ).to.equal( 0 );
+		} );
+	} );
 
-			for ( let delta of deltas ) {
-				delta.addOperation( new NoOperation( i++ ) );
-				delta.addOperation( new NoOperation( i++ ) );
-			}
+	describe( 'getDelta', () => {
+		it( 'should return array with one delta with given base version', () => {
+			let delta = getDelta( 0 );
+			history.addDelta( delta );
 
-			for ( let delta of deltas ) {
-				for ( let operation of delta.operations ) {
-					history.addOperation( operation );
-				}
-			}
+			const historyDelta = history.getDelta( 0 );
+			expect( historyDelta ).to.deep.equal( [ delta ] );
+		} );
+
+		it( 'should return array with all updated deltas of delta with given base version', () => {
+			let delta = getDelta( 0 );
+			history.addDelta( delta );
+
+			let deltas = getDeltaSet();
+			history.updateDelta( 0, deltas );
+
+			const historyDelta = history.getDelta( 0 );
+			expect( historyDelta ).to.deep.equal( deltas );
+		} );
+
+		it( 'should return null if delta has not been found in history', () => {
+			expect( history.getDelta( -1 ) ).to.be.null;
+			expect( history.getDelta( 2 ) ).to.be.null;
+			expect( history.getDelta( 20 ) ).to.be.null;
+		} );
 
-			expect( history._deltas.length ).to.equal( 3 );
-			expect( history._deltas[ 0 ] ).to.equal( deltaA );
-			expect( history._deltas[ 1 ] ).to.equal( deltaB );
-			expect( history._deltas[ 2 ] ).to.equal( deltaC );
+		it( 'should return null if delta has been removed by removeDelta', () => {
+			let delta = getDelta( 0 );
+			history.addDelta( delta );
+			history.removeDelta( 0 );
+
+			expect( history.getDelta( 0 ) ).to.be.null;
 		} );
 	} );
 
-	describe( 'getTransformedDelta', () => {
-		it( 'should transform given delta by deltas from history which were applied since the baseVersion of given delta', () => {
-			sinon.spy( History, '_transform' );
+	describe( 'getDeltas', () => {
+		let deltas;
 
-			let deltaA = new Delta();
-			deltaA.addOperation( new NoOperation( 0 ) );
+		beforeEach( () => {
+			deltas = getDeltaSet();
 
-			let deltaB = new Delta();
-			deltaB.addOperation( new NoOperation( 1 ) );
+			for ( let delta of deltas ) {
+				history.addDelta( delta );
+			}
+		} );
 
-			let deltaC = new Delta();
-			deltaC.addOperation( new NoOperation( 2 ) );
+		it( 'should return only history deltas from given base version', () => {
+			const historyDeltas = Array.from( history.getDeltas( 3 ) );
+			expect( historyDeltas ).to.deep.equal( deltas.slice( 1 ) );
+		} );
 
-			let deltaD = new Delta();
-			deltaD.addOperation( new NoOperation( 3 ) );
+		it( 'should return only history deltas to given base version', () => {
+			const historyDeltas = Array.from( history.getDeltas( 3, 6 ) );
+			expect( historyDeltas ).to.deep.equal( deltas.slice( 1, 2 ) );
+		} );
 
-			let deltaX = new Delta();
-			deltaX.addOperation( new NoOperation( 1 ) );
+		it( 'should return empty (finished) iterator if given history point is too high or negative', () => {
+			expect( Array.from( history.getDeltas( 20 ) ).length ).to.equal( 0 );
+			expect( Array.from( history.getDeltas( -1 ) ).length ).to.equal( 0 );
+		} );
 
-			history.addOperation( deltaA.operations[ 0 ] );
-			history.addOperation( deltaB.operations[ 0 ] );
-			history.addOperation( deltaC.operations[ 0 ] );
-			history.addOperation( deltaD.operations[ 0 ] );
+		it( 'should throw if given history point is "inside" delta', () => {
+			expect( () => {
+				Array.from( history.getDeltas( 2 ) );
+			} ).to.throw( CKEditorError, /history-wrong-version/ );
+		} );
+	} );
 
-			// `deltaX` bases on the same history point as `deltaB` -- so it already acknowledges `deltaA` existence.
-			// It should be transformed by `deltaB` and all following deltas (`deltaC` and `deltaD`).
-			history.getTransformedDelta( deltaX );
+	describe( 'updateDelta', () => {
+		it( 'should substitute delta from history by given deltas', () => {
+			history.addDelta( getDelta( 0 ) );
 
-			// `deltaX` was not transformed by `deltaA`.
-			expect( History._transform.calledWithExactly( deltaX, deltaA ) ).to.be.false;
+			const deltas = getDeltaSet();
+			history.updateDelta( 0, deltas );
 
-			expect( History._transform.calledWithExactly( deltaX, deltaB ) ).to.be.true;
-			// We can't do exact call matching because after first transformation, what we are further transforming
-			// is no longer `deltaX` but a result of transforming `deltaX` and `deltaB`.
-			expect( History._transform.calledWithExactly( sinon.match.instanceOf( Delta ), deltaC ) ).to.be.true;
-			expect( History._transform.calledWithExactly( sinon.match.instanceOf( Delta ), deltaD ) ).to.be.true;
+			const historyDeltas = Array.from( history.getDeltas() );
+			expect( historyDeltas ).to.deep.equal( deltas );
 		} );
 
-		it( 'should correctly set base versions if multiple deltas are result of transformation', () => {
-			// Let's stub History._transform so it will always return two deltas with two operations each.
-			History._transform = function() {
-				let resultA = new Delta();
-				resultA.addOperation( new NoOperation( 1 ) );
-				resultA.addOperation( new NoOperation( 1 ) );
+		it( 'should substitute all updated deltas by new deltas', () => {
+			history.addDelta( getDelta( 0 ) );
 
-				let resultB = new Delta();
-				resultB.addOperation( new NoOperation( 1 ) );
-				resultB.addOperation( new NoOperation( 1 ) );
+			// Change original single delta to three deltas generated by `getDeltaSet`.
+			// All those deltas should now be seen under base version 0.
+			history.updateDelta( 0, getDeltaSet() );
 
-				return [ resultA, resultB ];
-			};
+			const deltas = getDeltaSet();
+			// Change all three deltas from base version 0 to new set of deltas.
+			history.updateDelta( 0, deltas );
 
-			let deltaA = new Delta();
-			deltaA.addOperation( new NoOperation( 0 ) );
+			const historyDeltas = Array.from( history.getDeltas() );
+			expect( historyDeltas ).to.deep.equal( deltas );
+		} );
 
-			let deltaX = new Delta();
-			deltaX.addOperation( new NoOperation( 0 ) );
+		it( 'should do nothing if deltas for given base version has not been found in history', () => {
+			history.addDelta( getDelta( 0 ) );
+			history.removeDelta( 0 );
 
-			history.addOperation( deltaA.operations[ 0 ] );
+			const deltas = getDeltaSet();
 
-			let result = history.getTransformedDelta( deltaX );
+			history.updateDelta( 0, deltas );
 
-			expect( result[ 0 ].operations[ 0 ].baseVersion ).to.equal( 1 );
-			expect( result[ 0 ].operations[ 1 ].baseVersion ).to.equal( 2 );
-			expect( result[ 1 ].operations[ 0 ].baseVersion ).to.equal( 3 );
-			expect( result[ 1 ].operations[ 1 ].baseVersion ).to.equal( 4 );
+			expect( Array.from( history.getDeltas() ).length ).to.equal( 0 );
 		} );
+	} );
+
+	describe( 'removeDelta', () => {
+		it( 'should remove deltas that do not have graveyard related operations', () => {
+			for ( let delta of getDeltaSet() ) {
+				history.addDelta( delta );
+			}
 
-		it( 'should not transform given delta if it bases on current version of history', () => {
-			let deltaA = new Delta();
-			deltaA.addOperation( new NoOperation( 0 ) );
+			history.removeDelta( 3 );
 
-			let deltaB = new Delta();
-			let opB = new NoOperation( 1 );
-			deltaB.addOperation( opB );
+			const deltas = Array.from( history.getDeltas() );
+			expect( deltas.length ).to.equal( 2 );
+		} );
+
+		it( 'should remove multiple updated deltas', () => {
+			let delta = getDelta( 0 );
+			history.addDelta( delta );
 
-			history.addOperation( deltaA.operations[ 0 ] );
+			let updatedDeltas = getDeltaSet( 0 );
 
-			let result = history.getTransformedDelta( deltaB );
+			history.updateDelta( 0, updatedDeltas );
+			history.removeDelta( 0 );
 
-			expect( result.length ).to.equal( 1 );
-			expect( result[ 0 ] ).to.equal( deltaB );
-			expect( result[ 0 ].operations[ 0 ] ).to.equal( opB );
+			const deltas = Array.from( history.getDeltas() );
+			expect( deltas.length ).to.equal( 0 );
 		} );
 
-		it( 'should throw if given delta bases on an incorrect version of history', () => {
-			let deltaA = new Delta();
-			deltaA.addOperation( new NoOperation( 0 ) );
-			deltaA.addOperation( new NoOperation( 1 ) );
+		it( 'should do nothing if deltas for given base version has not been found in history', () => {
+			const deltas = getDeltaSet();
 
-			history.addOperation( deltaA.operations[ 0 ] );
-			history.addOperation( deltaA.operations[ 1 ] );
+			for ( let delta of deltas ) {
+				history.addDelta( delta );
+			}
 
-			let deltaB = new Delta();
-			// Wrong base version - should be either 0 or 2, operation can't be based on an operation that is
-			// in the middle of other delta, because deltas are atomic, not dividable structures.
-			deltaB.addOperation( new NoOperation( 1 ) );
+			history.removeDelta( 12 );
 
-			expect( () => {
-				history.getTransformedDelta( deltaB );
-			} ).to.throw( CKEditorError, /history-wrong-version/ );
+			expect( Array.from( history.getDeltas() ) ).to.deep.equal( deltas );
 		} );
 	} );
 } );
+
+function getDeltaSet() {
+	const deltas = [];
+
+	deltas.push( getDelta( 0 ) );
+	deltas.push( getDelta( 3 ) );
+	deltas.push( getDelta( 6 ) );
+
+	return deltas;
+}
+
+function getDelta( baseVersion ) {
+	const delta = new Delta();
+
+	for ( let i = 0; i < 3; i++ ) {
+		delta.addOperation( new Operation( i + baseVersion ) );
+	}
+
+	return delta;
+}

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

@@ -12,6 +12,7 @@ import ReinsertOperation from '/ckeditor5/engine/model/operation/reinsertoperati
 import RemoveOperation from '/ckeditor5/engine/model/operation/removeoperation.js';
 import MoveOperation from '/ckeditor5/engine/model/operation/moveoperation.js';
 import Position from '/ckeditor5/engine/model/position.js';
+import Element from '/ckeditor5/engine/model/element.js';
 import { jsonParseStringify } from '/tests/engine/model/_utils/utils.js';
 
 describe( 'ReinsertOperation', () => {
@@ -22,7 +23,7 @@ describe( 'ReinsertOperation', () => {
 		root = doc.createRoot();
 		graveyard = doc.graveyard;
 
-		graveyardPosition = new Position( graveyard, [ 0 ] );
+		graveyardPosition = new Position( graveyard, [ 0, 0 ] );
 		rootPosition = new Position( root, [ 0 ] );
 
 		operation = new ReinsertOperation(
@@ -70,28 +71,27 @@ describe( 'ReinsertOperation', () => {
 		expect( reverse.baseVersion ).to.equal( 1 );
 		expect( reverse.howMany ).to.equal( 2 );
 		expect( reverse.sourcePosition.isEqual( rootPosition ) ).to.be.true;
-		expect( reverse.targetPosition.isEqual( graveyardPosition ) ).to.be.true;
+		expect( reverse.targetPosition.root ).to.equal( graveyardPosition.root );
 	} );
 
 	it( 'should undo reinsert set of nodes by applying reverse operation', () => {
 		let reverse = operation.getReversed();
 
-		graveyard.insertChildren( 0, 'bar' );
+		const element = new Element();
+		element.insertChildren( 0, 'xx' );
+		graveyard.insertChildren( 0, element );
 
 		doc.applyOperation( operation );
 
 		expect( doc.version ).to.equal( 1 );
 		expect( root.getChildCount() ).to.equal( 2 );
+		expect( element.getChildCount() ).to.equal( 0 );
 
 		doc.applyOperation( reverse );
 
 		expect( doc.version ).to.equal( 2 );
 		expect( root.getChildCount() ).to.equal( 0 );
-		expect( graveyard.getChildCount() ).to.equal( 3 );
-
-		expect( graveyard.getChild( 0 ).character ).to.equal( 'b' );
-		expect( graveyard.getChild( 1 ).character ).to.equal( 'a' );
-		expect( graveyard.getChild( 2 ).character ).to.equal( 'r' );
+		// Don't check `element` - nodes are moved to new holder element.
 	} );
 
 	describe( 'toJSON', () => {

+ 37 - 4
packages/ckeditor5-engine/tests/model/operation/removeoperation.js

@@ -53,7 +53,7 @@ describe( 'RemoveOperation', () => {
 		expect( operation ).to.be.instanceof( MoveOperation );
 	} );
 
-	it( 'should remove set of nodes and append them to graveyard root', () => {
+	it( 'should remove set of nodes and append them to holder element in graveyard root', () => {
 		root.insertChildren( 0, 'fozbar' );
 
 		doc.applyOperation(
@@ -68,9 +68,42 @@ describe( 'RemoveOperation', () => {
 		expect( root.getChildCount() ).to.equal( 4 );
 		expect( root.getChild( 2 ).character ).to.equal( 'a' );
 
-		expect( graveyard.getChildCount() ).to.equal( 2 );
-		expect( graveyard.getChild( 0 ).character ).to.equal( 'z' );
-		expect( graveyard.getChild( 1 ).character ).to.equal( 'b' );
+		expect( graveyard.getChildCount() ).to.equal( 1 );
+		expect( graveyard.getChild( 0 ).getChild( 0 ).character ).to.equal( 'z' );
+		expect( graveyard.getChild( 0 ).getChild( 1 ).character ).to.equal( 'b' );
+	} );
+
+	it( 'should create new holder element for each remove operation', () => {
+		root.insertChildren( 0, 'fozbar' );
+
+		doc.applyOperation(
+			new RemoveOperation(
+				new Position( root, [ 0 ] ),
+				1,
+				doc.version
+			)
+		);
+
+		doc.applyOperation(
+			new RemoveOperation(
+				new Position( root, [ 0 ] ),
+				1,
+				doc.version
+			)
+		);
+
+		doc.applyOperation(
+			new RemoveOperation(
+				new Position( root, [ 0 ] ),
+				1,
+				doc.version
+			)
+		);
+
+		expect( graveyard.getChildCount() ).to.equal( 3 );
+		expect( graveyard.getChild( 0 ).getChild( 0 ).character ).to.equal( 'f' );
+		expect( graveyard.getChild( 1 ).getChild( 0 ).character ).to.equal( 'o' );
+		expect( graveyard.getChild( 2 ).getChild( 0 ).character ).to.equal( 'z' );
 	} );
 
 	it( 'should create RemoveOperation with same parameters when cloned', () => {

+ 58 - 3
packages/ckeditor5-engine/tests/model/operation/transform.js

@@ -7,22 +7,27 @@
 
 'use strict';
 
+import transform from '/ckeditor5/engine/model/operation/transform.js';
+
+import Document from '/ckeditor5/engine/model/document.js';
 import RootElement from '/ckeditor5/engine/model/rootelement.js';
 import Node from '/ckeditor5/engine/model/node.js';
 import Position from '/ckeditor5/engine/model/position.js';
 import Range from '/ckeditor5/engine/model/range.js';
-import transform from '/ckeditor5/engine/model/operation/transform.js';
+
 import InsertOperation from '/ckeditor5/engine/model/operation/insertoperation.js';
 import AttributeOperation from '/ckeditor5/engine/model/operation/attributeoperation.js';
 import RootAttributeOperation from '/ckeditor5/engine/model/operation/rootattributeoperation.js';
 import MoveOperation from '/ckeditor5/engine/model/operation/moveoperation.js';
+import RemoveOperation from '/ckeditor5/engine/model/operation/removeoperation.js';
 import NoOperation from '/ckeditor5/engine/model/operation/nooperation.js';
 
 describe( 'transform', () => {
-	let root, op, nodeA, nodeB, expected, baseVersion;
+	let doc, root, op, nodeA, nodeB, expected, baseVersion;
 
 	beforeEach( () => {
-		root = new RootElement( null );
+		doc = new Document();
+		root = doc.createRoot();
 
 		nodeA = new Node();
 		nodeB = new Node();
@@ -1330,6 +1335,56 @@ describe( 'transform', () => {
 				} );
 			} );
 		} );
+
+		describe( 'by RemoveOperation', () => {
+			beforeEach( () => {
+				start = new Position( doc.graveyard, [ 2, 0 ] );
+				end = new Position( doc.graveyard, [ 2, 4 ] );
+
+				range = new Range( start, end );
+
+				op = new AttributeOperation( range, 'foo', 'abc', 'bar', baseVersion );
+
+				expected.range = new Range( start, end );
+			} );
+
+			it( 'remove operation inserted holder element before attribute operation range: increment path', () => {
+				let transformBy = new RemoveOperation(
+					new Position( root, [ 0 ] ),
+					2,
+					baseVersion
+				);
+
+				transformBy.targetPosition.path = [ 0, 0 ];
+				transformBy.movedRangeStart.path = [ 0, 0 ];
+
+				let transOp = transform( op, transformBy );
+
+				expect( transOp.length ).to.equal( 1 );
+
+				expected.range.start.path = [ 3, 0 ];
+				expected.range.end.path = [ 3, 4 ];
+
+				expectOperation( transOp[ 0 ], expected );
+			} );
+
+			it( 'remove operation inserted holder element after attribute operation range: do nothing', () => {
+				let transformBy = new RemoveOperation(
+					new Position( root, [ 0 ] ),
+					2,
+					baseVersion
+				);
+
+				transformBy.targetPosition.path = [ 4, 0 ];
+				transformBy.movedRangeStart.path = [ 4, 0 ];
+
+				let transOp = transform( op, transformBy );
+
+				expect( transOp.length ).to.equal( 1 );
+
+				expectOperation( transOp[ 0 ], expected );
+			} );
+		} );
 	} );
 
 	describe( 'RootAttributeOperation', () => {