Browse Source

Merge pull request #1497 from ckeditor/t/ot-followups

Other: Moved error logging from debug tools to OT code.
Other: Aligned `Schema#getValidRanges` results to changes in `AttributeOperation`.
Other: Unified `RemoveOperation` and `ReinsertOperation` to have just one `MoveOperation`.
Other: Simplified `LiveRange#event:change` second parameter which is now an object containing `Position` not an `Operation`.
Internal: Operational transformations refactor followups. 
Internal: Additional improvements for the new OT algorithms.
Internal: Added relations to better solve cases in undo.
Internal: Improved cloning mechanism inside `WrapOperation`.
Internal: Improved `model.Range#getTransformedByMergeOperation`.
Internal: Other improvements in OT algorithms.
Internal: Improvements in docs.

Closes #1468. Closes #1464. Closes #1467. Closes #1460. Closes #1461. Closes #1462. Closes #1463. Closes #1475. Closes #1479.

BREAKING CHANGE: `LiveRange#event:change` second parameter is now an object containing property `deletionPosition`. It can be `model.Position` instance, if the range was moved to the graveyard root. The position is equal to the position from which nodes were removed. Otherwise, it is set to `null`.

BREAKING CHANGE: `Schema#getValidRanges` will return only flat ranges now. If an attribute is allowed on some nodes and in those nodes children, multiple "nested" ranges will be returned.

BREAKING CHANGE: `Schema#getValidRanges` is a generator now.
Piotr Jasiun 7 years ago
parent
commit
5333af1c89
42 changed files with 1146 additions and 1078 deletions
  1. 0 19
      packages/ckeditor5-engine/src/dev-utils/enableenginedebug.js
  2. 2 2
      packages/ckeditor5-engine/src/model/documentselection.js
  3. 13 5
      packages/ckeditor5-engine/src/model/liverange.js
  4. 0 1
      packages/ckeditor5-engine/src/model/operation/detachoperation.js
  5. 3 3
      packages/ckeditor5-engine/src/model/operation/insertoperation.js
  6. 6 0
      packages/ckeditor5-engine/src/model/operation/moveoperation.js
  7. 0 4
      packages/ckeditor5-engine/src/model/operation/operationfactory.js
  8. 0 76
      packages/ckeditor5-engine/src/model/operation/reinsertoperation.js
  9. 0 60
      packages/ckeditor5-engine/src/model/operation/removeoperation.js
  10. 4 0
      packages/ckeditor5-engine/src/model/operation/splitoperation.js
  11. 421 144
      packages/ckeditor5-engine/src/model/operation/transform.js
  12. 8 1
      packages/ckeditor5-engine/src/model/operation/wrapoperation.js
  13. 16 4
      packages/ckeditor5-engine/src/model/position.js
  14. 27 6
      packages/ckeditor5-engine/src/model/range.js
  15. 46 19
      packages/ckeditor5-engine/src/model/schema.js
  16. 15 1
      packages/ckeditor5-engine/src/model/selection.js
  17. 2 3
      packages/ckeditor5-engine/src/model/writer.js
  18. 9 1
      packages/ckeditor5-engine/src/view/selection.js
  19. 2 37
      packages/ckeditor5-engine/tests/dev-utils/enableenginedebug.js
  20. 1 2
      packages/ckeditor5-engine/tests/model/differ.js
  21. 6 7
      packages/ckeditor5-engine/tests/model/documentselection.js
  22. 64 17
      packages/ckeditor5-engine/tests/model/liverange.js
  23. 3 3
      packages/ckeditor5-engine/tests/model/operation/insertoperation.js
  24. 12 8
      packages/ckeditor5-engine/tests/model/operation/moveoperation.js
  25. 0 157
      packages/ckeditor5-engine/tests/model/operation/reinsertoperation.js
  26. 0 191
      packages/ckeditor5-engine/tests/model/operation/removeoperation.js
  27. 135 151
      packages/ckeditor5-engine/tests/model/operation/transform.js
  28. 2 2
      packages/ckeditor5-engine/tests/model/operation/transform/attribute.js
  29. 1 3
      packages/ckeditor5-engine/tests/model/operation/transform/insert.js
  30. 36 3
      packages/ckeditor5-engine/tests/model/operation/transform/marker.js
  31. 2 6
      packages/ckeditor5-engine/tests/model/operation/transform/move.js
  32. 21 12
      packages/ckeditor5-engine/tests/model/operation/transform/remove.js
  33. 12 15
      packages/ckeditor5-engine/tests/model/operation/transform/split.js
  34. 1 4
      packages/ckeditor5-engine/tests/model/operation/transform/unwrap.js
  35. 31 13
      packages/ckeditor5-engine/tests/model/operation/transform/utils.js
  36. 133 17
      packages/ckeditor5-engine/tests/model/operation/transform/wrap.js
  37. 4 5
      packages/ckeditor5-engine/tests/model/range.js
  38. 102 70
      packages/ckeditor5-engine/tests/model/schema.js
  39. 1 1
      packages/ckeditor5-engine/tests/model/selection.js
  40. 1 1
      packages/ckeditor5-engine/tests/model/writer.js
  41. 2 2
      packages/ckeditor5-engine/tests/view/documentselection.js
  42. 2 2
      packages/ckeditor5-engine/tests/view/selection.js

+ 0 - 19
packages/ckeditor5-engine/src/dev-utils/enableenginedebug.js

@@ -25,7 +25,6 @@ import MoveOperation from '../model/operation/moveoperation';
 import NoOperation from '../model/operation/nooperation';
 import RenameOperation from '../model/operation/renameoperation';
 import RootAttributeOperation from '../model/operation/rootattributeoperation';
-import transform from '../model/operation/transform';
 import Model from '../model/model';
 import ModelDocument from '../model/document';
 import ModelDocumentFragment from '../model/documentfragment';
@@ -349,24 +348,6 @@ function enableLoggingTools() {
 			`"${ this.key }": ${ JSON.stringify( this.oldValue ) } -> ${ JSON.stringify( this.newValue ) }, ${ this.root.rootName }`;
 	} );
 
-	const _transformTransform = transform.transform;
-
-	sandbox.mock( transform, 'transform', function( a, b, context ) {
-		let results;
-
-		try {
-			results = _transformTransform( a, b, context );
-		} catch ( e ) {
-			logger.error( 'Error during operation transformation!' );
-			logger.error( a.toString() + ( context.isStrong ? ' (important)' : '' ) );
-			logger.error( b.toString() + ( context.isStrong ? '' : ' (important)' ) );
-
-			throw e;
-		}
-
-		return results;
-	} );
-
 	sandbox.mock( ViewText.prototype, 'toString', function() {
 		return `#${ this.data }`;
 	} );

+ 2 - 2
packages/ckeditor5-engine/src/model/documentselection.js

