Merge pull request #977 from ckeditor/remove-refactor

Feature: OT will use context information to achieve better conflict resolution.

This change includes refactoring of: `History`, `RemoveOperation`, operational transformation algorithms, delta transformation algorithms and more.

Context information will be used instead of removing deltas from history, which caused bugs in more complicated scenarios. This mostly affects undo algorithms.

BREAKING CHANGE: `History` API for deleting undone deltas has been removed.
BREAKING CHANGE: `deltaTransform#transformDeltaSets()` is now an internal method. Use `Document#transformDeltas()` instead.

src/conversion/modelconversiondispatcher.js

@@ -224,7 +224,7 @@ export default class ModelConversionDispatcher {
 			}
 
 			// Check if inserted content contains a marker.
-			if ( range.containsRange( markerRange ) || range.isEqual( markerRange ) ) {
+			if ( range.containsRange( markerRange, true ) ) {
 				this.convertMarker( 'addMarker', marker.name, markerRange );
 			}
 		}

src/dev-utils/enableenginedebug.js

@@ -287,7 +287,7 @@ function enableLoggingTools() {
 		const range = ModelRange.createFromPositionAndShift( this.sourcePosition, this.howMany );
 
 		return getClassName( this ) + `( ${ this.baseVersion } ): ` +
-			`${ range } -> ${ this.targetPosition }`;
+			`${ range } -> ${ this.targetPosition }${ this.isSticky ? ' (sticky)' : '' }`;
 	};
 
 	NoOperation.prototype.toString = function() {
@@ -334,15 +334,15 @@ function enableLoggingTools() {
 
 	const _deltaTransformTransform = deltaTransform.transform;
 
-	deltaTransform.transform = function( a, b, isAMoreImportantThanB ) {
+	deltaTransform.transform = function( a, b, context ) {
 		let results;
 
 		try {
-			results = _deltaTransformTransform( a, b, isAMoreImportantThanB );
+			results = _deltaTransformTransform( a, b, context );
 		} catch ( e ) {
 			logger.error( 'Error during delta transformation!' );
-			logger.error( a.toString() + ( isAMoreImportantThanB ? ' (important)' : '' ) );
-			logger.error( b.toString() + ( isAMoreImportantThanB ? '' : ' (important)' ) );
+			logger.error( a.toString() + ( context.isStrong ? ' (important)' : '' ) );
+			logger.error( b.toString() + ( context.isStrong ? '' : ' (important)' ) );
 
 			throw e;
 		}
@@ -351,7 +351,7 @@ function enableLoggingTools() {
 			results[ i ]._saveHistory( {
 				before: a,
 				transformedBy: b,
-				wasImportant: !!isAMoreImportantThanB,
+				wasImportant: !!context.isStrong,
 				resultIndex: i,
 				resultsTotal: results.length
 			} );

src/model/delta/attributedelta.js

@@ -12,6 +12,7 @@ import DeltaFactory from './deltafactory';
 import { register } from '../batch';
 import AttributeOperation from '../operation/attributeoperation';
 import RootAttributeOperation from '../operation/rootattributeoperation';
+import NoOperation from '../operation/nooperation';
 import Position from '../position';
 import Range from '../range';
 
@@ -65,6 +66,10 @@ export default class AttributeDelta extends Delta {
 		let end = null;
 
 		for ( const operation of this.operations ) {
+			if ( operation instanceof NoOperation ) {
+				continue;
+			}
+
 			if ( start === null || start.isAfter( operation.range.start ) ) {
 				start = operation.range.start;
 			}
@@ -104,13 +109,6 @@ export default class AttributeDelta extends Delta {
 	static get className() {
 		return 'engine.model.delta.AttributeDelta';
 	}
-
-	/**
-	 * @inheritDoc
-	 */
-	static get _priority() {
-		return 20;
-	}
 }
 
 /**

src/model/delta/delta.js

@@ -162,18 +162,6 @@ export default class Delta {
 	static get className() {
 		return 'engine.model.delta.Delta';
 	}
-
-	/**
-	 * Delta priority. Used in {@link module:engine/model/delta/transform~transform delta transformations}. Delta with the higher
-	 * priority will be treated as more important when resolving transformation conflicts. If deltas have same
-	 * priority, other factors will be used to determine which delta is more important.
-	 *
-	 * @private
-	 * @type {Number}
-	 */
-	static get _priority() {
-		return 0;
-	}
 }
 
 DeltaFactory.register( Delta );

src/model/delta/insertdelta.js

@@ -75,13 +75,6 @@ export default class InsertDelta extends Delta {
 	static get className() {
 		return 'engine.model.delta.InsertDelta';
 	}
-
-	/**
-	 * @inheritDoc
-	 */
-	static get _priority() {
-		return 20;
-	}
 }
 
 /**

src/model/delta/mergedelta.js

@@ -40,19 +40,6 @@ export default class MergeDelta extends Delta {
 		return this._removeOperation ? this._removeOperation.sourcePosition : null;
 	}
 
-	/**
-	 * @inheritDoc
-	 */
-	getReversed() {
-		const delta = super.getReversed();
-
-		if ( delta.operations.length > 0 ) {
-			delta.operations[ 1 ].isSticky = false;
-		}
-
-		return delta;
-	}
-
 	/**
 	 * Operation in this delta that removes the node after merge position (which will be empty at that point) or
 	 * `null` if the delta has no operations. Note, that after {@link module:engine/model/delta/transform~transform transformation}
@@ -131,7 +118,10 @@ register( 'merge', function( position ) {
 	delta.addOperation( move );
 	this.document.applyOperation( move );
 
-	const remove = new RemoveOperation( position, 1, this.document.version );
+	const graveyard = this.document.graveyard;
+	const gyPosition = new Position( graveyard, [ 0 ] );
+
+	const remove = new RemoveOperation( position, 1, gyPosition, this.document.version );
 	delta.addOperation( remove );
 	this.document.applyOperation( remove );
 

src/model/delta/movedelta.js

@@ -83,13 +83,6 @@ export default class MoveDelta extends Delta {
 	static get className() {
 		return 'engine.model.delta.MoveDelta';
 	}
-
-	/**
-	 * @inheritDoc
-	 */
-	static get _priority() {
-		return 20;
-	}
 }
 
 function addMoveOperation( batch, delta, sourcePosition, howMany, targetPosition ) {

src/model/delta/removedelta.js

@@ -28,8 +28,14 @@ export default class RemoveDelta extends MoveDelta {
 	}
 }
 
-function addRemoveOperation( batch, delta, position, howMany ) {
-	const operation = new RemoveOperation( position, howMany, batch.document.version );
+function addRemoveDelta( batch, position, howMany ) {
+	const delta = new RemoveDelta();
+	batch.addDelta( delta );
+
+	const graveyard = batch.document.graveyard;
+	const gyPosition = new Position( graveyard, [ 0 ] );
+
+	const operation = new RemoveOperation( position, howMany, gyPosition, batch.document.version );
 	delta.addOperation( operation );
 	batch.document.applyOperation( operation );
 }
@@ -42,18 +48,15 @@ function addRemoveOperation( batch, delta, position, howMany ) {
  * @param {module:engine/model/item~Item|module:engine/model/range~Range} itemOrRange Model item or range to remove.
  */
 register( 'remove', function( itemOrRange ) {
-	const delta = new RemoveDelta();
-	this.addDelta( delta );
-
 	if ( itemOrRange instanceof Range ) {
-		// The array is reversed, so the ranges are correct and do not have to be updated.
+		// The array is reversed, so the ranges to remove are in correct order and do not have to be updated.
 		const ranges = itemOrRange.getMinimalFlatRanges().reverse();
 
 		for ( const flat of ranges ) {
-			addRemoveOperation( this, delta, flat.start, flat.end.offset - flat.start.offset );
+			addRemoveDelta( this, flat.start, flat.end.offset - flat.start.offset );
 		}
 	} else {
-		addRemoveOperation( this, delta, Position.createBefore( itemOrRange ), 1 );
+		addRemoveDelta( this, Position.createBefore( itemOrRange ), 1 );
 	}
 
 	return this;

src/model/delta/splitdelta.js

@@ -39,19 +39,6 @@ export default class SplitDelta extends Delta {
 		return this._moveOperation ? this._moveOperation.sourcePosition : null;
 	}
 
-	/**
-	 * @inheritDoc
-	 */
-	getReversed() {
-		const delta = super.getReversed();
-
-		if ( delta.operations.length > 0 ) {
-			delta.operations[ 0 ].isSticky = true;
-		}
-
-		return delta;
-	}
-
 	/**
 	 * Operation in the delta that adds to model an element into which split nodes will be moved, or `null` if
 	 * there are no operations in the delta.
@@ -79,7 +66,7 @@ export default class SplitDelta extends Delta {
 	 * @type {module:engine/model/operation/moveoperation~MoveOperation|null}
 	 */
 	get _moveOperation() {
-		return this.operations[ 1 ] || null;
+		return this.operations[ 1 ] && this.operations[ 1 ] instanceof MoveOperation ? this.operations[ 1 ] : null;
 	}
 
 	/**
@@ -95,13 +82,6 @@ export default class SplitDelta extends Delta {
 	static get className() {
 		return 'engine.model.delta.SplitDelta';
 	}
-
-	/**
-	 * @inheritDoc
-	 */
-	static get _priority() {
-		return 5;
-	}
 }
 
 /**