@@ -750,14 +750,14 @@ class LiveSelection extends Selection {
 
 		const liveRange = LiveRange.createFromRange( range );
 
-		liveRange.on( 'change:range', ( evt, oldRange, operation ) => {
+		liveRange.on( 'change:range', ( evt, oldRange, data ) => {
 			this._hasChangedRange = true;
 
 			// If `LiveRange` is in whole moved to the graveyard, save necessary data. It will be fixed on `Model#applyOperation` event.
 			if ( liveRange.root == this._document.graveyard ) {
 				this._fixGraveyardRangesData.push( {
 					liveRange,
-					sourcePosition: operation.sourcePosition
+					sourcePosition: data.deletionPosition
 				} );
 			}
 		} );

+ 13 - 5
packages/ckeditor5-engine/src/model/liverange.js

@@ -86,9 +86,7 @@ export default class LiveRange extends Range {
 	 * @param {Object} data Object with additional information about the change. Those parameters are passed from
 	 * {@link module:engine/model/document~Document#event:change document change event}.
 	 * @param {String} data.type Change type.
-	 * @param {module:engine/model/batch~Batch} data.batch Batch which changed the live range.
-	 * @param {module:engine/model/range~Range} data.range Range containing the result of applied change.
-	 * @param {module:engine/model/position~Position} data.sourcePosition Source position for move, remove and reinsert change types.
+	 * @param {module:engine/model/position~Position|null} deletionPosition Source position for move, remove and reinsert change types.
 	 */
 
 	/**
@@ -145,18 +143,28 @@ function transform( operation ) {
 	const result = Range.createFromRanges( ranges );
 	const boundariesChanged = !result.isEqual( this );
 	const contentChanged = doesOperationChangeRangeContent( this, operation );
+	let deletionPosition = null;
 
 	if ( boundariesChanged ) {
+		if ( result.root.rootName == '$graveyard' ) {
+			if ( operation.type == 'remove' ) {
+				deletionPosition = operation.sourcePosition;
+			} else {
+				// Merge operation.
+				deletionPosition = operation.deletionPosition;
+			}
+		}
+
 		// If range boundaries have changed, fire `change:range` event.
 		const oldRange = Range.createFromRange( this );
 
 		this.start = result.start;
 		this.end = result.end;
 
-		this.fire( 'change:range', oldRange, operation );
+		this.fire( 'change:range', oldRange, { deletionPosition } );
 	} else if ( contentChanged ) {
 		// If range boundaries have not changed, but there was change inside the range, fire `change:content` event.
-		this.fire( 'change:content', Range.createFromRange( this ), operation );
+		this.fire( 'change:content', Range.createFromRange( this ), { deletionPosition } );
 	}
 }
 

+ 0 - 1
packages/ckeditor5-engine/src/model/operation/detachoperation.js

@@ -60,7 +60,6 @@ export default class DetachOperation extends Operation {
 		if ( this.sourcePosition.root.document ) {
 			/**
 			 * Cannot detach document node.
-			 * Use {@link module:engine/model/operation/removeoperation~RemoveOperation remove operation} instead.
 			 *
 			 * @error detach-operation-on-document-node
 			 */

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

@@ -10,7 +10,7 @@
 import Operation from './operation';
 import Position from '../position';
 import NodeList from '../nodelist';
-import RemoveOperation from './removeoperation';
+import MoveOperation from './moveoperation';
 import { _insert, _normalizeNodes } from './utils';
 import Text from '../text';
 import Element from '../element';
@@ -87,13 +87,13 @@ export default class InsertOperation extends Operation {
 	/**
 	 * See {@link module:engine/model/operation/operation~Operation#getReversed `Operation#getReversed()`}.
 	 *
-	 * @returns {module:engine/model/operation/removeoperation~RemoveOperation}
+	 * @returns {module:engine/model/operation/moveoperation~MoveOperation}
 	 */
 	getReversed() {
 		const graveyard = this.position.root.document.graveyard;
 		const gyPosition = new Position( graveyard, [ 0 ] );
 
-		return new RemoveOperation( this.position, this.nodes.maxOffset, gyPosition, this.baseVersion + 1 );
+		return new MoveOperation( this.position, this.nodes.maxOffset, gyPosition, this.baseVersion + 1 );
 	}
 
 	/**

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

@@ -64,6 +64,12 @@ export default class MoveOperation extends Operation {
 	 * @inheritDoc
 	 */
 	get type() {
+		if ( this.targetPosition.root.rootName == '$graveyard' ) {
+			return 'remove';
+		} else if ( this.sourcePosition.root.rootName == '$graveyard' ) {
+			return 'reinsert';
+		}
+
 		return 'move';
 	}
 

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

@@ -13,8 +13,6 @@ import MarkerOperation from '../operation/markeroperation';
 import MoveOperation from '../operation/moveoperation';
 import NoOperation from '../operation/nooperation';
 import Operation from '../operation/operation';
-import ReinsertOperation from '../operation/reinsertoperation';
-import RemoveOperation from '../operation/removeoperation';
 import RenameOperation from '../operation/renameoperation';
 import RootAttributeOperation from '../operation/rootattributeoperation';
 import SplitOperation from '../operation/splitoperation';
@@ -29,8 +27,6 @@ operations[ MarkerOperation.className ] = MarkerOperation;
 operations[ MoveOperation.className ] = MoveOperation;
 operations[ NoOperation.className ] = NoOperation;
 operations[ Operation.className ] = Operation;
-operations[ ReinsertOperation.className ] = ReinsertOperation;
-operations[ RemoveOperation.className ] = RemoveOperation;
 operations[ RenameOperation.className ] = RenameOperation;
 operations[ RootAttributeOperation.className ] = RootAttributeOperation;
 operations[ SplitOperation.className ] = SplitOperation;

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

@@ -1,76 +0,0 @@
-/**
- * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved.
- * For licensing, see LICENSE.md.
- */
-
-/**
- * @module engine/model/operation/reinsertoperation
- */
-
-import MoveOperation from './moveoperation';
-import RemoveOperation from './removeoperation';
-import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
-
-/**
- * Operation to reinsert previously removed nodes back to the non-graveyard root. This operation acts like
- * {@link module:engine/model/operation/moveoperation~MoveOperation} but it returns
- * {@link module:engine/model/operation/removeoperation~RemoveOperation} when reversed
- * and fires different change event.
- */
-export default class ReinsertOperation extends MoveOperation {
-	/**
-	 * Position where nodes will be re-inserted.
-	 *
-	 * @type {module:engine/model/position~Position}
-	 */
-	get position() {
-		return this.targetPosition;
-	}
-
-	/**
-	 * @param {module:engine/model/position~Position} pos
-	 */
-	set position( pos ) {
-		this.targetPosition = pos;
-	}
-
-	/**
-	 * @inheritDoc
-	 */
-	get type() {
-		return 'reinsert';
-	}
-
-	/**
-	 * See {@link module:engine/model/operation/operation~Operation#getReversed `Operation#getReversed()`}.
-	 *
-	 * @returns {module:engine/model/operation/removeoperation~RemoveOperation}
-	 */
-	getReversed() {
-		const newTargetPosition = this.sourcePosition._getTransformedByInsertion( this.targetPosition, this.howMany );
-
-		return new RemoveOperation( this.getMovedRangeStart(), this.howMany, newTargetPosition, this.baseVersion + 1 );
-	}
-
-	/**
-	 * @inheritDoc
-	 */
-	_validate() {
-		super._validate();
-
-		if ( !this.sourcePosition.root.document ) {
-			throw new CKEditorError( 'reinsert-operation-on-detached-item: Cannot reinsert detached item.' );
-		}
-
-		if ( !this.targetPosition.root.document ) {
-			throw new CKEditorError( 'reinsert-operation-to-detached-parent: Cannot reinsert item to detached parent.' );
-		}
-	}
-
-	/**
-	 * @inheritDoc
-	 */
-	static get className() {
-		return 'engine.model.operation.ReinsertOperation';
-	}
-}

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

@@ -1,60 +0,0 @@
-/**
- * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved.
- * For licensing, see LICENSE.md.
- */
-
-/**
- * @module engine/model/operation/removeoperation
- */
-
-import MoveOperation from './moveoperation';
-import ReinsertOperation from './reinsertoperation';
-import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
-
-/**
- * Operation to remove a range of nodes.
- */
-export default class RemoveOperation extends MoveOperation {
-	/**
-	 * @inheritDoc
-	 */
-	get type() {
-		return 'remove';
-	}
-
-	/**
-	 * See {@link module:engine/model/operation/operation~Operation#getReversed `Operation#getReversed()`}.
-	 *
-	 * @returns {module:engine/model/operation/reinsertoperation~ReinsertOperation|module:engine/model/operation/nooperation~NoOperation}
-	 */
-	getReversed() {
-		const newTargetPosition = this.sourcePosition._getTransformedByInsertion( this.targetPosition, this.howMany );
-
-		return new ReinsertOperation( this.getMovedRangeStart(), this.howMany, newTargetPosition, this.baseVersion + 1 );
-	}
-
-	/**
-	 * @inheritDoc
-	 */
-	_validate() {
-		super._validate();
-
-		if ( !this.sourcePosition.root.document ) {
-			/**
-			 * Item that is going to be removed needs to be a {@link module:engine/model/document~Document document} child.
-			 * To remove Item from detached document fragment use
-			 * {@link module:engine/model/operation/detachoperation~DetachOperation DetachOperation}.
-			 *
-			 * @error remove-operation-on-detached-item
-			 */
-			throw new CKEditorError( 'remove-operation-on-detached-item: Cannot remove detached item.' );
-		}
-	}
-
-	/**
-	 * @inheritDoc
-	 */
-	static get className() {
-		return 'engine.model.operation.RemoveOperation';
-	}
-}

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

@@ -43,6 +43,10 @@ export default class SplitOperation extends Operation {
 		this.position.stickiness = 'toNext';
 
 		this.graveyardPosition = graveyardPosition ? Position.createFromPosition( graveyardPosition ) : null;
+
+		if ( this.graveyardPosition ) {
+			this.graveyardPosition.stickiness = 'toNext';
+		}
 	}
 
 	/**

+ 421 - 144
packages/ckeditor5-engine/src/model/operation/transform.js

@@ -3,8 +3,6 @@ import AttributeOperation from './attributeoperation';
 import RenameOperation from './renameoperation';
 import MarkerOperation from './markeroperation';
 import MoveOperation from './moveoperation';
-import RemoveOperation from './removeoperation';
-import ReinsertOperation from './reinsertoperation';
 import RootAttributeOperation from './rootattributeoperation';
 import MergeOperation from './mergeoperation';
 import SplitOperation from './splitoperation';
@@ -13,7 +11,9 @@ import UnwrapOperation from './unwrapoperation';
 import NoOperation from './nooperation';
 import Range from '../range';
 import Position from '../position';
+
 import compareArrays from '@ckeditor/ckeditor5-utils/src/comparearrays';
+import log from '@ckeditor/ckeditor5-utils/src/log';
 
 const transformations = new Map();
 
@@ -29,32 +29,10 @@ function setTransformation( OperationA, OperationB, transformationFunction ) {
 }
 
 function getTransformation( a, b ) {
-	const OperationA = a.constructor;
-	const OperationB = b.constructor;
-
-	const aGroups = new Set();
-	const aGroup = transformations.get( OperationA );
-
-	if ( aGroup ) {
-		aGroups.add( aGroup );
-	}
-
-	for ( const Operation of transformations.keys() ) {
-		if ( a instanceof Operation ) {
-			aGroups.add( transformations.get( Operation ) );
-		}
-	}
+	const aGroup = transformations.get( a.constructor );
 
-	for ( const group of aGroups ) {
-		if ( group.has( OperationB ) ) {
-			return group.get( OperationB );
-		}
-
-		for ( const Operation of group.keys() ) {
-			if ( b instanceof Operation ) {
-				return group.get( Operation );
-			}
-		}
+	if ( aGroup && aGroup.has( b.constructor ) ) {
+		return aGroup.get( b.constructor );
 	}
 
 	return noUpdateTransformation;
@@ -72,13 +50,26 @@ function updateBaseVersions( operations, baseVersion ) {
 	return operations;
 }
 
-function transform( a, b, context = { aIsStrong: false } ) {
+export function transform( a, b, context = {} ) {
 	const transformationFunction = getTransformation( a, b );
 
-	return transformationFunction( a.clone(), b, context );
+	try {
+		return transformationFunction( a.clone(), b, context );
+	} catch ( e ) {
+		log.error( 'Error during operation transformation!' );
+		log.error( 'Transformed operation', a );
+		log.error( 'Operation transformed by', b );
+		log.error( 'context.aIsStrong', context.aIsStrong );
+		log.error( 'context.aWasUndone', context.aWasUndone );
+		log.error( 'context.bWasUndone', context.bWasUndone );
+		log.error( 'context.abRelation', context.abRelation );
+		log.error( 'context.baRelation', context.baRelation );
+
+		throw e;
+	}
 }
 
-function transformSets( operationsA, operationsB, options ) {
+export function transformSets( operationsA, operationsB, options ) {
 	operationsA = operationsA.slice();
 	operationsB = operationsB.slice();
 
@@ -113,17 +104,34 @@ function transformSets( operationsA, operationsB, options ) {
 					const opA = opsA[ k ];
 					const opB = opsB[ l ];
 
-					context.aIsStrong = true;
-					const newOpA = transform( opA, opB, context );
-
-					context.aIsStrong = false;
-					const newOpB = transform( opB, opA, context );
+					if ( options.useContext ) {
+						updateRelations( context, opA, opB );
+					}
 
-					delete context.aIsStrong;
+					const contextAB = {
+						aIsStrong: true,
+						aWasUndone: context.wasUndone( opA ),
+						bWasUndone: context.wasUndone( opB ),
+						abRelation: context.getRelation( opA, opB ),
+						baRelation: context.getRelation( opB, opA )
+					};
+
+					const contextBA = {
+						aIsStrong: false,
+						aWasUndone: context.wasUndone( opB ),
+						bWasUndone: context.wasUndone( opA ),
+						abRelation: context.getRelation( opB, opA ),
+						baRelation: context.getRelation( opA, opB )
+					};
+
+					const newOpA = transform( opA, opB, contextAB );
+					const newOpB = transform( opB, opA, contextBA );
 
 					if ( options.useContext ) {
 						updateOriginalOperation( context, opA, newOpA );
 						updateOriginalOperation( context, opB, newOpB );
+
+						updateRelations( context, opA, opB );
 					}
 
 					opsA.splice( k, 1, ...newOpA );
@@ -155,11 +163,6 @@ function transformSets( operationsA, operationsB, options ) {
 	return { operationsA, operationsB };
 }
 
-export default {
-	transform,
-	transformSets
-};
-
 function padWithNoOps( operations, howMany ) {
 	for ( let i = 0; i < howMany; i++ ) {
 		operations.push( new NoOperation( 0 ) );
@@ -176,6 +179,7 @@ function initializeContext( opsA, opsB, options ) {
 	}
 
 	context.document = options.document;
+	context.relations = new Map();
 
 	context.wasUndone = function( op ) {
 		if ( !options.useContext ) {
@@ -187,9 +191,152 @@ function initializeContext( opsA, opsB, options ) {
 		return this.document.history.isUndoneOperation( originalOp );
 	};
 
+	context.getRelation = function( opA, opB ) {
+		if ( !options.useContext ) {
+			return null;
+		}
+
+		const origB = this.originalOperations.get( opB );
+		const undoneB = this.document.history.getUndoneOperation( origB );
+
+		if ( !undoneB ) {
+			return null;
+		}
+
+		const origA = this.originalOperations.get( opA );
+		const relationsA = this.relations.get( origA );
+
+		if ( relationsA ) {
+			return relationsA.get( undoneB ) || null;
+		}
+
+		return null;
+	};
+
 	return context;
 }
 
+function updateRelations( context, opA, opB ) {
+	switch ( opA.constructor ) {
+		case MoveOperation: {
+			switch ( opB.constructor ) {
+				case MergeOperation: {
+					if ( opA.targetPosition.isEqual( opB.sourcePosition ) || opB.movedRange.containsPosition( opA.targetPosition ) ) {
+						setRelation( context, opA, opB, 'insertAtSource' );
+						setRelation( context, opB, opA, 'splitBefore' );
+					}
+
+					break;
+				}
+
+				case MoveOperation: {
+					if ( opA.targetPosition.isEqual( opB.sourcePosition ) || opA.targetPosition.isBefore( opB.sourcePosition ) ) {
+						setRelation( context, opA, opB, 'insertBefore' );
+						setRelation( context, opB, opA, 'insertAfter' );
+					} else {
+						setRelation( context, opA, opB, 'insertAfter' );
+						setRelation( context, opB, opA, 'insertBefore' );
+					}
+
+					break;
+				}
+
+				case UnwrapOperation: {
+					const isInside = opA.targetPosition.hasSameParentAs( opB.targetPosition );
+
+					if ( isInside ) {
+						setRelation( context, opA, opB, 'insertInside' );
+					}
+
+					break;
+				}
+			}
+
+			break;
+		}
+
+		case SplitOperation: {
+			switch ( opB.constructor ) {
+				case MergeOperation: {
+					if ( opA.position.isBefore( opB.sourcePosition ) ) {
+						setRelation( context, opA, opB, 'splitBefore' );
+						setRelation( context, opB, opA, 'splitAfter' );
+					}
+
+					break;
+				}
+
+				case MoveOperation: {
+					if ( opA.position.isEqual( opB.sourcePosition ) || opA.position.isBefore( opB.sourcePosition ) ) {
+						setRelation( context, opA, opB, 'splitBefore' );
+						setRelation( context, opB, opA, 'insertAtSource' );
+					}
+
+					break;
+				}
+
+				case UnwrapOperation: {
+					const isInside = opA.position.hasSameParentAs( opB.position );
+
+					if ( isInside ) {
+						setRelation( context, opA, opB, 'splitInside' );
+					}
+
+					break;
+				}
+			}
+
+			break;
+		}
+
+		case InsertOperation: {
+			switch ( opB.constructor ) {
+				case MergeOperation: {
+					if ( opA.position.isEqual( opB.sourcePosition ) || opB.movedRange.containsPosition( opA.position ) ) {
+						setRelation( context, opA, opB, 'insertAtSource' );
+					}
+
+					break;
+				}
+
+				case MoveOperation: {
+					if ( opA.position.isEqual( opB.sourcePosition ) || opA.position.isBefore( opB.sourcePosition ) ) {
+						setRelation( context, opA, opB, 'insertBefore' );
+					}
+
+					break;
+				}
+
+				case UnwrapOperation: {
+					const isInside = opA.position.hasSameParentAs( opB.position );
+
+					if ( isInside ) {
+						setRelation( context, opA, opB, 'insertInside' );
+					}
+
+					break;
+				}
+			}
+
+			break;
+		}
+	}
+}
+
+function setRelation( context, opA, opB, relation ) {
+	const origA = context.originalOperations.get( opA );
+	const origB = context.originalOperations.get( opB );
+
+	let relationsA = context.relations.get( origA );
+
+	if ( !relationsA ) {
+		relationsA = new Map();
+		context.relations.set( origA, relationsA );
+	}
+
+	relationsA.set( origB, relation );
+}
+
 function updateOriginalOperation( context, oldOp, newOps ) {
 	const originalOp = context.originalOperations.get( oldOp );
 
@@ -269,7 +416,7 @@ setTransformation( AttributeOperation, InsertOperation, ( a, b ) => {
 		// <p>Fo[zb]ar<p>
 		//
 		// New text with `highlight="red"` is typed:
-		// <p>Fo[z<$text higlight="red">x</$text>a]r</p>
+		// <p>Fo[z<$text highlight="red">x</$text>a]r</p>
 		//
 		// In this case three operations are needed: `oldValue=null, newValue="yellow"` for `z`, `oldValue="red",
 		// newValue="yellow"` for `x` and `oldValue=null, newValue="yellow"` for `a`. It could even happen that
@@ -345,6 +492,8 @@ setTransformation( AttributeOperation, MergeOperation, ( a, b ) => {
 
 	// Case 1:	Attribute change on the merged element. In this case, the merged element was moved to graveyard.
 	//			An additional attribute operation that will change the (re)moved element needs to be generated.
+	//			Do it only, if there is more than one element in attribute range. If there is only one element,
+	//			it will be handled by the default algorithm.
 	//
 	if ( a.range.start.hasSameParentAs( b.deletionPosition ) ) {
 		if ( a.range.containsPosition( b.deletionPosition ) || a.range.start.isEqual( b.deletionPosition ) ) {
@@ -352,7 +501,12 @@ setTransformation( AttributeOperation, MergeOperation, ( a, b ) => {
 		}
 	}
 
-	ranges.push( a.range._getTransformedByMergeOperation( b ) );
+	const range = a.range._getTransformedByMergeOperation( b );
+
+	// Do not add empty (collapsed) ranges to the result. `range` may be collapsed if it contained only the merged element.
+	if ( !range.isCollapsed ) {
+		ranges.push( range );
+	}
 
 	// Create `AttributeOperation`s out of the ranges.
 	return ranges.map( range => {
@@ -361,20 +515,10 @@ setTransformation( AttributeOperation, MergeOperation, ( a, b ) => {
 } );
 
 setTransformation( AttributeOperation, MoveOperation, ( a, b ) => {
-	const movedRange = Range.createFromPositionAndShift( b.sourcePosition, b.howMany );
 	const ranges = breakRangeByMoveOperation( a.range, b, true );
 
 	// Create `AttributeOperation`s out of the ranges.
-	return ranges.map( range => {
-		if ( movedRange.containsRange( range, true ) ) {
-			range = range._getTransformedByMoveOperation( b, false )[ 0 ];
-		} else {
-			range = range._getTransformedByDeletion( b.sourcePosition, b.howMany );
-			range = range._getTransformedByInsertion( b.targetPosition, b.howMany, false )[ 0 ];
-		}
-
-		return new AttributeOperation( range, a.key, a.oldValue, a.newValue, a.baseVersion );
-	} );
+	return ranges.map( range => new AttributeOperation( range, a.key, a.oldValue, a.newValue, a.baseVersion ) );
 } );
 
 function breakRangeByMoveOperation( range, moveOp, includeCommon ) {
@@ -382,32 +526,48 @@ function breakRangeByMoveOperation( range, moveOp, includeCommon ) {
 
 	const movedRange = Range.createFromPositionAndShift( moveOp.sourcePosition, moveOp.howMany );
 
-	if ( range.start.hasSameParentAs( moveOp.sourcePosition ) ) {
+	if ( movedRange.containsRange( range, true ) ) {
+		ranges = [ ...range._getTransformedByMoveOperation( moveOp ) ];
+	} else if ( range.start.hasSameParentAs( moveOp.sourcePosition ) ) {
 		ranges = range.getDifference( movedRange );
 
+		_flatMoveTransform( ranges, moveOp );
+
 		if ( includeCommon ) {
 			const common = range.getIntersection( movedRange );
 
 			if ( common ) {
-				ranges.push( common );
+				ranges.push( ...common._getTransformedByMoveOperation( moveOp ) );
 			}
 		}
 	} else {
 		ranges = [ range ];
+
+		_flatMoveTransform( ranges, moveOp );
 	}
 
+	return ranges;
+}
+
+function _flatMoveTransform( ranges, moveOp ) {
+	const targetPosition = moveOp.getMovedRangeStart();
+
 	for ( let i = 0; i < ranges.length; i++ ) {
-		const range = ranges[ i ];
+		let range = ranges[ i ];
 
-		if ( range.start.hasSameParentAs( moveOp.targetPosition ) && range.containsPosition( moveOp.targetPosition ) ) {
-			ranges.splice( i, 1,
-				new Range( range.start, moveOp.targetPosition ),
-				new Range( moveOp.targetPosition, range.end )
-			);
+		if ( range.start.hasSameParentAs( moveOp.sourcePosition ) ) {
+			range = range._getTransformedByDeletion( moveOp.sourcePosition, moveOp.howMany );
 		}
-	}
 
-	return ranges;
+		if ( range.start.hasSameParentAs( targetPosition ) ) {
+			const result = range._getTransformedByInsertion( targetPosition, moveOp.howMany, true );
+
+			ranges.splice( i, 1, ...result );
+			i = i - 1 + result.length;
+		} else {
+			ranges[ i ] = range;
+		}
+	}
 }
 
 setTransformation( AttributeOperation, SplitOperation, ( a, b ) => {
@@ -443,10 +603,13 @@ setTransformation( AttributeOperation, SplitOperation, ( a, b ) => {
 	if ( a.range.start.hasSameParentAs( b.position ) && a.range.containsPosition( b.position ) ) {
 		const secondPart = a.clone();
 
-		secondPart.range.start = Position.createFromPosition( b.moveTargetPosition );
-		secondPart.range.end = a.range.end._getCombined( b.position, b.moveTargetPosition );
+		secondPart.range = new Range(
+			Position.createFromPosition( b.moveTargetPosition ),
+			a.range.end._getCombined( b.position, b.moveTargetPosition )
+		);
 
 		a.range.end = Position.createFromPosition( b.position );
+		a.range.end.stickiness = 'toPrevious';
 
 		return [ a, secondPart ];
 	}
@@ -541,13 +704,23 @@ setTransformation( InsertOperation, InsertOperation, ( a, b, context ) => {
 	return [ a ];
 } );
 
-setTransformation( InsertOperation, MoveOperation, ( a, b ) => {
+setTransformation( InsertOperation, MoveOperation, ( a, b, context ) => {
+	if ( a.position.isEqual( b.targetPosition ) && context.abRelation == 'insertBefore' ) {
+		return [ a ];
+	}
+
 	a.position = a.position._getTransformedByMoveOperation( b );
 
 	return [ a ];
 } );
 
-setTransformation( InsertOperation, SplitOperation, ( a, b ) => {
+setTransformation( InsertOperation, SplitOperation, ( a, b, context ) => {
+	if ( a.position.isEqual( b.position ) && context.abRelation == 'insertAtSource' ) {
+		a.position = b.moveTargetPosition;
+
+		return [ a ];
+	}
+
 	a.position = a.position._getTransformedBySplitOperation( b );
 
 	return [ a ];
@@ -559,7 +732,13 @@ setTransformation( InsertOperation, MergeOperation, ( a, b ) => {
 	return [ a ];
 } );
 
-setTransformation( InsertOperation, WrapOperation, ( a, b ) => {
+setTransformation( InsertOperation, WrapOperation, ( a, b, context ) => {
+	if ( a.position.isEqual( b.position ) && context.abRelation == 'insertInside' ) {
+		a.position = b.targetPosition;
+
+		return [ a ];
+	}
+
 	a.position = a.position._getTransformedByWrapOperation( b );
 
 	return [ a ];
@@ -707,7 +886,7 @@ setTransformation( MergeOperation, MoveOperation, ( a, b, context ) => {
 	//
 	const removedRange = Range.createFromPositionAndShift( b.sourcePosition, b.howMany );
 
-	if ( b instanceof RemoveOperation && !context.wasUndone( b ) ) {
+	if ( b.type == 'remove' && !context.bWasUndone ) {
 		if ( a.deletionPosition.hasSameParentAs( b.sourcePosition ) && removedRange.containsPosition( a.sourcePosition ) ) {
 			return getNoOp();
 		}
@@ -716,7 +895,7 @@ setTransformation( MergeOperation, MoveOperation, ( a, b, context ) => {
 	a.sourcePosition = a.sourcePosition._getTransformedByMoveOperation( b );
 	a.targetPosition = a.targetPosition._getTransformedByMoveOperation( b );
 
-	if ( !a.graveyardPosition.isEqual( b.targetPosition ) || !context.aIsStrong ) {
+	if ( !a.graveyardPosition.isEqual( b.targetPosition ) ) {
 		a.graveyardPosition = a.graveyardPosition._getTransformedByMoveOperation( b );
 	}
 
@@ -724,8 +903,6 @@ setTransformation( MergeOperation, MoveOperation, ( a, b, context ) => {
 } );
 
 setTransformation( MergeOperation, SplitOperation, ( a, b ) => {
-	a.sourcePosition = a.sourcePosition._getTransformedBySplitOperation( b );
-
 	if ( b.graveyardPosition ) {
 		a.graveyardPosition = a.graveyardPosition._getTransformedByDeletion( b.graveyardPosition, 1 );
 	}
@@ -744,13 +921,18 @@ setTransformation( MergeOperation, SplitOperation, ( a, b ) => {
 	//			This means that `targetPosition` needs to be transformed. This is the default case though.
 	//			For example, if the split would be after `F`, `targetPosition` should also be transformed.
 	//
-	//			There is one exception though - when the split operation is a result of undo. In those cases, it is needed
-	//			to keep `targetPosition` intact, so the nodes are returned to the correct element.
+	//			There is an exception though. It is when merge operation targets into inside of an element.
+	//			Such merge operation can be a result of merge x merge transformation, when merges are identical.
+	//			Such merge operation's source position is in graveyard and we will use that to recognize it
+	//			(although a more precise method would be more correct).
 	//
-	if ( a.targetPosition.isEqual( b.position ) && b.graveyardPosition ) {
+	if ( a.targetPosition.isEqual( b.position ) && a.sourcePosition.root.rootName == '$graveyard' ) {
+		a.sourcePosition = a.sourcePosition._getTransformedBySplitOperation( b );
+
 		return [ a ];
 	}
 
+	a.sourcePosition = a.sourcePosition._getTransformedBySplitOperation( b );
 	a.targetPosition = a.targetPosition._getTransformedBySplitOperation( b );
 
 	return [ a ];
@@ -774,6 +956,23 @@ setTransformation( MergeOperation, WrapOperation, ( a, b ) => {
 		return [ a ];
 	}
 
+	// Case 2:	Merged element is wrapped and this is the last (only) element in the wrap.
+	//			Because of how this is resolved in `WrapOperation` x `MergeOperation`, we need to apply special handling here.
+	//			If the last element from wrapped range is "removed" from it, the wrap is effectively on empty range.
+	//			In that case, the wrapper element is moved to graveyard. This happens in `WrapOperation` x
+	//			`MergeOperation` and we need to mirror it here.
+	//
+	if ( b.position.isEqual( a.deletionPosition ) && b.howMany == 1 ) {
+		// We need to change `MergeOperation#graveyardPosition` so the merged node is moved into the wrapper element.
+		// Since `UnwrapOperation` created from reverse has graveyard position at [ 0 ], we can safely set the path here to [ 0, 0 ].
+		a.graveyardPosition = new Position( a.graveyardPosition.root, [ 0, 0 ] );
+
+		return [
+			b.getReversed(),
+			a
+		];
+	}
+
 	a.sourcePosition = a.sourcePosition._getTransformedByWrapOperation( b );
 	a.targetPosition = a.targetPosition._getTransformedByWrapOperation( b );
 
@@ -814,14 +1013,14 @@ setTransformation( MergeOperation, UnwrapOperation, ( a, b, context ) => {
 
 // -----------------------
 
-setTransformation( MoveOperation, InsertOperation, ( a, b ) => {
+setTransformation( MoveOperation, InsertOperation, ( a, b, context ) => {
 	const moveRange = Range.createFromPositionAndShift( a.sourcePosition, a.howMany );
 	const transformed = moveRange._getTransformedByInsertOperation( b, false )[ 0 ];
 
 	a.sourcePosition = transformed.start;
 	a.howMany = transformed.end.offset - transformed.start.offset;
 
-	if ( !a.targetPosition.isEqual( b.position ) ) {
+	if ( !a.targetPosition.isEqual( b.position ) || context.abRelation == 'insertBefore' ) {
 		a.targetPosition = a.targetPosition._getTransformedByInsertOperation( b );
 	}
 
@@ -840,12 +1039,27 @@ setTransformation( MoveOperation, MoveOperation, ( a, b, context ) => {
 	// this algorithm and we do not want to override original `context.aIsStrong` that will be used in later transformations.
 	let aIsStrong = context.aIsStrong;
 
+	if ( context.abRelation == 'insertBefore' ) {
+		aIsStrong = true;
+	} else if ( context.abRelation == 'insertAfter' ) {
+		aIsStrong = false;
+	}
+
 	// `a.targetPosition` could be affected by the `b` operation. We will transform it.
-	const newTargetPosition = a.targetPosition._getTransformedByMove(
-		b.sourcePosition,
-		b.targetPosition,
-		b.howMany
-	);
+	let newTargetPosition;
+
+	if ( a.targetPosition.isEqual( b.targetPosition ) && aIsStrong ) {
+		newTargetPosition = a.targetPosition._getTransformedByDeletion(
+			b.sourcePosition,
+			b.howMany
+		);
+	} else {
+		newTargetPosition = a.targetPosition._getTransformedByMove(
+			b.sourcePosition,
+			b.targetPosition,
+			b.howMany
+		);
+	}
 
 	//
 	// Special case #1 + mirror.
@@ -926,9 +1140,9 @@ setTransformation( MoveOperation, MoveOperation, ( a, b, context ) => {
 	//
 	// 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 ) ) {
+	if ( a.type == 'remove' && b.type != 'remove' ) {
 		aIsStrong = true;
-	} else if ( !( a instanceof RemoveOperation ) && b instanceof RemoveOperation ) {
+	} else if ( a.type != 'remove' && b.type == 'remove' ) {
 		aIsStrong = false;
 	}
 
@@ -993,7 +1207,7 @@ setTransformation( MoveOperation, MoveOperation, ( a, b, context ) => {
 	return makeMoveOperationsFromRanges( ranges, newTargetPosition );
 } );
 
-setTransformation( MoveOperation, SplitOperation, ( a, b ) => {
+setTransformation( MoveOperation, SplitOperation, ( a, b, context ) => {
 	const newTargetPosition = a.targetPosition._getTransformedBySplitOperation( b );
 
 	// Case 1:	Last element in the moved range got split.
@@ -1028,8 +1242,8 @@ setTransformation( MoveOperation, SplitOperation, ( a, b ) => {
 		rightRange = rightRange._getTransformedBySplitOperation( b );
 
 		const ranges = [
-			rightRange,
-			Range.createFromPositionAndShift( moveRange.start, b.position.offset - moveRange.start.offset )
+			new Range( moveRange.start, b.position ),
+			rightRange
 		];
 
 		return makeMoveOperationsFromRanges( ranges, newTargetPosition );
@@ -1043,6 +1257,10 @@ setTransformation( MoveOperation, SplitOperation, ( a, b ) => {
 	a.howMany = transformed.end.offset - transformed.start.offset;
 	a.targetPosition = newTargetPosition;
 
+	if ( a.targetPosition.isEqual( b.position ) && context.abRelation == 'insertAtSource' ) {
+		a.targetPosition = b.targetPosition;
+	}
+
 	return [ a ];
 } );
 
@@ -1050,14 +1268,14 @@ setTransformation( MoveOperation, MergeOperation, ( a, b, context ) => {
 	const movedRange = Range.createFromPositionAndShift( a.sourcePosition, a.howMany );
 
 	if ( b.deletionPosition.hasSameParentAs( a.sourcePosition ) && movedRange.containsPosition( b.sourcePosition ) ) {
-		if ( a instanceof RemoveOperation ) {
+		if ( a.type == 'remove' ) {
 			// Case 1:	The element to remove got merged.
 			//			Merge operation does support merging elements which are not siblings. So it would not be a problem
 			//			from technical point of view. However, if the element was removed, the intention of the user
 			//			deleting it was to have it all deleted. From user experience point of view, moving back the
 			//			removed nodes might be unexpected. This means that in this scenario we will reverse merging and remove the element.
 			//
-			if ( !context.wasUndone( a ) ) {
+			if ( !context.aWasUndone ) {
 				return [ b.getReversed(), a ];
 			}
 		} else {
@@ -1081,7 +1299,7 @@ setTransformation( MoveOperation, MergeOperation, ( a, b, context ) => {
 	return [ a ];
 } );
 
-setTransformation( MoveOperation, WrapOperation, ( a, b ) => {
+setTransformation( MoveOperation, WrapOperation, ( a, b, context ) => {
 	const moveRange = Range.createFromPositionAndShift( a.sourcePosition, a.howMany );
 	const newTargetPosition = a.targetPosition._getTransformedByWrapOperation( b );
 
@@ -1133,6 +1351,10 @@ setTransformation( MoveOperation, WrapOperation, ( a, b ) => {
 	a.howMany = transformed.end.offset - transformed.start.offset;
 	a.targetPosition = newTargetPosition;
 
+	if ( a.targetPosition.isEqual( b.position ) && context.abRelation == 'insertInside' ) {
+		a.targetPosition = b.targetPosition;
+	}
+
 	return [ a ];
 } );
 
@@ -1156,6 +1378,13 @@ setTransformation( RenameOperation, InsertOperation, ( a, b ) => {
 } );
 
 setTransformation( RenameOperation, MergeOperation, ( a, b ) => {
+	if ( a.position.isEqual( b.deletionPosition ) ) {
+		a.position = Position.createFromPosition( b.graveyardPosition );
+		a.position.stickiness = 'toNext';
+
+		return [ a ];
+	}
+
 	a.position = a.position._getTransformedByMergeOperation( b );
 
 	return [ a ];
@@ -1235,7 +1464,11 @@ setTransformation( RootAttributeOperation, RootAttributeOperation, ( a, b, conte
 
 // -----------------------
 
-setTransformation( SplitOperation, InsertOperation, ( a, b ) => {
+setTransformation( SplitOperation, InsertOperation, ( a, b, context ) => {
+	if ( a.position.isEqual( b.position ) && context.baRelation == 'insertAtSource' ) {
+		return [ a ];
+	}
+
 	a.position = a.position._getTransformedByInsertOperation( b );
 
 	return [ a ];
@@ -1251,7 +1484,7 @@ setTransformation( SplitOperation, MergeOperation, ( a, b ) => {
 	return [ a ];
 } );
 
-setTransformation( SplitOperation, MoveOperation, ( a, b ) => {
+setTransformation( SplitOperation, MoveOperation, ( a, b, context ) => {
 	if ( a.graveyardPosition ) {
 		a.graveyardPosition = a.graveyardPosition._getTransformedByMoveOperation( b );
 	}
@@ -1279,6 +1512,12 @@ setTransformation( SplitOperation, MoveOperation, ( a, b ) => {
 		return [ a ];
 	}
 
+	if ( a.position.isEqual( b.targetPosition ) && context.abRelation == 'splitBefore' ) {
+		a.position = a.position._getTransformedByDeletion( b.sourcePosition, b.howMany );
+
+		return [ a ];
+	}
+
 	// The default case.
 	//
 	a.position = a.position._getTransformedByMoveOperation( b );
@@ -1286,7 +1525,7 @@ setTransformation( SplitOperation, MoveOperation, ( a, b ) => {
 	return [ a ];
 } );
 
-setTransformation( SplitOperation, SplitOperation, ( a, b ) => {
+setTransformation( SplitOperation, SplitOperation, ( a, b, context ) => {
 	if ( a.position.isEqual( b.position ) ) {
 		if ( !a.graveyardPosition && !b.graveyardPosition ) {
 			return getNoOp();
@@ -1295,6 +1534,8 @@ setTransformation( SplitOperation, SplitOperation, ( a, b ) => {
 		if ( a.graveyardPosition && b.graveyardPosition && a.graveyardPosition.isEqual( b.graveyardPosition ) ) {
 			return getNoOp();
 		}
+	} else if ( a.position.isEqual( b.insertionPosition ) && context.abRelation == 'splitBefore' ) {
+		return [ a ];
 	} else {
 		a.position = a.position._getTransformedBySplitOperation( b );
 	}
@@ -1306,7 +1547,7 @@ setTransformation( SplitOperation, SplitOperation, ( a, b ) => {
 	return [ a ];
 } );
 
-setTransformation( SplitOperation, WrapOperation, ( a, b ) => {
+setTransformation( SplitOperation, WrapOperation, ( a, b, context ) => {
 	// Case 1:	If split position has been wrapped, reverse the wrapping so that split can be applied as intended.
 	//			This is an edge case scenario where it is difficult to find a correct solution.
 	//			Since it will be a rare (or only theoretical) scenario, the algorithm will perform the easy solution.
@@ -1324,20 +1565,33 @@ setTransformation( SplitOperation, WrapOperation, ( a, b ) => {
 		return [ reversed, a ];
 	}
 
-	a.position = a.position._getTransformedByWrapOperation( b );
+	if ( a.position.isEqual( b.position ) && context.abRelation == 'splitInside' ) {
+		a.position = b.targetPosition;
+	} else {
+		a.position = a.position._getTransformedByWrapOperation( b );
+	}
 
-	if ( a.graveyardPosition && b.graveyardPosition ) {
-		a.graveyardPosition = a.graveyardPosition._getTransformedByDeletion( b.graveyardPosition, 1 );
+	if ( a.graveyardPosition ) {
+		a.graveyardPosition = a.graveyardPosition._getTransformedByWrapOperation( b );
 	}
 
 	return [ a ];
 } );
 
-setTransformation( SplitOperation, UnwrapOperation, ( a, b ) => {
-	a.position = a.position._getTransformedByUnwrapOperation( b );
+setTransformation( SplitOperation, UnwrapOperation, ( a, b, context ) => {
+	const splitInside = a.position.hasSameParentAs( b.position );
+
+	if ( splitInside && !context.bWasUndone ) {
+		const path = b.graveyardPosition.path.slice();
+		path.push( 0 );
+
+		a.position = new Position( b.graveyardPosition.root, path );
+	} else {
+		a.position = a.position._getTransformedByUnwrapOperation( b );
+	}
 
 	if ( a.graveyardPosition ) {
-		a.graveyardPosition = a.graveyardPosition._getTransformedByInsertion( b.graveyardPosition, 1 );
+		a.graveyardPosition = a.graveyardPosition._getTransformedByUnwrapOperation( b );
 	}
 
 	return [ a ];
@@ -1345,7 +1599,13 @@ setTransformation( SplitOperation, UnwrapOperation, ( a, b ) => {
 
 // -----------------------
 
-setTransformation( WrapOperation, InsertOperation, ( a, b ) => {
+setTransformation( WrapOperation, InsertOperation, ( a, b, context ) => {
+	if ( a.position.isEqual( b.position ) && context.baRelation == 'insertInside' ) {
+		a.howMany += b.howMany;
+
+		return [ a ];
+	}
+
 	const transformed = a.wrappedRange._getTransformedByInsertOperation( b, false )[ 0 ];
 
 	a.position = transformed.start;
@@ -1355,31 +1615,41 @@ setTransformation( WrapOperation, InsertOperation, ( a, b ) => {
 } );
 
 setTransformation( WrapOperation, MergeOperation, ( a, b ) => {
+	if ( a.graveyardPosition ) {
+		a.graveyardPosition = a.graveyardPosition._getTransformedByInsertion( b.graveyardPosition, 1 );
+	}
+
+	// Case 1:	The element to wrap got merged.
+	//
+	if ( a.position.isEqual( b.deletionPosition ) ) {
+		a.position = Position.createFromPosition( b.graveyardPosition );
+		a.position.stickiness = 'toNext';
+
+		return [ a ];
+	}
+
 	const transformed = a.wrappedRange._getTransformedByMergeOperation( b );
 
 	a.position = transformed.start;
 	a.howMany = transformed.end.offset - transformed.start.offset;
 
-	if ( a.graveyardPosition ) {
-		a.graveyardPosition = a.graveyardPosition._getTransformedByInsertion( b.graveyardPosition, 1 );
-	}
-
 	return [ a ];
 } );
 
-setTransformation( WrapOperation, MoveOperation, ( a, b ) => {
+setTransformation( WrapOperation, MoveOperation, ( a, b, context ) => {
 	if ( a.graveyardPosition ) {
 		a.graveyardPosition = a.graveyardPosition._getTransformedByMoveOperation( b );
 	}
 
-	const ranges = breakRangeByMoveOperation( a.wrappedRange, b, false );
-
-	if ( ranges.length == 0 ) {
-		const range = a.wrappedRange._getTransformedByMoveOperation( b )[ 0 ];
+	if ( a.position.isEqual( b.targetPosition ) && context.baRelation == 'insertInside' ) {
+		a.position._getTransformedByDeletion( b.sourcePosition, b.howMany );
+		a.howMany += b.howMany;
 
-		ranges.push( range );
+		return [ a ];
 	}
 
+	const ranges = breakRangeByMoveOperation( a.wrappedRange, b, false );
+
 	return ranges.reverse().map( range => {
 		const howMany = range.end.offset - range.start.offset;
 		const elementOrGraveyardPosition = a.graveyardPosition ? a.graveyardPosition : a.element;
@@ -1388,20 +1658,25 @@ setTransformation( WrapOperation, MoveOperation, ( a, b ) => {
 	} );
 } );
 
-setTransformation( WrapOperation, SplitOperation, ( a, b ) => {
-	// Case 1:	If range to wrap got split by split operation cancel the wrapping.
+setTransformation( WrapOperation, SplitOperation, ( a, b, context ) => {
+	// Case 1:	If range to wrap got split cancel the wrapping.
+	//			Do that only if this is not undo mode. If `b` operation was earlier transformed by unwrap operation
+	//			and the split position was inside the unwrapped range, then proceed without special case.
 	//
-	if ( a.position.hasSameParentAs( b.position ) && a.wrappedRange.containsPosition( b.position ) ) {
+	const isInside = a.position.hasSameParentAs( b.position ) && a.wrappedRange.containsPosition( b.position );
+
+	if ( isInside && context.baRelation !== 'splitInside' ) {
 		// We cannot just return no-op in this case, because in the mirror case scenario the wrap is reversed, which
 		// might introduce a new node in the graveyard (if the wrap didn't have `graveyardPosition`, then the wrap
 		// created a new element which was put to the graveyard when the wrap was reversed).
 		//
 		// Instead, a node in graveyard will be inserted.
+		//
 		if ( a.element ) {
 			const graveyard = a.position.root.document.graveyard;
 			const graveyardPosition = new Position( graveyard, [ 0 ] );
 
-			return new InsertOperation( graveyardPosition, a.element, 0 );
+			return [ new InsertOperation( graveyardPosition, a.element, 0 ) ];
 		} else {
 			return getNoOp();
 		}
@@ -1609,11 +1884,21 @@ setTransformation( UnwrapOperation, MergeOperation, ( a, b, context ) => {
 		return [ b.getReversed(), a ];
 	}
 
-	const transformed = a.unwrappedRange._getTransformedByMergeOperation( b );
+	// // Case 2:	The element to unwrap was merged-to and has new nodes.
+	// //
+	// if ( a.position.hasSameParentAs( b.targetPosition ) ) {
+	// 	// Merge operation needs `howMany`!
+	// }
 
-	a.position = transformed.start;
-	a.position.stickiness = 'toPrevious';
-	a.howMany = transformed.end.offset - transformed.start.offset;
+	if ( a.position.hasSameParentAs( b.graveyardPosition ) ) {
+		a.howMany++;
+	}
+
+	if ( a.position.hasSameParentAs( b.deletionPosition ) ) {
+		a.howMany--;
+	}
+
+	a.position = a.position._getTransformedByMergeOperation( b );
 
 	if ( !a.graveyardPosition.isEqual( b.graveyardPosition ) || !context.aIsStrong ) {
 		a.graveyardPosition = a.graveyardPosition._getTransformedByMergeOperation( b );
@@ -1622,7 +1907,7 @@ setTransformation( UnwrapOperation, MergeOperation, ( a, b, context ) => {
 	return [ a ];
 } );
 
-setTransformation( UnwrapOperation, MoveOperation, ( a, b, context ) => {
+setTransformation( UnwrapOperation, MoveOperation, ( a, b ) => {
 	// Case 1:	Move operation moves nodes from the unwrapped element.
 	//			This does not have any impact on `UnwrapOperation#position`, but `#howMany` has to be changed.
 	//
@@ -1640,7 +1925,7 @@ setTransformation( UnwrapOperation, MoveOperation, ( a, b, context ) => {
 
 	a.position = a.position._getTransformedByMoveOperation( b );
 
-	if ( !a.graveyardPosition.isEqual( b.targetPosition ) || !context.aIsStrong ) {
+	if ( !a.graveyardPosition.isEqual( b.targetPosition ) ) {
 		a.graveyardPosition = a.graveyardPosition._getTransformedByMoveOperation( b );
 	}
 
@@ -1667,17 +1952,17 @@ setTransformation( UnwrapOperation, SplitOperation, ( a, b ) => {
 	// Case 2:	The split element is the last element in unwrapped element. In this case, we need to manually modify
 	//			`howMany` property because it wouldn't be correctly calculated by `_getTransformedBySplitOperation`.
 	//
-	if ( b.insertionPosition.isEqual( a.position.getShiftedBy( a.howMany ) ) ) {
+	if ( a.position.hasSameParentAs( b.insertionPosition ) ) {
 		a.howMany++;
 
 		return [ a ];
 	}
 
-	const transformed = a.unwrappedRange._getTransformedBySplitOperation( b );
+	a.position = a.position._getTransformedBySplitOperation( b );
 
-	a.position = transformed.start;
-	a.position.stickiness = 'toPrevious';
-	a.howMany = transformed.end.offset - transformed.start.offset;
+	if ( b.graveyardPosition && b.graveyardPosition.hasSameParentAs( a.position ) ) {
+		a.howMany--;
+	}
 
 	a.graveyardPosition = a.graveyardPosition._getTransformedBySplitOperation( b );
 
@@ -1691,6 +1976,10 @@ setTransformation( UnwrapOperation, WrapOperation, ( a, b ) => {
 		a.howMany = a.howMany - b.howMany + 1;
 	}
 
+	if ( b.graveyardPosition && compareArrays( a.position.getParentPath(), b.graveyardPosition.path ) == 'same' ) {
+		a.howMany = b.howMany;
+	}
+
 	// The default case.
 	//
 	a.position = a.position._getTransformedByWrapOperation( b );
@@ -1709,6 +1998,8 @@ setTransformation( UnwrapOperation, UnwrapOperation, ( a, b, context ) => {
 		a.position = new Position( b.graveyardPosition.root, path );
 		a.howMany = 0;
 		a.graveyardPosition = Position.createFromPosition( b.graveyardPosition );
+
+		return [ a ];
 	}
 
 	a.position = a.position._getTransformedByUnwrapOperation( b );
@@ -1759,33 +2050,19 @@ function makeMoveOperationsFromRanges( ranges, targetPosition ) {
 			ranges[ j ] = ranges[ j ]._getTransformedByMove( op.sourcePosition, op.targetPosition, op.howMany )[ 0 ];
 		}
 
-		// targetPosition.stickiness = 'toPrevious';
-		targetPosition = targetPosition._getTransformedByMove( op.sourcePosition, op.targetPosition, op.howMany, true );
+		targetPosition = targetPosition._getTransformedByMove( op.sourcePosition, op.targetPosition, op.howMany );
 	}
 
 	return operations;
 }
 
 function makeMoveOperation( range, targetPosition ) {
-	// We want to keep correct operation class.
-	let OperationClass;
-
-	if ( targetPosition.root.rootName == '$graveyard' ) {
-		OperationClass = RemoveOperation;
-	} else if ( range.start.root.rootName == '$graveyard' ) {
-		OperationClass = ReinsertOperation;
-	} else {
-		OperationClass = MoveOperation;
-	}
-
 	targetPosition.stickiness = 'toNone';
 
-	const result = new OperationClass(
+	return new MoveOperation(
 		range.start,
 		range.end.offset - range.start.offset,
 		targetPosition,
-		0 // Is corrected anyway later.
+		0
 	);
-
-	return result;
 }

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

@@ -62,6 +62,10 @@ export default class WrapOperation extends Operation {
 		this.element = elementOrPosition instanceof Element ? elementOrPosition : null;
 
 		this.graveyardPosition = elementOrPosition instanceof Element ? null : Position.createFromPosition( elementOrPosition );
+
+		if ( this.graveyardPosition ) {
+			this.graveyardPosition.stickiness = 'toNext';
+		}
 	}
 
 	/**
@@ -148,7 +152,10 @@ export default class WrapOperation extends Operation {
 		const targetPosition = new Position( this.position.root, targetPath );
 
 		if ( this.element ) {
-			_insert( insertPosition, this.element._clone() );
+			const originalElement = this.element;
+			this.element = this.element._clone();
+
+			_insert( insertPosition, originalElement );
 		} else {
 			_move( Range.createFromPositionAndShift( this.graveyardPosition, 1 ), insertPosition );
 		}

+ 16 - 4
packages/ckeditor5-engine/src/model/position.js

@@ -563,6 +563,8 @@ export default class Position {
 				// Above happens during OT when the merged element is moved before the merged-to element.
 				pos = pos._getTransformedByDeletion( operation.deletionPosition, 1 );
 			}
+		} else if ( this.isEqual( operation.deletionPosition ) ) {
+			pos = Position.createFromPosition( operation.deletionPosition );
 		} else {
 			pos = this._getTransformedByMove( operation.deletionPosition, operation.graveyardPosition, 1 );
 		}
@@ -599,15 +601,24 @@ export default class Position {
 			unwrappedRange.start.isEqual( this ) ||
 			unwrappedRange.end.isEqual( this );
 
+		let pos;
+
 		if ( isContained ) {
-			return this._getCombined( operation.position, operation.targetPosition );
+			pos = this._getCombined( operation.position, operation.targetPosition );
 		} else if ( this.isEqual( operation.targetPosition ) ) {
-			return Position.createFromPosition( this );
+			pos = Position.createFromPosition( this );
 		} else {
-			const pos = this._getTransformedByInsertion( operation.targetPosition, operation.howMany - 1 );
+			pos = this._getTransformedByInsertion( operation.targetPosition, operation.howMany );
+		}
 
-			return pos._getTransformedByInsertion( operation.graveyardPosition, 1 );
+		const targetPosition = operation.targetPosition.getShiftedBy( operation.howMany );
+
+		if ( !targetPosition.isEqual( operation.graveyardPosition ) ) {
+			pos = pos._getTransformedByDeletion( targetPosition, 1 );
+			pos = pos._getTransformedByInsertion( operation.graveyardPosition, 1 );
 		}
+
+		return pos;
 	}
 
 	/**
@@ -761,6 +772,7 @@ export default class Position {
 
 		// The first part of a path to combined position is a path to the place where nodes were moved.
 		const combined = Position.createFromPosition( target );
+		combined.stickiness = this.stickiness;
 
 		// Then we have to update the rest of the path.
 

+ 27 - 6
packages/ckeditor5-engine/src/model/range.js

@@ -493,16 +493,37 @@ export default class Range {
 	}
 
 	_getTransformedByMergeOperation( operation ) {
-		const start = this.start._getTransformedByMergeOperation( operation );
-		const end = this.end._getTransformedByMergeOperation( operation );
+		let start = this.start._getTransformedByMergeOperation( operation );
+		let end = this.end._getTransformedByMergeOperation( operation );
 
 		if ( start.isAfter( end ) ) {
-			// This happens in the following case:
+			// This happens in the following two, similar cases:
 			//
-			// <p>abc</p>{<p>x}yz</p> -> <p>abcx}yz</p>{
-			//                           <p>abcx{yz</p>}
+			// Case 1: Range start is directly before merged node.
+			//         Resulting range should include only nodes from the merged element:
 			//
-			return new Range( end, start );
+			// Before: <p>aa</p>{<p>b}b</p><p>cc</p>
+			// Merge:  <p>aab}b</p>{<p>cc</p>
+			// Fix:    <p>aa{b}b</p><p>cc</p>
+			//
+			// Case 2: Range start is not directly before merged node.
+			//         Result should include all nodes that were in the original range.
+			//
+			// Before: <p>aa</p>{<p>cc</p><p>b}b</p>
+			// Merge:  <p>aab}b</p>{<p>cc</p>
+			// Fix:    <p>aa{bb</p><p>cc</p>}
+			//
+			//         The range is expanded by an additional `b` letter but it is better than dropping the whole `cc` paragraph.
+			//
+			if ( !operation.deletionPosition.isEqual( start ) ) {
+				// Case 2.
+				end = operation.deletionPosition;
+			}
+
+			// In both cases start is at the end of the merge-to element.
+			start = operation.targetPosition;
+
+			return new Range( start, end );
 		}
 
 		return new Range( start, end );

+ 46 - 19
packages/ckeditor5-engine/src/model/schema.js

@@ -665,38 +665,54 @@ export default class Schema {
 	 *
 	 * @param {Array.<module:engine/model/range~Range>} ranges Ranges to be validated.
 	 * @param {String} attribute The name of the attribute to check.
-	 * @returns {Array.<module:engine/model/range~Range>} Ranges in which the attribute is allowed.
+	 * @returns {Iterator.<module:engine/model/range~Range>} Ranges in which the attribute is allowed.
 	 */
-	getValidRanges( ranges, attribute ) {
-		const validRanges = [];
+	* getValidRanges( ranges, attribute ) {
+		ranges = convertToMinimalFlatRanges( ranges );
 
 		for ( const range of ranges ) {
-			let last = range.start;
-			let from = range.start;
-			const to = range.end;
-
-			for ( const value of range.getWalker() ) {
-				if ( !this.checkAttribute( value.item, attribute ) ) {
-					if ( !from.isEqual( last ) ) {
-						validRanges.push( new Range( from, last ) );
-					}
+			yield* this._getValidRangesForRange( range, attribute );
+		}
+	}
 
-					from = value.nextPosition;
-				}
+	/**
+	 * Takes a flat range and an attribute name. Traverses the range recursively and deeply to find and return all ranges
+	 * inside the given range on which the attribute can be applied.
+	 *
+	 * This is a helper function for {@link ~Schema#getValidRanges}.
+	 *
+	 * @private
+	 * @param {module:engine/model/range~Range} range Range to process.
+	 * @param {String} attribute The name of the attribute to check.
+	 * @returns {Iterator.<module:engine/model/range~Range>} Ranges in which the attribute is allowed.
+	 */
+	* _getValidRangesForRange( range, attribute ) {
+		let start = range.start;
+		let end = range.start;
 
-				last = value.nextPosition;
+		for ( const item of range.getItems( { shallow: true } ) ) {
+			if ( item.is( 'element' ) ) {
+				yield* this._getValidRangesForRange( Range.createIn( item ), attribute );
 			}
 
-			if ( from && !from.isEqual( to ) ) {
-				validRanges.push( new Range( from, to ) );
+			if ( !this.checkAttribute( item, attribute ) ) {
+				if ( !start.isEqual( end ) ) {
+					yield new Range( start, end );
+				}
+
+				start = Position.createAfter( item );
 			}
+
+			end = Position.createAfter( item );
 		}
 
-		return validRanges;
+		if ( !start.isEqual( end ) ) {
+			yield new Range( start, end );
+		}
 	}
 
 	/**
-	 * Basing on given the `position`, finds and returns a {@link module:engine/model/range~Range range} which is
+	 * Basing on given `position`, finds and returns a {@link module:engine/model/range~Range range} which is
 	 * nearest to that `position` and is a correct range for selection.
 	 *
 	 * The correct selection range might be collapsed when it is located in a position where the text node can be placed.
@@ -1572,3 +1588,14 @@ function* combineWalkers( backward, forward ) {
 		}
 	}
 }
+
+// Takes an array of non-intersecting ranges. For each of them gets minimal flat ranges covering that range and returns
+// all those minimal flat ranges.
+//
+// @param {Array.<module:engine/model/range~Range>} ranges Ranges to process.
+// @returns {Iterator.<module:engine/model/range~Range>} Minimal flat ranges of given `ranges`.
+function* convertToMinimalFlatRanges( ranges ) {
+	for ( const range of ranges ) {
+		yield* range.getMinimalFlatRanges();
+	}
+}

+ 15 - 1
packages/ckeditor5-engine/src/model/selection.js

@@ -431,7 +431,21 @@ export default class Selection {
 		// Check whether there is any range in new ranges set that is different than all already added ranges.
 		const anyNewRange = newRanges.some( newRange => {
 			if ( !( newRange instanceof Range ) ) {
-				throw new CKEditorError( 'model-selection-added-not-range: Trying to add an object that is not an instance of Range.' );
+				/**
+				 * Selection range set to an object that is not an instance of {@link module:engine/model/range~Range}.
+				 *
+				 * Only {@link module:engine/model/range~Range} instances can be used to set a selection.
+				 * Common mistakes leading to this error are:
+				 *
+				 * * using DOM `Range` object,
+				 * * incorrect CKEditor 5 installation with multiple `ckeditor5-engine` packages having different versions.
+				 *
+				 * @error model-selection-set-ranges-not-range
+				 */
+				throw new CKEditorError(
+					'model-selection-set-ranges-not-range: ' +
+					'Selection range set to an object that is not an instance of model.Range.'
+				);
 			}
 
 			return this._ranges.every( oldRange => {

+ 2 - 3
packages/ckeditor5-engine/src/model/writer.js

@@ -12,7 +12,6 @@ import DetachOperation from './operation/detachoperation';
 import InsertOperation from './operation/insertoperation';
 import MarkerOperation from './operation/markeroperation';
 import MoveOperation from './operation/moveoperation';
-import RemoveOperation from './operation/removeoperation';
 import RenameOperation from './operation/renameoperation';
 import RootAttributeOperation from './operation/rootattributeoperation';
 import SplitOperation from './operation/splitoperation';
@@ -1330,7 +1329,7 @@ function applyMarkerOperation( writer, name, oldRange, newRange, affectsData ) {
 	model.applyOperation( operation );
 }
 
-// Creates `RemoveOperation` or `DetachOperation` that removes `howMany` nodes starting from `position`.
+// Creates `MoveOperation` or `DetachOperation` that removes `howMany` nodes starting from `position`.
 // The operation will be applied on given model instance and added to given operation instance.
 //
 // @private
@@ -1345,7 +1344,7 @@ function applyRemoveOperation( position, howMany, batch, model ) {
 		const doc = model.document;
 		const graveyardPosition = new Position( doc.graveyard, [ 0 ] );
 
-		operation = new RemoveOperation( position, howMany, graveyardPosition, doc.version );
+		operation = new MoveOperation( position, howMany, graveyardPosition, doc.version );
 	} else {
 		operation = new DetachOperation( position, howMany );
 	}

+ 9 - 1
packages/ckeditor5-engine/src/view/selection.js

@@ -643,7 +643,15 @@ export default class Selection {
 	 */
 	_addRange( range, isBackward = false ) {
 		if ( !( range instanceof Range ) ) {
-			throw new CKEditorError( 'view-selection-invalid-range: Invalid Range.' );
+			/**
+			 * Selection range set to an object that is not an instance of {@link module:engine/view/range~Range}.
+			 *
+			 * @error view-selection-add-range-not-range
+			 */
+			throw new CKEditorError(
+				'view-selection-add-range-not-range: ' +
+				'Selection range set to an object that is not an instance of view.Range'
+			);
 		}
 
 		this._pushRange( range );

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

@@ -21,8 +21,6 @@ import MoveOperation from '../../src/model/operation/moveoperation';
 import NoOperation from '../../src/model/operation/nooperation';
 import RenameOperation from '../../src/model/operation/renameoperation';
 import RootAttributeOperation from '../../src/model/operation/rootattributeoperation';
-import RemoveOperation from '../../src/model/operation/removeoperation';
-import transform from '../../src/model/operation/transform';
 import Model from '../../src/model/model';
 import ModelDocumentFragment from '../../src/model/documentfragment';
 
@@ -532,8 +530,8 @@ describe( 'debug tools', () => {
 				model.applyOperation( insert );
 
 				const graveyard = modelDoc.graveyard;
-				const remove = new RemoveOperation( ModelPosition.createAt( modelRoot, 1 ), 2, ModelPosition.createAt( graveyard, 0 ), 1 );
-				model.applyOperation( remove );
+				const move = new MoveOperation( ModelPosition.createAt( modelRoot, 1 ), 2, ModelPosition.createAt( graveyard, 0 ), 1 );
+				model.applyOperation( move );
 			} );
 
 			log.resetHistory();
@@ -668,39 +666,6 @@ describe( 'debug tools', () => {
 		} );
 	} );
 
-	describe( 'should provide error logging for transformation', () => {
-		let model, document, root, otherRoot;
-
-		beforeEach( () => {
-			model = new Model();
-			document = model.document;
-			root = document.createRoot();
-			otherRoot = document.createRoot( 'other', 'other' );
-		} );
-
-		it.skip( 'with more important operation A', () => {
-			const opA = new MoveOperation( ModelPosition.createAt( root, 4 ), 4, ModelPosition.createAt( otherRoot, 4 ), 0 );
-			const opB = new InsertOperation( ModelPosition.createAt( root, 0 ), new ModelText( 'a' ), 0 );
-
-			expect( () => {
-				transform.transform( opA, opB, { isStrong: true } );
-			} ).to.throw( Error );
-			expect( error.calledWith( opA.toString() + ' (important)' ) ).to.be.true;
-			expect( error.calledWith( opB.toString() ) ).to.be.true;
-		} );
-
-		it.skip( 'with more important operation B', () => {
-			const opA = new MoveOperation( ModelPosition.createAt( root, 4 ), 4, ModelPosition.createAt( otherRoot, 4 ), 0 );
-			const opB = new InsertOperation( ModelPosition.createAt( root, 0 ), new ModelText( 'a' ), 0 );
-
-			testUtils.sinon.stub( transform, 'transform' ).throws( new Error() );
-
-			expect( () => transform.transform( opA, opB, { isStrong: true } ) ).to.throw( Error );
-			expect( error.calledWith( opA.toString() ) ).to.be.true;
-			expect( error.calledWith( opB.toString() + ' (important)' ) ).to.be.true;
-		} );
-	} );
-
 	function expectLog( expectedLogMsg ) {
 		expect( log.calledWithExactly( expectedLogMsg ) ).to.be.true;
 		log.resetHistory();

+ 1 - 2
packages/ckeditor5-engine/tests/model/differ.js

@@ -10,7 +10,6 @@ import Position from '../../src/model/position';
 import Range from '../../src/model/range';
 
 import InsertOperation from '../../src/model/operation/insertoperation';
-import RemoveOperation from '../../src/model/operation/removeoperation';
 import MoveOperation from '../../src/model/operation/moveoperation';
 import RenameOperation from '../../src/model/operation/renameoperation';
 import AttributeOperation from '../../src/model/operation/attributeoperation';
@@ -1617,7 +1616,7 @@ describe( 'Differ', () => {
 
 	function remove( sourcePosition, howMany ) {
 		const targetPosition = Position.createAt( doc.graveyard, doc.graveyard.maxOffset );
-		const operation = new RemoveOperation( sourcePosition, howMany, targetPosition, doc.version );
+		const operation = new MoveOperation( sourcePosition, howMany, targetPosition, doc.version );
 
 		model.applyOperation( operation );
 	}

+ 6 - 7
packages/ckeditor5-engine/tests/model/documentselection.js

@@ -13,7 +13,6 @@ import LiveRange from '../../src/model/liverange';
 import DocumentSelection from '../../src/model/documentselection';
 import InsertOperation from '../../src/model/operation/insertoperation';
 import MoveOperation from '../../src/model/operation/moveoperation';
-import RemoveOperation from '../../src/model/operation/removeoperation';
 import AttributeOperation from '../../src/model/operation/attributeoperation';
 import SplitOperation from '../../src/model/operation/splitoperation';
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
@@ -334,7 +333,7 @@ describe( 'DocumentSelection', () => {
 		it( 'should throw an error when range is invalid', () => {
 			expect( () => {
 				selection._setTo( [ { invalid: 'range' } ] );
-			} ).to.throw( CKEditorError, /model-selection-added-not-range/ );
+			} ).to.throw( CKEditorError, /model-selection-set-ranges-not-range/ );
 		} );
 
 		it( 'should log a warning when trying to set selection to the graveyard', () => {
@@ -1131,12 +1130,12 @@ describe( 'DocumentSelection', () => {
 			} );
 		} );
 
-		describe( 'RemoveOperation', () => {
+		describe( 'MoveOperation to graveyard', () => {
 			it( 'fix selection range if it ends up in graveyard #1', () => {
 				selection._setTo( new Position( root, [ 1, 3 ] ) );
 
 				model.applyOperation(
-					new RemoveOperation(
+					new MoveOperation(
 						new Position( root, [ 1, 2 ] ),
 						2,
 						new Position( doc.graveyard, [ 0 ] ),
@@ -1151,7 +1150,7 @@ describe( 'DocumentSelection', () => {
 				selection._setTo( [ new Range( new Position( root, [ 1, 2 ] ), new Position( root, [ 1, 4 ] ) ) ] );
 
 				model.applyOperation(
-					new RemoveOperation(
+					new MoveOperation(
 						new Position( root, [ 1, 2 ] ),
 						2,
 						new Position( doc.graveyard, [ 0 ] ),
@@ -1166,7 +1165,7 @@ describe( 'DocumentSelection', () => {
 				selection._setTo( [ new Range( new Position( root, [ 1, 1 ] ), new Position( root, [ 1, 2 ] ) ) ] );
 
 				model.applyOperation(
-					new RemoveOperation(
+					new MoveOperation(
 						new Position( root, [ 1 ] ),
 						2,
 						new Position( doc.graveyard, [ 0 ] ),
@@ -1179,7 +1178,7 @@ describe( 'DocumentSelection', () => {
 
 			it( 'fix selection range if it ends up in graveyard #4 - whole content removed', () => {
 				model.applyOperation(
-					new RemoveOperation(
+					new MoveOperation(
 						new Position( root, [ 0 ] ),
 						3,
 						new Position( doc.graveyard, [ 0 ] ),

+ 64 - 17
packages/ckeditor5-engine/tests/model/liverange.js

@@ -3,13 +3,14 @@
  * For licensing, see LICENSE.md.
  */
 
-import Batch from '../../src/model/batch';
 import Model from '../../src/model/model';
 import Element from '../../src/model/element';
 import Position from '../../src/model/position';
 import LiveRange from '../../src/model/liverange';
 import Range from '../../src/model/range';
 import Text from '../../src/model/text';
+import MoveOperation from '../../src/model/operation/moveoperation';
+import MergeOperation from '../../src/model/operation/mergeoperation';
 import { stringify, setData } from '../../src/dev-utils/model';
 
 describe( 'LiveRange', () => {
@@ -90,7 +91,7 @@ describe( 'LiveRange', () => {
 		range.detach();
 	} );
 
-	it( 'should fire change:range event with proper data when its boundaries are changed', () => {
+	it( 'should fire change:range event with when its boundaries are changed', () => {
 		const live = new LiveRange( new Position( root, [ 0, 1, 4 ] ), new Position( root, [ 0, 2, 2 ] ) );
 		const copy = Range.createFromRange( live );
 
@@ -99,15 +100,11 @@ describe( 'LiveRange', () => {
 
 		const sourcePosition = new Position( root, [ 2 ] );
 		const targetPosition = new Position( root, [ 0 ] );
-		const batch = new Batch();
-		let op = null;
 
-		model.enqueueChange( batch, writer => {
+		model.change( writer => {
 			const sourceRange = Range.createFromPositionAndShift( sourcePosition, 1 );
 
 			writer.move( sourceRange, targetPosition );
-
-			op = batch.operations[ 0 ];
 		} );
 
 		expect( spy.calledOnce ).to.be.true;
@@ -115,11 +112,11 @@ describe( 'LiveRange', () => {
 		// First parameter available in event should be a range that is equal to the live range before the live range changed.
 		expect( spy.args[ 0 ][ 1 ].isEqual( copy ) ).to.be.true;
 
-		// Second parameter is the operation that changed the range.
-		expect( spy.args[ 0 ][ 2 ] ).to.equal( op );
+		// Second parameter is null for operations that did not move the range into graveyard.
+		expect( spy.args[ 0 ][ 2 ].deletionPosition ).to.be.null;
 	} );
 
-	it( 'should fire change:content event with proper data when content inside the range has changed', () => {
+	it( 'should fire change:content event when content inside the range has changed', () => {
 		const live = new LiveRange( new Position( root, [ 0, 1 ] ), new Position( root, [ 0, 3 ] ) );
 
 		const spy = sinon.spy();
@@ -127,15 +124,11 @@ describe( 'LiveRange', () => {
 
 		const sourcePosition = new Position( root, [ 0, 2, 0 ] );
 		const targetPosition = new Position( root, [ 0, 4, 0 ] );
-		const batch = new Batch();
-		let op = null;
 
-		model.enqueueChange( batch, writer => {
+		model.change( writer => {
 			const sourceRange = Range.createFromPositionAndShift( sourcePosition, 2 );
 
 			writer.move( sourceRange, targetPosition );
-
-			op = batch.operations[ 0 ];
 		} );
 
 		expect( spy.calledOnce ).to.be.true;
@@ -143,8 +136,62 @@ describe( 'LiveRange', () => {
 		// First parameter available in event should be a range that is equal to the live range before the live range changed.
 		expect( spy.args[ 0 ][ 1 ].isEqual( live ) ).to.be.true;
 
-		// Second parameter is the operation that changed the range.
-		expect( spy.args[ 0 ][ 2 ] ).to.equal( op );
+		// Second parameter is null for operations that did not move the range into graveyard.
+		expect( spy.args[ 0 ][ 2 ].deletionPosition ).to.be.null;
+	} );
+
+	it( 'should pass deletion position if range was removed (remove)', () => {
+		const live = new LiveRange( new Position( root, [ 0, 2 ] ), new Position( root, [ 0, 4 ] ) );
+
+		const spy = sinon.spy();
+		live.on( 'change:range', spy );
+
+		const sourcePosition = new Position( root, [ 0, 0 ] );
+
+		model.change( writer => {
+			writer.remove( Range.createFromPositionAndShift( sourcePosition, 6 ) );
+		} );
+
+		// Second parameter is deletion position.
+		expect( spy.args[ 0 ][ 2 ].deletionPosition.isEqual( sourcePosition ) ).to.be.true;
+	} );
+
+	// This scenario is hypothetically possible during OT if the element to merge-into was removed.
+	// In that case a live range inside the merged element will be merged into an element which is in graveyard.
+	// Because it may happen only in OT, in the test below we will generate operations by hand.
+	it( 'should pass deletion position if range was removed (merge)', () => {
+		const live = new LiveRange( new Position( root, [ 1, 0 ] ), new Position( root, [ 1, 1 ] ) );
+
+		const spy = sinon.spy();
+		live.on( 'change:range', spy );
+
+		model.change( writer => {
+			const batch = writer.batch;
+			const gy = model.document.graveyard;
+
+			const remove = new MoveOperation(
+				new Position( root, [ 0 ] ),
+				1,
+				new Position( gy, [ 0 ] ),
+				model.document.version
+			);
+
+			const merge = new MergeOperation(
+				new Position( root, [ 0, 0 ] ),
+				new Position( gy, [ 0, 0 ] ),
+				new Position( gy, [ 0 ] ),
+				model.document.version + 1
+			);
+
+			batch.addOperation( remove );
+			model.applyOperation( remove );
+
+			batch.addOperation( merge );
+			model.applyOperation( merge );
+		} );
+
+		// Second parameter is deletion position.
+		expect( spy.args[ 1 ][ 2 ].deletionPosition.isEqual( new Position( root, [ 0 ] ) ) ).to.be.true;
 	} );
 
 	describe( 'should get transformed and fire change:range if', () => {

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

@@ -7,7 +7,7 @@ import Model from '../../../src/model/model';
 import NodeList from '../../../src/model/nodelist';
 import Element from '../../../src/model/element';
 import InsertOperation from '../../../src/model/operation/insertoperation';
-import RemoveOperation from '../../../src/model/operation/removeoperation';
+import MoveOperation from '../../../src/model/operation/moveoperation';
 import Position from '../../../src/model/position';
 import Text from '../../../src/model/text';
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
@@ -107,7 +107,7 @@ describe( 'InsertOperation', () => {
 		expect( root.getChild( 0 ).data ).to.equal( 'fooxbar' );
 	} );
 
-	it( 'should create a RemoveOperation as a reverse', () => {
+	it( 'should create a MoveOperation as a reverse', () => {
 		const position = new Position( root, [ 0 ] );
 		const operation = new InsertOperation(
 			position,
@@ -117,7 +117,7 @@ describe( 'InsertOperation', () => {
 
 		const reverse = operation.getReversed();
 
-		expect( reverse ).to.be.an.instanceof( RemoveOperation );
+		expect( reverse ).to.be.an.instanceof( MoveOperation );
 		expect( reverse.baseVersion ).to.equal( 1 );
 		expect( reverse.sourcePosition.isEqual( position ) ).to.be.true;
 		expect( reverse.howMany ).to.equal( 7 );

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

@@ -12,23 +12,27 @@ import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 import { jsonParseStringify } from '../../../tests/model/_utils/utils';
 
 describe( 'MoveOperation', () => {
-	let model, doc, root;
+	let model, doc, root, gy;
 
 	beforeEach( () => {
 		model = new Model();
 		doc = model.document;
 		root = doc.createRoot();
+		gy = doc.graveyard;
 	} );
 
 	it( 'should have proper type', () => {
-		const op = new MoveOperation(
-			new Position( root, [ 0, 0 ] ),
-			1,
-			new Position( root, [ 1, 0 ] ),
-			doc.version
-		);
+		const move = new MoveOperation( new Position( root, [ 0, 0 ] ), 1, new Position( root, [ 1, 0 ] ), 0 );
+		expect( move.type ).to.equal( 'move' );
+
+		const remove1 = new MoveOperation( new Position( root, [ 0, 0 ] ), 1, new Position( gy, [ 0 ] ), 0 );
+		expect( remove1.type ).to.equal( 'remove' );
+
+		const remove2 = new MoveOperation( new Position( gy, [ 0 ] ), 1, new Position( gy, [ 1 ] ), 0 );
+		expect( remove2.type ).to.equal( 'remove' );
 
-		expect( op.type ).to.equal( 'move' );
+		const reinsert = new MoveOperation( new Position( gy, [ 0 ] ), 1, new Position( root, [ 0, 0 ] ), 0 );
+		expect( reinsert.type ).to.equal( 'reinsert' );
 	} );
 
 	it( 'should move from one node to another', () => {

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

@@ -1,157 +0,0 @@
-/**
- * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved.
- * For licensing, see LICENSE.md.
- */
-
-import Model from '../../../src/model/model';
-import ReinsertOperation from '../../../src/model/operation/reinsertoperation';
-import RemoveOperation from '../../../src/model/operation/removeoperation';
-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';
-import { jsonParseStringify } from '../../../tests/model/_utils/utils';
-
-describe( 'ReinsertOperation', () => {
-	let model, doc, root, graveyard, operation, graveyardPosition, rootPosition;
-
-	beforeEach( () => {
-		model = new Model();
-		doc = model.document;
-		root = doc.createRoot();
-		graveyard = doc.graveyard;
-
-		graveyardPosition = new Position( graveyard, [ 0 ] );
-		rootPosition = new Position( root, [ 0 ] );
-
-		operation = new ReinsertOperation(
-			graveyardPosition,
-			2,
-			rootPosition,
-			doc.version
-		);
-	} );
-
-	it( 'should have position property equal to the position where node will be reinserted', () => {
-		expect( operation.position.isEqual( rootPosition ) ).to.be.true;
-
-		// Setting also works:
-		operation.position = new Position( root, [ 1 ] );
-		expect( operation.position.isEqual( new Position( root, [ 1 ] ) ) ).to.be.true;
-	} );
-
-	it( 'should have proper type', () => {
-		expect( operation.type ).to.equal( 'reinsert' );
-	} );
-
-	it( 'should extend MoveOperation class', () => {
-		expect( operation ).to.be.instanceof( MoveOperation );
-	} );
-
-	it( 'should create ReinsertOperation with same parameters when cloned', () => {
-		const clone = operation.clone();
-
-		expect( clone ).to.be.instanceof( ReinsertOperation );
-		expect( clone.sourcePosition.isEqual( operation.sourcePosition ) ).to.be.true;
-		expect( clone.targetPosition.isEqual( operation.targetPosition ) ).to.be.true;
-		expect( clone.howMany ).to.equal( operation.howMany );
-		expect( clone.baseVersion ).to.equal( operation.baseVersion );
-	} );
-
-	it( 'should create RemoveOperation as a reverse', () => {
-		graveyard._appendChild( new Element( 'x' ) );
-
-		const reverse = operation.getReversed();
-
-		expect( reverse ).to.be.an.instanceof( RemoveOperation );
-		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;
-	} );
-
-	it( 'should create correct RemoveOperation when reversed if target position was in graveyard', () => {
-		const operation = new ReinsertOperation( new Position( doc.graveyard, [ 0 ] ), 1, new Position( doc.graveyard, [ 3 ] ), 0 );
-		const reverse = operation.getReversed();
-
-		expect( reverse.sourcePosition.path ).to.deep.equal( [ 2 ] );
-		expect( reverse.targetPosition.path ).to.deep.equal( [ 0 ] );
-	} );
-
-	it( 'should undo reinsert set of nodes by applying reverse operation', () => {
-		const reverse = operation.getReversed();
-
-		graveyard._insertChild( 0, new Text( 'xx' ) );
-
-		model.applyOperation( operation );
-
-		expect( doc.version ).to.equal( 1 );
-		expect( root.maxOffset ).to.equal( 2 );
-		expect( graveyard.maxOffset ).to.equal( 0 );
-
-		model.applyOperation( reverse );
-
-		expect( doc.version ).to.equal( 2 );
-		expect( root.maxOffset ).to.equal( 0 );
-		expect( graveyard.maxOffset ).to.equal( 2 );
-	} );
-
-	describe( '_validate()', () => {
-		it( 'should throw when target position is not in the document', () => {
-			const docFrag = new DocumentFragment();
-
-			graveyard._insertChild( 0, new Text( 'xx' ) );
-
-			operation = new ReinsertOperation(
-				graveyardPosition,
-				1,
-				Position.createAt( docFrag ),
-				doc.version
-			);
-
-			expect( () => {
-				operation._validate();
-			} ).to.throw( CKEditorError, /^reinsert-operation-to-detached-parent/ );
-		} );
-
-		it( 'should throw when source position is not in the document', () => {
-			const docFrag = new DocumentFragment( new Text( 'xx' ) );
-
-			operation = new ReinsertOperation(
-				Position.createAt( docFrag ),
-				1,
-				rootPosition,
-				doc.version
-			);
-
-			expect( () => {
-				operation._validate();
-			} ).to.throw( CKEditorError, /^reinsert-operation-on-detached-item/ );
-		} );
-	} );
-
-	describe( 'toJSON', () => {
-		it( 'should create proper json object', () => {
-			const serialized = jsonParseStringify( operation );
-
-			expect( serialized ).to.deep.equal( {
-				__className: 'engine.model.operation.ReinsertOperation',
-				baseVersion: 0,
-				howMany: 2,
-				sourcePosition: jsonParseStringify( operation.sourcePosition ),
-				targetPosition: jsonParseStringify( operation.targetPosition )
-			} );
-		} );
-	} );
-
-	describe( 'fromJSON', () => {
-		it( 'should create proper ReinsertOperation from json object', () => {
-			const serialized = jsonParseStringify( operation );
-			const deserialized = ReinsertOperation.fromJSON( serialized, doc );
-
-			expect( deserialized ).to.deep.equal( operation );
-		} );
-	} );
-} );

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

@@ -1,191 +0,0 @@
-/**
- * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved.
- * For licensing, see LICENSE.md.
- */
-
-import Model from '../../../src/model/model';
-import ReinsertOperation from '../../../src/model/operation/reinsertoperation';
-import RemoveOperation from '../../../src/model/operation/removeoperation';
-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';
-import { jsonParseStringify } from '../../../tests/model/_utils/utils';
-
-describe( 'RemoveOperation', () => {
-	let model, doc, root, graveyard;
-
-	beforeEach( () => {
-		model = new Model();
-		doc = model.document;
-		root = doc.createRoot();
-		graveyard = doc.graveyard;
-	} );
-
-	it( 'should have proper type', () => {
-		const op = new RemoveOperation(
-			new Position( root, [ 2 ] ),
-			2,
-			new Position( doc.graveyard, [ 0 ] ),
-			doc.version
-		);
-
-		expect( op.type ).to.equal( 'remove' );
-	} );
-
-	it( 'should extend MoveOperation class', () => {
-		const operation = new RemoveOperation(
-			new Position( root, [ 2 ] ),
-			2,
-			new Position( doc.graveyard, [ 0 ] ),
-			doc.version
-		);
-
-		expect( operation ).to.be.instanceof( MoveOperation );
-	} );
-
-	it( 'should be able to remove set of nodes and append them to graveyard root', () => {
-		root._insertChild( 0, new Text( 'fozbar' ) );
-
-		model.applyOperation(
-			new RemoveOperation(
-				new Position( root, [ 2 ] ),
-				2,
-				new Position( doc.graveyard, [ 0 ] ),
-				doc.version
-			)
-		);
-
-		expect( doc.version ).to.equal( 1 );
-		expect( root.maxOffset ).to.equal( 4 );
-		expect( root.getChild( 0 ).data ).to.equal( 'foar' );
-
-		expect( graveyard.maxOffset ).to.equal( 2 );
-		expect( graveyard.getChild( 0 ).data ).to.equal( 'zb' );
-	} );
-
-	it( 'should create RemoveOperation with same parameters when cloned', () => {
-		const pos = new Position( root, [ 2 ] );
-
-		const operation = new RemoveOperation( pos, 2, new Position( doc.graveyard, [ 0 ] ), doc.version );
-		const clone = operation.clone();
-
-		expect( clone ).to.be.instanceof( RemoveOperation );
-		expect( clone.sourcePosition.isEqual( pos ) ).to.be.true;
-		expect( clone.targetPosition.isEqual( operation.targetPosition ) ).to.be.true;
-		expect( clone.howMany ).to.equal( operation.howMany );
-		expect( clone.baseVersion ).to.equal( operation.baseVersion );
-	} );
-
-	it( 'should create ReinsertOperation when reversed', () => {
-		const position = new Position( root, [ 0 ] );
-		const operation = new RemoveOperation( position, 2, new Position( doc.graveyard, [ 0 ] ), 0 );
-		const reverse = operation.getReversed();
-
-		expect( reverse ).to.be.an.instanceof( ReinsertOperation );
-		expect( reverse.baseVersion ).to.equal( 1 );
-		expect( reverse.howMany ).to.equal( 2 );
-		expect( reverse.sourcePosition.isEqual( operation.targetPosition ) ).to.be.true;
-		expect( reverse.targetPosition.isEqual( position ) ).to.be.true;
-	} );
-
-	it( 'should create correct ReinsertOperation when reversed if source range was in graveyard', () => {
-		const operation = new RemoveOperation( new Position( doc.graveyard, [ 2 ] ), 1, new Position( doc.graveyard, [ 0 ] ), 0 );
-		const reverse = operation.getReversed();
-
-		expect( reverse.sourcePosition.path ).to.deep.equal( [ 0 ] );
-		expect( reverse.targetPosition.path ).to.deep.equal( [ 3 ] );
-	} );
-
-	it( 'should undo remove set of nodes by applying reverse operation', () => {
-		const position = new Position( root, [ 0 ] );
-		const operation = new RemoveOperation( position, 3, new Position( doc.graveyard, [ 0 ] ), 0 );
-		const reverse = operation.getReversed();
-
-		root._insertChild( 0, new Text( 'bar' ) );
-
-		model.applyOperation( operation );
-
-		expect( doc.version ).to.equal( 1 );
-		expect( root.maxOffset ).to.equal( 0 );
-
-		model.applyOperation( reverse );
-
-		expect( doc.version ).to.equal( 2 );
-		expect( root.maxOffset ).to.equal( 3 );
-		expect( root.getChild( 0 ).data ).to.equal( 'bar' );
-	} );
-
-	it( 'should properly remove a node that is already in a graveyard', () => {
-		doc.graveyard._appendChild( [ new Element( 'x' ), new Element( 'y' ), new Element( 'z' ) ] );
-
-		const position = new Position( doc.graveyard, [ 2 ] );
-		const operation = new RemoveOperation( position, 1, new Position( doc.graveyard, [ 0 ] ), 0 );
-
-		model.applyOperation( operation );
-
-		expect( doc.graveyard.childCount ).to.equal( 3 );
-		expect( doc.graveyard.getChild( 0 ).name ).to.equal( 'z' );
-		expect( doc.graveyard.getChild( 1 ).name ).to.equal( 'x' );
-		expect( doc.graveyard.getChild( 2 ).name ).to.equal( 'y' );
-	} );
-
-	describe( '_validate()', () => {
-		it( 'should throw when is executed on detached item', () => {
-			const docFrag = new DocumentFragment();
-			const item = new Element( 'foo' );
-
-			docFrag._appendChild( [ item ] );
-
-			const op = new RemoveOperation(
-				new Position( docFrag, [ 0 ] ),
-				1,
-				new Position( doc.graveyard, [ 0 ] ),
-				doc.version
-			);
-
-			expect( () => {
-				op._validate();
-			} ).to.throw( CKEditorError, /^remove-operation-on-detached-item/ );
-		} );
-	} );
-
-	describe( 'toJSON', () => {
-		it( 'should create proper json object', () => {
-			const op = new RemoveOperation(
-				new Position( root, [ 2 ] ),
-				2,
-				new Position( doc.graveyard, [ 0 ] ),
-				doc.version
-			);
-
-			const serialized = jsonParseStringify( op );
-
-			expect( serialized ).to.deep.equal( {
-				__className: 'engine.model.operation.RemoveOperation',
-				baseVersion: 0,
-				howMany: 2,
-				sourcePosition: jsonParseStringify( op.sourcePosition ),
-				targetPosition: jsonParseStringify( op.targetPosition )
-			} );
-		} );
-	} );
-
-	describe( 'fromJSON', () => {
-		it( 'should create proper RemoveOperation from json object', () => {
-			const op = new RemoveOperation(
-				new Position( root, [ 2 ] ),
-				2,
-				new Position( doc.graveyard, [ 0 ] ),
-				doc.version
-			);
-
-			const serialized = jsonParseStringify( op );
-			const deserialized = RemoveOperation.fromJSON( serialized, doc );
-
-			expect( deserialized ).to.deep.equal( op );
-		} );
-	} );
-} );

File diff suppressed because it is too large
+ 135 - 151
packages/ckeditor5-engine/tests/model/operation/transform.js


+ 2 - 2
packages/ckeditor5-engine/tests/model/operation/transform/attribute.js

@@ -325,7 +325,7 @@ describe( 'transform', () => {
 				kate.setData( '<paragraph>F[oo] Bar</paragraph>' );
 
 				john.setAttribute( 'bold', true );
-				kate.move( [ 0, 7 ], [ 0, 1 ], [ 0, 3 ] );
+				kate.move( [ 0, 7 ] );
 
 				syncClients();
 
@@ -795,7 +795,7 @@ describe( 'transform', () => {
 				expectClients( '<paragraph>FooB<$text bold="true">ar</$text></paragraph>' );
 			} );
 
-			it.skip( 'element into paragraph #2, then undo', () => {
+			it( 'element into paragraph #2, then undo', () => {
 				john.setData( '<paragraph>Foo</paragraph>[<paragraph>Bar</paragraph>]' );
 				kate.setData( '<paragraph>Foo</paragraph>[]<paragraph>Bar</paragraph>' );
 

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

@@ -811,7 +811,7 @@ describe( 'transform', () => {
 				expectClients( '<paragraph>Foo</paragraph><paragraph>BarAbc</paragraph>' );
 			} );
 
-			it.skip( 'element, then add marker, split and undo with type #2', () => {
+			it( 'element, then add marker, split and undo with type #2', () => {
 				john.setData( '<paragraph>Foo</paragraph>[]' );
 				kate.setData( '[<paragraph>Foo</paragraph>]' );
 
@@ -845,8 +845,6 @@ describe( 'transform', () => {
 
 				syncClients();
 
-				// Actual content:
-				// <paragraph></paragraph><paragraph>Ba</paragraph><paragraph></paragraph>
 				expectClients( '<paragraph>Foo</paragraph><paragraph>BarAbc</paragraph>' );
 			} );
 		} );

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

@@ -183,7 +183,6 @@ describe( 'transform', () => {
 				kate.remove();
 
 				syncClients();
-
 				expectClients( '<paragraph></paragraph>' );
 
 				john.undo();
@@ -191,8 +190,7 @@ describe( 'transform', () => {
 
 				syncClients();
 
-				// Actual result for Kate:
-				// <paragraph><m1:start></m1:start>Fo<m1:end></m1:end><m2:start></m2:start>o<m2:end></m2:end> Bar</paragraph>
+				// Wrong markers transforming.
 				expectClients(
 					'<paragraph>' +
 						'<m1:start></m1:start>Foo<m1:end></m1:end><m2:start></m2:start> Bar<m2:end></m2:end>' +
@@ -789,6 +787,41 @@ describe( 'transform', () => {
 					'</blockQuote>'
 				);
 			} );
+
+			it( 'only marker end is inside merged element #1', () => {
+				john.setData( '<paragraph>Foo</paragraph>[<paragraph>B]ar</paragraph>' );
+				kate.setData( '<paragraph>Foo</paragraph>[]<paragraph>Bar</paragraph>' );
+
+				john.setMarker( 'm1' );
+				kate.merge();
+
+				syncClients();
+
+				expectClients( '<paragraph>Foo<m1:start></m1:start>B<m1:end></m1:end>ar</paragraph>' );
+			} );
+
+			it( 'only marker end is inside merged element #2', () => {
+				john.setData( '<paragraph>Foo[]Bar</paragraph>' );
+				kate.setData( '<paragraph>Foo[]Bar</paragraph>' );
+
+				kate.split();
+
+				syncClients();
+				expectClients( '<paragraph>Foo</paragraph><paragraph>Bar</paragraph>' );
+
+				john.setSelection( [ 1 ] );
+				john.insert( '<paragraph>Xyz</paragraph>' );
+
+				syncClients();
+				expectClients( '<paragraph>Foo</paragraph><paragraph>Xyz</paragraph><paragraph>Bar</paragraph>' );
+
+				john.setSelection( [ 1 ], [ 2, 1 ] );
+				john.setMarker( 'm1' );
+				kate.undo();
+
+				syncClients();
+				expectClients( '<paragraph>Foo<m1:start></m1:start>Bar</paragraph><paragraph>Xyz</paragraph><m1:end></m1:end>' );
+			} );
 		} );
 
 		describe( 'by rename', () => {

+ 2 - 6
packages/ckeditor5-engine/tests/model/operation/transform/move.js

@@ -335,7 +335,7 @@ describe( 'transform', () => {
 				);
 			} );
 
-			it.skip( 'text in other user\'s selection', () => {
+			it( 'text in other user\'s selection', () => {
 				john.setData( '<paragraph>[Foo] Bar</paragraph>' );
 				kate.setData( '<paragraph>F[]oo Bar</paragraph>' );
 
@@ -366,7 +366,7 @@ describe( 'transform', () => {
 				);
 			} );
 
-			it.skip( 'inside moved text', () => {
+			it( 'inside moved text', () => {
 				john.setData( '<paragraph>F[oo Ba]r</paragraph><paragraph>Abc</paragraph>' );
 				kate.setData( '<paragraph>Foo[] Bar</paragraph><paragraph>Abc</paragraph>' );
 
@@ -375,10 +375,6 @@ describe( 'transform', () => {
 
 				syncClients();
 
-				// Actual result for Kate:
-				// <paragraph>F</paragraph><paragraph>r</paragraph><paragraph> BaooAbc</paragraph>
-				// Move operations have wrong targets when they target at same place.
-
 				expectClients(
 					'<paragraph>F</paragraph>' +
 					'<paragraph>r</paragraph>' +

+ 21 - 12
packages/ckeditor5-engine/tests/model/operation/transform/remove.js

@@ -158,13 +158,30 @@ describe( 'transform', () => {
 				kate.wrap( 'blockQuote' );
 
 				syncClients();
+				expectClients( '<paragraph>Foo</paragraph>' );
 
 				john.undo();
 
 				syncClients();
 
-				// Actual content:
-				// <paragraph>Foo</paragraph><paragraph>Bar</paragraph><blockQuote></blockQuote>
+				expectClients(
+					'<paragraph>Foo</paragraph>' +
+					'<blockQuote>' +
+						'<paragraph>Bar</paragraph>' +
+					'</blockQuote>'
+				);
+			} );
+
+			it( 'element while removing, then undo #2', () => {
+				john.setData( '<paragraph>Foo</paragraph>[<paragraph>Bar</paragraph>]' );
+				kate.setData( '<paragraph>Foo</paragraph>[<paragraph>Bar</paragraph>]' );
+
+				john.remove();
+				john.undo();
+				kate.wrap( 'blockQuote' );
+
+				syncClients();
+
 				expectClients(
 					'<paragraph>Foo</paragraph>' +
 					'<blockQuote>' +
@@ -218,7 +235,7 @@ describe( 'transform', () => {
 				);
 			} );
 
-			it.skip( 'element while removing, then undo', () => {
+			it( 'element while removing, then undo', () => {
 				john.setData( '<paragraph>Foo</paragraph><blockQuote>[<paragraph>Bar</paragraph>]</blockQuote>' );
 				kate.setData( '<paragraph>Foo</paragraph><blockQuote>[]<paragraph>Bar</paragraph></blockQuote>' );
 
@@ -226,15 +243,11 @@ describe( 'transform', () => {
 				kate.unwrap();
 
 				syncClients();
-
 				expectClients( '<paragraph>Foo</paragraph>' );
 
 				john.undo();
 
 				syncClients();
-
-				// Actual result:
-				// <paragraph>Foo</paragraph><blockQuote></blockQuote>
 				expectClients(
 					'<paragraph>Foo</paragraph>' +
 					'<paragraph>Bar</paragraph>'
@@ -443,7 +456,7 @@ describe( 'transform', () => {
 				);
 			} );
 
-			it.skip( 'element into paragraph, then undo', () => {
+			it( 'element into paragraph, then undo', () => {
 				john.setData( '<paragraph>F[oo]</paragraph><paragraph>Bar</paragraph>' );
 				kate.setData( '<paragraph>Foo</paragraph>[]<paragraph>Bar</paragraph>' );
 
@@ -451,15 +464,11 @@ describe( 'transform', () => {
 				kate.merge();
 
 				syncClients();
-
 				expectClients( '<paragraph>FBar</paragraph>' );
 
 				john.undo();
 
 				syncClients();
-
-				// Actual result:
-				// <paragraph>F<paragraph></paragraph>oBar</paragraph>
 				expectClients( '<paragraph>FooBar</paragraph>' );
 			} );
 

+ 12 - 15
packages/ckeditor5-engine/tests/model/operation/transform/split.js

@@ -90,7 +90,7 @@ describe( 'transform', () => {
 				);
 			} );
 
-			it.skip( 'intersecting wrap', () => {
+			it( 'intersecting wrap', () => {
 				john.setData( '<paragraph>Fo[]o</paragraph>' );
 				kate.setData( '<paragraph>F[oo]</paragraph>' );
 
@@ -98,10 +98,9 @@ describe( 'transform', () => {
 				kate.wrap( 'div' );
 
 				syncClients();
-
 				expectClients(
-					'<paragraph>F</paragraph>' +
-					'<paragraph><div>oo</div></paragraph>'
+					'<paragraph>Fo</paragraph>' +
+					'<paragraph>o</paragraph>'
 				);
 			} );
 		} );
@@ -122,7 +121,7 @@ describe( 'transform', () => {
 				);
 			} );
 
-			it.skip( 'element in same position', () => {
+			it( 'element in same position', () => {
 				john.setData( '<blockQuote><paragraph>[]Foo</paragraph></blockQuote>' );
 				kate.setData( '<blockQuote><paragraph>[]Foo</paragraph></blockQuote>' );
 
@@ -130,7 +129,6 @@ describe( 'transform', () => {
 				kate.unwrap();
 
 				syncClients();
-
 				expectClients( '<blockQuote>Foo</blockQuote>' );
 			} );
 
@@ -285,7 +283,7 @@ describe( 'transform', () => {
 				expectClients( '<paragraph>FooBar</paragraph>' );
 			} );
 
-			it.skip( 'element into paragraph #3', () => {
+			it( 'element into paragraph #3', () => {
 				john.setData( '<paragraph>Foo[]</paragraph><paragraph>Bar</paragraph>' );
 				kate.setData( '<paragraph>Foo</paragraph>[]<paragraph>Bar</paragraph>' );
 
@@ -299,13 +297,10 @@ describe( 'transform', () => {
 				kate.undo();
 
 				syncClients();
-				// Actual content for Kate:
-				// <paragraph>FooBar</paragraph><paragraph></paragraph>
-				// There is a problem in Merge x Split transform in undo case.
 				expectClients( '<paragraph>Foo</paragraph><paragraph>Bar</paragraph>' );
 			} );
 
-			it.skip( 'element into paragraph #4', () => {
+			it( 'element into paragraph #4', () => {
 				john.setData( '<paragraph>Foo</paragraph><paragraph>B[]ar</paragraph>' );
 				kate.setData( '<paragraph>Foo</paragraph>[]<paragraph>Bar</paragraph>' );
 
@@ -313,14 +308,16 @@ describe( 'transform', () => {
 				kate.merge();
 
 				syncClients();
+				expectClients(
+					'<paragraph>FooB</paragraph>' +
+					'<paragraph>ar</paragraph>'
+				);
 
 				john.undo();
 				kate.undo();
 
-				expectClients(
-					'<paragraph>Fo</paragraph>' +
-					'<paragraph>oBar</paragraph>'
-				);
+				syncClients();
+				expectClients( '<paragraph>Foo</paragraph><paragraph>Bar</paragraph>' );
 			} );
 		} );
 

+ 1 - 4
packages/ckeditor5-engine/tests/model/operation/transform/unwrap.js

@@ -50,7 +50,7 @@ describe( 'transform', () => {
 				expectClients( '<paragraph>Foo</paragraph>' );
 			} );
 
-			it.skip( 'the same element, then undo', () => {
+			it( 'the same element, then undo', () => {
 				john.setData( '<blockQuote>[]<paragraph>Foo</paragraph></blockQuote>' );
 				kate.setData( '<blockQuote>[]<paragraph>Foo</paragraph></blockQuote>' );
 
@@ -63,9 +63,6 @@ describe( 'transform', () => {
 				john.undo();
 
 				syncClients();
-				// Actual content for Kate:
-				// <paragraph><paragraph>Foo</paragraph></paragraph>
-				// Kate has a different order of nodes in graveyard after syncing.
 				expectClients( '<blockQuote><paragraph>Foo</paragraph></blockQuote>' );
 			} );
 		} );

+ 31 - 13
packages/ckeditor5-engine/tests/model/operation/transform/utils.js

@@ -7,7 +7,7 @@ import UndoEditing from '@ckeditor/ckeditor5-undo/src/undoediting';
 import BlockQuoteEditing from '@ckeditor/ckeditor5-block-quote/src/blockquoteediting';
 
 import { getData, parse } from '../../../../src/dev-utils/model';
-import transform from '../../../../src/model/operation/transform';
+import { transformSets } from '../../../../src/model/operation/transform';
 import Position from '../../../../src/model/position';
 import Range from '../../../../src/model/range';
 import OperationFactory from '../../../../src/model/operation/operationfactory';
@@ -253,16 +253,34 @@ function bufferOperations( operations, client ) {
 }
 
 export function syncClients() {
-	for ( const client of clients ) {
-		for ( const item of bufferedOperations ) {
-			const remoteOperations = item.operations.map( op => OperationFactory.fromJSON( JSON.parse( op ), client.document ) );
-			const remoteClient = item.client;
+	const clientsOperations = {};
+
+	// For each client, flatten all buffered operations into one set.
+	for ( const item of bufferedOperations ) {
+		const name = item.client.name;
+
+		if ( !clientsOperations[ name ] ) {
+			clientsOperations[ name ] = [];
+		}
+
+		clientsOperations[ name ].push( ...item.operations );
+	}
 
-			if ( remoteClient == client ) {
+	for ( const localClient of clients ) {
+		for ( const remoteClient of clients ) {
+			if ( remoteClient == localClient ) {
 				continue;
 			}
 
-			const clientOperations = Array.from( client.document.history.getOperations( client.syncedVersion ) );
+			const remoteOperationsJson = clientsOperations[ remoteClient.name ];
+
+			if ( !remoteOperationsJson ) {
+				continue;
+			}
+
+			const remoteOperations = remoteOperationsJson.map( op => OperationFactory.fromJSON( JSON.parse( op ), localClient.document ) );
+
+			const localOperations = Array.from( localClient.document.history.getOperations( localClient.syncedVersion ) );
 
 			let remoteOperationsTransformed = null;
 
@@ -271,21 +289,21 @@ export function syncClients() {
 				padWithNoOps: true
 			};
 
-			if ( client.orderNumber < remoteClient.orderNumber ) {
-				remoteOperationsTransformed = transform.transformSets( clientOperations, remoteOperations, options ).operationsB;
+			if ( localClient.orderNumber < remoteClient.orderNumber ) {
+				remoteOperationsTransformed = transformSets( localOperations, remoteOperations, options ).operationsB;
 			} else {
-				remoteOperationsTransformed = transform.transformSets( remoteOperations, clientOperations, options ).operationsA;
+				remoteOperationsTransformed = transformSets( remoteOperations, localOperations, options ).operationsA;
 			}
 
-			client.editor.model.enqueueChange( 'transparent', writer => {
+			localClient.editor.model.enqueueChange( 'transparent', writer => {
 				for ( const operation of remoteOperationsTransformed ) {
 					writer.batch.addOperation( operation );
-					client.editor.model.applyOperation( operation );
+					localClient.editor.model.applyOperation( operation );
 				}
 			} );
 		}
 
-		client.syncedVersion = client.document.version;
+		localClient.syncedVersion = localClient.document.version;
 	}
 
 	bufferedOperations.clear();

+ 133 - 17
packages/ckeditor5-engine/tests/model/operation/transform/wrap.js

@@ -63,19 +63,24 @@ describe( 'transform', () => {
 				);
 			} );
 
-			it.skip( 'intersecting wrap #2', () => {
-				john.setData( '<paragraph>[Foo]</paragraph>' );
-				kate.setData( '<paragraph>F[o]o</paragraph>' );
+			it( 'intersecting wrap #2', () => {
+				john.setData( '[<paragraph>Foo</paragraph><paragraph>Bar</paragraph><paragraph>Abc</paragraph>]' );
+				kate.setData( '<paragraph>Foo</paragraph>[<paragraph>Bar</paragraph>]<paragraph>Abc</paragraph>' );
 
-				john.wrap( 'div' );
+				john.wrap( 'blockQuote' );
 				kate.wrap( 'div' );
 
 				syncClients();
-
-				expectClients( '<paragraph><div>Foo</div></pragraph>' );
+				expectClients(
+					'<blockQuote>' +
+						'<paragraph>Foo</paragraph>' +
+						'<paragraph>Bar</paragraph>' +
+						'<paragraph>Abc</paragraph>' +
+					'</blockQuote>'
+				);
 			} );
 
-			it.skip( 'intersecting wrap, then undo #1', () => {
+			it( 'intersecting wrap, then undo #1', () => {
 				john.setData( '[<paragraph>Foo</paragraph><paragraph>Bar</paragraph>]<paragraph>Abc</paragraph>' );
 				kate.setData( '<paragraph>Foo</paragraph>[<paragraph>Bar</paragraph><paragraph>Abc</paragraph>]' );
 
@@ -83,12 +88,20 @@ describe( 'transform', () => {
 				kate.wrap( 'div' );
 
 				syncClients();
+				expectClients(
+					'<blockQuote>' +
+						'<paragraph>Foo</paragraph>' +
+						'<paragraph>Bar</paragraph>' +
+					'</blockQuote>' +
+					'<div>' +
+						'<paragraph>Abc</paragraph>' +
+					'</div>'
+				);
 
 				john.undo();
 				kate.undo();
 
 				syncClients();
-
 				expectClients(
 					'<paragraph>Foo</paragraph>' +
 					'<paragraph>Bar</paragraph>' +
@@ -96,21 +109,30 @@ describe( 'transform', () => {
 				);
 			} );
 
-			it.skip( 'intersecting wrap, then undo #2', () => {
-				john.setData( '<paragraph>[Foo]</paragraph>' );
-				kate.setData( '<paragraph>F[o]o</paragraph>' );
+			it( 'intersecting wrap, then undo #2', () => {
+				john.setData( '[<paragraph>Foo</paragraph><paragraph>Bar</paragraph>]<paragraph>Abc</paragraph>' );
+				kate.setData( '<paragraph>Foo</paragraph>[<paragraph>Bar</paragraph><paragraph>Abc</paragraph>]' );
 
-				john.wrap( 'div' );
+				john.wrap( 'blockQuote' );
 				kate.wrap( 'div' );
 
 				syncClients();
+				expectClients(
+					'<blockQuote>' +
+						'<paragraph>Foo</paragraph>' +
+						'<paragraph>Bar</paragraph>' +
+					'</blockQuote>' +
+					'<div>' +
+						'<paragraph>Abc</paragraph>' +
+					'</div>'
+				);
 
 				john.undo();
 				kate.undo();
 
 				syncClients();
 
-				expectClients( '<paragraph><div>Foo</div></pragraph>' );
+				expectClients( '<paragraph>Foo</paragraph><paragraph>Bar</paragraph><paragraph>Abc</paragraph>' );
 			} );
 
 			it( 'element and text', () => {
@@ -189,6 +211,102 @@ describe( 'transform', () => {
 			} );
 		} );
 
+		describe( 'by remove', () => {
+			it( 'remove the only wrapped element', () => {
+				john.setData( '[<paragraph>Foo</paragraph>]<paragraph>Bar</paragraph>' );
+				kate.setData( '[<paragraph>Foo</paragraph>]<paragraph>Bar</paragraph>' );
+
+				john.wrap( 'blockQuote' );
+				kate.remove();
+
+				syncClients();
+
+				expectClients( '<paragraph>Bar</paragraph>' );
+			} );
+
+			it( 'remove one of two wrapped elements', () => {
+				john.setData( '[<paragraph>Foo</paragraph><paragraph>Bar</paragraph>]' );
+				kate.setData( '[<paragraph>Foo</paragraph>]<paragraph>Bar</paragraph>' );
+
+				john.wrap( 'blockQuote' );
+				kate.remove();
+
+				syncClients();
+
+				expectClients( '<blockQuote><paragraph>Bar</paragraph></blockQuote>' );
+			} );
+
+			it( 'remove all wrapped elements', () => {
+				john.setData( '[<paragraph>Foo</paragraph><paragraph>Bar</paragraph>]<paragraph>Xyz</paragraph>' );
+				kate.setData( '[<paragraph>Foo</paragraph>]<paragraph>Bar</paragraph><paragraph>Xyz</paragraph>' );
+
+				john.wrap( 'blockQuote' );
+
+				kate.remove();
+				kate.setSelection( [ 0 ], [ 1 ] );
+				kate.remove();
+
+				syncClients();
+
+				expectClients( '<paragraph>Xyz</paragraph>' );
+			} );
+
+			it( 'remove the only wrapped element with undo', () => {
+				john.setData( '[<paragraph>Foo</paragraph>]<paragraph>Bar</paragraph>' );
+				kate.setData( '[<paragraph>Foo</paragraph>]<paragraph>Bar</paragraph>' );
+
+				john.wrap( 'blockQuote' );
+				kate.remove();
+
+				syncClients();
+				expectClients( '<paragraph>Bar</paragraph>' );
+
+				john.undo();
+				kate.undo();
+
+				syncClients();
+				expectClients( '<paragraph>Foo</paragraph><paragraph>Bar</paragraph>' );
+			} );
+
+			it( 'remove one of two wrapped elements with undo', () => {
+				john.setData( '[<paragraph>Foo</paragraph><paragraph>Bar</paragraph>]' );
+				kate.setData( '[<paragraph>Foo</paragraph>]<paragraph>Bar</paragraph>' );
+
+				john.wrap( 'blockQuote' );
+				kate.remove();
+
+				syncClients();
+				expectClients( '<blockQuote><paragraph>Bar</paragraph></blockQuote>' );
+
+				john.undo();
+				kate.undo();
+
+				syncClients();
+				expectClients( '<paragraph>Foo</paragraph><paragraph>Bar</paragraph>' );
+			} );
+
+			it( 'remove all wrapped elements with undo', () => {
+				john.setData( '[<paragraph>Foo</paragraph><paragraph>Bar</paragraph>]<paragraph>Xyz</paragraph>' );
+				kate.setData( '[<paragraph>Foo</paragraph>]<paragraph>Bar</paragraph><paragraph>Xyz</paragraph>' );
+
+				john.wrap( 'blockQuote' );
+
+				kate.remove();
+				kate.setSelection( [ 0 ], [ 1 ] );
+				kate.remove();
+
+				syncClients();
+				expectClients( '<paragraph>Xyz</paragraph>' );
+
+				john.undo();
+				kate.undo();
+				kate.undo();
+
+				syncClients();
+				expectClients( '<paragraph>Foo</paragraph><paragraph>Bar</paragraph><paragraph>Xyz</paragraph>' );
+			} );
+		} );
+
 		describe( 'by merge', () => {
 			it( 'element into paragraph #1', () => {
 				john.setData( '[<paragraph>Foo</paragraph>]<paragraph>Bar</paragraph>' );
@@ -219,7 +337,7 @@ describe( 'transform', () => {
 				expectClients( '<paragraph>Foo</paragraph><paragraph>Bar</paragraph>' );
 			} );
 
-			it.skip( 'element into paragraph, then undo', () => {
+			it( 'element into paragraph, then undo', () => {
 				john.setData( '<paragraph>Foo</paragraph>[<paragraph>Bar</paragraph>]' );
 				kate.setData( '<paragraph>Foo</paragraph>[]<paragraph>Bar</paragraph>' );
 
@@ -227,14 +345,12 @@ describe( 'transform', () => {
 				kate.merge();
 
 				syncClients();
-
-				expectClients( '<blockQuote><paragraph>FooBar</paragraph></blockQuote>' );
+				expectClients( '<paragraph>FooBar</paragraph>' );
 
 				john.undo();
 				kate.undo();
 
 				syncClients();
-
 				expectClients( '<paragraph>Foo</paragraph><paragraph>Bar</paragraph>' );
 			} );
 		} );

+ 4 - 5
packages/ckeditor5-engine/tests/model/range.js

@@ -12,7 +12,6 @@ import TreeWalker from '../../src/model/treewalker';
 import AttributeOperation from '../../src/model/operation/attributeoperation';
 import InsertOperation from '../../src/model/operation/insertoperation';
 import MoveOperation from '../../src/model/operation/moveoperation';
-import RemoveOperation from '../../src/model/operation/removeoperation';
 import RenameOperation from '../../src/model/operation/renameoperation';
 import MergeOperation from '../../src/model/operation/mergeoperation';
 import SplitOperation from '../../src/model/operation/splitoperation';
@@ -898,10 +897,10 @@ describe( 'Range', () => {
 			} );
 		} );
 
-		describe( 'by RemoveOperation', () => {
+		describe( 'by MoveOperation to graveyard', () => {
 			it( 'remove before range', () => {
 				const start = new Position( root, [ 0 ] );
-				const op = new RemoveOperation( start, 2, gyPos, 1 );
+				const op = new MoveOperation( start, 2, gyPos, 1 );
 
 				const transformed = range.getTransformedByOperation( op );
 
@@ -910,7 +909,7 @@ describe( 'Range', () => {
 
 			it( 'remove intersecting with range', () => {
 				const start = new Position( root, [ 4 ] );
-				const op = new RemoveOperation( start, 2, gyPos, 1 );
+				const op = new MoveOperation( start, 2, gyPos, 1 );
 
 				const transformed = range.getTransformedByOperation( op );
 
@@ -922,7 +921,7 @@ describe( 'Range', () => {
 			it( 'remove inside the range', () => {
 				const start = new Position( root, [ 3 ] );
 
-				const op = new RemoveOperation( start, 2, gyPos, 1 );
+				const op = new MoveOperation( start, 2, gyPos, 1 );
 
 				const transformed = range.getTransformedByOperation( op );
 

+ 102 - 70
packages/ckeditor5-engine/tests/model/schema.js

@@ -1106,8 +1106,7 @@ describe( 'Schema', () => {
 	} );
 
 	describe( 'getValidRanges()', () => {
-		const attribute = 'bold';
-		let model, doc, root, schema, ranges;
+		let model, doc, root, schema;
 
 		beforeEach( () => {
 			model = new Model();
@@ -1116,112 +1115,145 @@ describe( 'Schema', () => {
 			root = doc.createRoot();
 
 			schema.register( 'p', { inheritAllFrom: '$block' } );
-			schema.register( 'h1', { inheritAllFrom: '$block' } );
-			schema.register( 'img', {
-				allowWhere: '$text'
-			} );
+			schema.register( 'img', { allowWhere: '$text' } );
 
-			schema.addAttributeCheck( ( ctx, attributeName ) => {
-				// Allow 'bold' on p>$text.
-				if ( ctx.endsWith( 'p $text' ) && attributeName == 'bold' ) {
-					return true;
-				}
+			// This is a "hack" to allow setting any ranges in `setData` util.
+			schema.extend( '$text', { allowIn: '$root' } );
+		} );
 
-				// Allow 'bold' on $root>p.
-				if ( ctx.endsWith( '$root p' ) && attributeName == 'bold' ) {
-					return true;
-				}
-			} );
+		function testValidRangesForAttribute( input, attribute, output ) {
+			setData( model, input );
 
-			// Parse data string to model.
-			const parsedModel = parse( '<p>foo<img />bar</p>', model.schema, { context: [ root.name ] } );
+			const validRanges = schema.getValidRanges( doc.selection.getRanges(), attribute );
+			const sel = new Selection( validRanges );
 
-			// Set parsed model data to prevent selection post-fixer from running.
-			model.change( writer => {
-				writer.insert( parsedModel, root );
-			} );
+			expect( stringify( root, sel ) ).to.equal( output );
+		}
+
+		it( 'should return a range with p for an attribute allowed only on p', () => {
+			schema.extend( 'p', { allowAttributes: 'foo' } );
 
-			ranges = [ Range.createOn( root.getChild( 0 ) ) ];
+			testValidRangesForAttribute(
+				'[<p>foo<img></img>bar</p>]',
+				'foo',
+				'[<p>foo<img></img>bar</p>]'
+			);
 		} );
 
-		it( 'should return unmodified ranges when attribute is allowed on each item (text is not allowed in img)', () => {
-			schema.extend( 'img', { allowAttributes: 'bold' } );
+		it( 'should return ranges on text nodes for an attribute allowed only on text', () => {
+			schema.extend( '$text', { allowAttributes: 'bold' } );
 
-			expect( schema.getValidRanges( ranges, attribute ) ).to.deep.equal( ranges );
+			testValidRangesForAttribute(
+				'[<p>foo<img></img>bar</p>]',
+				'bold',
+				'<p>[foo]<img></img>[bar]</p>'
+			);
 		} );
 
-		it( 'should return unmodified ranges when attribute is allowed on each item (text is allowed in img)', () => {
-			schema.extend( 'img', { allowAttributes: 'bold' } );
-			schema.extend( '$text', { allowIn: 'img' } );
+		it( 'should return a range on img for an attribute allowed only on img', () => {
+			schema.extend( 'img', { allowAttributes: 'src' } );
 
-			expect( schema.getValidRanges( ranges, attribute ) ).to.deep.equal( ranges );
+			testValidRangesForAttribute(
+				'[<p>foo<img></img>bar</p>]',
+				'src',
+				'<p>foo[<img></img>]bar</p>'
+			);
 		} );
 
-		it( 'should return two ranges when attribute is not allowed on one item', () => {
+		it( 'should return a range containing all children for an attribute allowed on all children', () => {
+			schema.extend( '$text', { allowAttributes: 'bold' } );
 			schema.extend( 'img', { allowAttributes: 'bold' } );
-			schema.extend( '$text', { allowIn: 'img' } );
 
-			setData( model, '<p>[foo<img>xxx</img>bar]</p>' );
+			testValidRangesForAttribute(
+				'[<p>foo<img></img>bar</p>]',
+				'bold',
+				'<p>[foo<img></img>bar]</p>'
+			);
+		} );
 
-			const validRanges = schema.getValidRanges( doc.selection.getRanges(), attribute );
-			const sel = new Selection( validRanges );
+		it( 'should return a range with p and a range on all children for an attribute allowed on p and its children', () => {
+			schema.extend( 'p', { allowAttributes: 'foo' } );
+			schema.extend( '$text', { allowAttributes: 'foo' } );
+			schema.extend( 'img', { allowAttributes: 'foo' } );
 
-			expect( stringify( root, sel ) ).to.equal( '<p>[foo<img>]xxx[</img>bar]</p>' );
-		} );
+			setData( model, '[<p>foo<img></img>bar</p>]' );
 
-		it( 'should return three ranges when attribute is not allowed on one element but is allowed on its child', () => {
-			schema.extend( '$text', { allowIn: 'img' } );
+			const validRanges = Array.from( schema.getValidRanges( doc.selection.getRanges(), 'foo' ) );
 
-			schema.addAttributeCheck( ( ctx, attributeName ) => {
-				// Allow 'bold' on img>$text.
-				if ( ctx.endsWith( 'img $text' ) && attributeName == 'bold' ) {
-					return true;
-				}
-			} );
+			expect( validRanges.length ).to.equal( 2 );
 
-			setData( model, '<p>[foo<img>xxx</img>bar]</p>' );
+			expect( validRanges[ 0 ].start.path ).to.deep.equal( [ 0, 0 ] );
+			expect( validRanges[ 0 ].end.path ).to.deep.equal( [ 0, 7 ] );
 
-			const validRanges = schema.getValidRanges( doc.selection.getRanges(), attribute );
-			const sel = new Selection( validRanges );
+			expect( validRanges[ 1 ].start.path ).to.deep.equal( [ 0 ] );
+			expect( validRanges[ 1 ].end.path ).to.deep.equal( [ 1 ] );
+		} );
+
+		it( 'should not break a range if children are not allowed to have the attribute', () => {
+			schema.extend( 'p', { allowAttributes: 'foo' } );
 
-			expect( stringify( root, sel ) ).to.equal( '<p>[foo]<img>[xxx]</img>[bar]</p>' );
+			testValidRangesForAttribute(
+				'[<p>foo</p><p>bar</p>]',
+				'foo',
+				'[<p>foo</p><p>bar</p>]'
+			);
 		} );
 
-		it( 'should not leak beyond the given ranges', () => {
-			setData( model, '<p>[foo<img></img>bar]x[bar<img></img>foo]</p>' );
+		it( 'should search deeply', () => {
+			schema.extend( '$text', { allowAttributes: 'bold', allowIn: 'img' } );
 
-			const validRanges = schema.getValidRanges( doc.selection.getRanges(), attribute );
-			const sel = new Selection( validRanges );
+			testValidRangesForAttribute(
+				'[<p>foo<img>xxx</img>bar</p>]',
+				'bold',
+				'<p>[foo]<img>[xxx]</img>[bar]</p>'
+			);
+		} );
+
+		it( 'should work with multiple ranges', () => {
+			schema.extend( '$text', { allowAttributes: 'bold' } );
 
-			expect( stringify( root, sel ) ).to.equal( '<p>[foo]<img></img>[bar]x[bar]<img></img>[foo]</p>' );
+			testValidRangesForAttribute(
+				'[<p>a</p><p>b</p>]<p>c</p><p>[d]</p>',
+				'bold',
+				'<p>[a]</p><p>[b]</p><p>c</p><p>[d]</p>'
+			);
 		} );
 
-		it( 'should correctly handle a range which ends in a disallowed position', () => {
-			schema.extend( '$text', { allowIn: 'img' } );
+		it( 'should work with non-flat ranges', () => {
+			schema.extend( '$text', { allowAttributes: 'bold' } );
 
-			setData( model, '<p>[foo<img>bar]</img>bom</p>' );
+			testValidRangesForAttribute(
+				'[<p>a</p><p>b</p><p>c]</p><p>d</p>',
+				'bold',
+				'<p>[a]</p><p>[b]</p><p>[c]</p><p>d</p>'
+			);
+		} );
 
-			const validRanges = schema.getValidRanges( doc.selection.getRanges(), attribute );
-			const sel = new Selection( validRanges );
+		it( 'should not leak beyond the given ranges', () => {
+			schema.extend( '$text', { allowAttributes: 'bold' } );
 
-			expect( stringify( root, sel ) ).to.equal( '<p>[foo]<img>bar</img>bom</p>' );
+			testValidRangesForAttribute(
+				'[<p>foo</p><p>b]a[r</p><p>x]yz</p>',
+				'bold',
+				'<p>[foo]</p><p>[b]a[r]</p><p>[x]yz</p>'
+			);
 		} );
 
-		it( 'should split range into two ranges and omit disallowed element', () => {
+		it( 'should correctly handle a range which ends in a disallowed position', () => {
+			schema.extend( '$text', { allowAttributes: 'bold', allowIn: 'img' } );
+
+			// Disallow bold on text inside image.
 			schema.addAttributeCheck( ( ctx, attributeName ) => {
-				// Disallow 'bold' on p>img.
-				if ( ctx.endsWith( 'p img' ) && attributeName == 'bold' ) {
+				if ( ctx.endsWith( 'img $text' ) && attributeName == 'bold' ) {
 					return false;
 				}
 			} );
 
-			const result = schema.getValidRanges( ranges, attribute );
-
-			expect( result ).to.length( 2 );
-			expect( result[ 0 ].start.path ).to.members( [ 0 ] );
-			expect( result[ 0 ].end.path ).to.members( [ 0, 3 ] );
-			expect( result[ 1 ].start.path ).to.members( [ 0, 4 ] );
-			expect( result[ 1 ].end.path ).to.members( [ 1 ] );
+			testValidRangesForAttribute(
+				'[<p>foo<img>xx]x</img>bar</p>',
+				'bold',
+				'<p>[foo]<img>xxx</img>bar</p>'
+			);
 		} );
 	} );
 

+ 1 - 1
packages/ckeditor5-engine/tests/model/selection.js

@@ -633,7 +633,7 @@ describe( 'Selection', () => {
 		it( 'should throw an error when range is invalid', () => {
 			expect( () => {
 				selection._setRanges( [ { invalid: 'range' } ] );
-			} ).to.throw( CKEditorError, /model-selection-added-not-range/ );
+			} ).to.throw( CKEditorError, /model-selection-set-ranges-not-range/ );
 		} );
 
 		it( 'should remove all ranges and add given ranges', () => {

+ 1 - 1
packages/ckeditor5-engine/tests/model/writer.js

@@ -1545,7 +1545,7 @@ describe( 'Writer', () => {
 				expect( batch.operations.length ).to.equal( 2 );
 			} );
 
-			it( 'should use RemoveOperation', () => {
+			it( 'should use MoveOperation to graveyard', () => {
 				batch = new Batch();
 				remove( div );
 

+ 2 - 2
packages/ckeditor5-engine/tests/view/documentselection.js

@@ -113,7 +113,7 @@ describe( 'DocumentSelection', () => {
 			expect( () => {
 				// eslint-disable-next-line no-new
 				new DocumentSelection( [ { invalid: 'range' } ] );
-			} ).to.throw( CKEditorError, 'view-selection-invalid-range: Invalid Range.' );
+			} ).to.throw( CKEditorError, /view-selection-add-range-not-range/ );
 		} );
 
 		it( 'should throw an error when ranges intersects', () => {
@@ -1004,7 +1004,7 @@ describe( 'DocumentSelection', () => {
 			it( 'should throw an error when range is invalid', () => {
 				expect( () => {
 					documentSelection._setTo( [ { invalid: 'range' } ] );
-				} ).to.throw( CKEditorError, 'view-selection-invalid-range: Invalid Range.' );
+				} ).to.throw( CKEditorError, /view-selection-add-range-not-range/ );
 			} );
 
 			it( 'should throw when range is intersecting with already added range', () => {

+ 2 - 2
packages/ckeditor5-engine/tests/view/selection.js

@@ -113,7 +113,7 @@ describe( 'Selection', () => {
 			expect( () => {
 				// eslint-disable-next-line no-new
 				new Selection( [ { invalid: 'range' } ] );
-			} ).to.throw( CKEditorError, 'view-selection-invalid-range: Invalid Range.' );
+			} ).to.throw( CKEditorError, /view-selection-add-range-not-range/ );
 		} );
 
 		it( 'should throw an error when ranges intersects', () => {
@@ -878,7 +878,7 @@ describe( 'Selection', () => {
 			it( 'should throw an error when range is invalid', () => {
 				expect( () => {
 					selection.setTo( [ { invalid: 'range' } ] );
-				} ).to.throw( CKEditorError, 'view-selection-invalid-range: Invalid Range.' );
+				} ).to.throw( CKEditorError, /view-selection-add-range-not-range/ );
 			} );
 
 			it( 'should throw when range is intersecting with already added range', () => {

Some files were not shown because too many files changed in this diff