Explorar el Código

Merge branch 'master' into t/1555

# Conflicts:
#	src/model/operation/transform.js
Maciej Gołaszewski hace 7 años
padre
commit
e2cec333d0

+ 3 - 0
packages/ckeditor5-engine/.travis.yml

@@ -12,6 +12,9 @@ node_js:
 - '8'
 cache:
 - node_modules
+branches:
+   except:
+   - stable
 before_install:
 - export DISPLAY=:99.0
 - sh -e /etc/init.d/xvfb start

+ 1 - 1
packages/ckeditor5-engine/CONTRIBUTING.md

@@ -1,4 +1,4 @@
 Contributing
 ========================================
 
-Information about contributing can be found at the following page: <https://github.com/ckeditor/ckeditor5/blob/master/CONTRIBUTING.md>.
+See the [official contributors' guide to CKEditor 5](https://ckeditor.com/docs/ckeditor5/latest/framework/guides/contributing/contributing.html) to learn more.

+ 32 - 34
packages/ckeditor5-engine/docs/framework/guides/deep-dive/schema.md

@@ -4,11 +4,11 @@ category: framework-deep-dive
 
 # Schema
 
-In this article we assume that you have already read the {@link framework/guides/architecture/editing-engine#schema "Schema"} section of the {@link framework/guides/architecture/editing-engine Introduction to the "Editing engine architecture"}.
+This article assumes that you have already read the {@link framework/guides/architecture/editing-engine#schema "Schema"} section of the {@link framework/guides/architecture/editing-engine introduction to the editing engine architecture}.
 
 ## Quick recap
 
-Editor's schema is available in {@link module:engine/model/model~Model#schema `editor.model.schema`} property. It defines allowed model structures (how model elements can be nested) and allowed attributes (of both, elements and text nodes). This information is later used by editing features and the editing engine to make decision on how to process the model, where to enable features, etc.
+The editor's schema is available in the {@link module:engine/model/model~Model#schema `editor.model.schema`} property. It defines allowed model structures (how model elements can be nested) and allowed attributes (of both elements and text nodes). This information is later used by editing features and the editing engine to decide how to process the model, where to enable features, etc.
 
 Schema rules can be defined by using the {@link module:engine/model/schema~Schema#register `Schema#register()`} or {@link module:engine/model/schema~Schema#extend `Schema#extend()`} methods. The former can be used only once for a given item name which ensures that only a single editing feature can introduce this item. Similarly, `extend()` can only be used for defined items.
 
@@ -16,9 +16,7 @@ Elements and attributes are checked by features separately by using the {@link m
 
 ## Defining allowed structures
 
-When a feature introduces a model element it should register it in the schema. Besides
-defining that such an element may exist in the model, the feature also needs to define where
-this element may be placed:
+When a feature introduces a model element, it should register it in the schema. Besides defining that such an element may exist in the model, the feature also needs to define where this element can be placed:
 
 ```js
 schema.register( 'myElement', {
@@ -26,7 +24,7 @@ schema.register( 'myElement', {
 } );
 ```
 
-This lets the schema know that `<myElement>` may be a child of the `<$root>` element. `$root` is one of the generic nodes defined by the editing framework. By default, the editor names the main root element a `<$root>`, so the above definition allows `<myElement>` in the main editor element.
+This lets the schema know that `<myElement>` can be a child of `<$root>`. The `$root` element is one of the generic nodes defined by the editing framework. By default, the editor names the main root element a `<$root>`, so the above definition allows `<myElement>` in the main editor element.
 
 In other words, this would be correct:
 
@@ -36,7 +34,7 @@ In other words, this would be correct:
 </$root>
 ```
 
-While this would not be correct:
+While this would be incorrect:
 
 ```js
 <$root>
@@ -85,11 +83,11 @@ schema.register( 'paragraph', {
 Which can be read as:
 
 * The `<paragraph>` element will be allowed in elements in which `<$block>` is allowed (e.g. in `<$root>`).
-* The `<paragraph>` element will allow all nodes which are allowed in `<$block>` (e.g. `$text`).
-* The `<paragraph>` element will allow all attributes allowed on `<$block>`.
+* The `<paragraph>` element will allow all nodes that are allowed in `<$block>` (e.g. `$text`).
+* The `<paragraph>` element will allow all attributes allowed in `<$block>`.
 * The `<paragraph>` element will inherit all `is*` properties of `<$block>` (e.g. `isBlock`).
 
-Thanks to the fact that `<paragraph>`'s definition is inherited from `<$block>` other features can use the `<$block>` type to indirectly extend `<paragraph>`'s definition. For example, the {@link module:block-quote/blockquote~BlockQuote} feature does this:
+Thanks to the fact that the `<paragraph>` definition is inherited from `<$block>` other features can use the `<$block>` type to indirectly extend the `<paragraph>` definition. For example, the {@link module:block-quote/blockquote~BlockQuote} feature does this:
 
 ```js
 schema.register( 'blockQuote', {
@@ -98,26 +96,26 @@ schema.register( 'blockQuote', {
 } );
 ```
 
-Thanks to that, despite the fact that block quote and paragraph features know nothing about themselves, paragraphs will be allowed in block quotes and block quotes will be allowed in all places where blocks are allowed. So if anyone will register a `<section>` element (with `allowContentOf: '$root'` rule), that `<section>` elements will allow block quotes too.
+Thanks to that, despite the fact that block quote and paragraph features know nothing about themselves, paragraphs will be allowed in block quotes and block quotes will be allowed in all places where blocks are allowed. So if anyone registers a `<section>` element (with the `allowContentOf: '$root'` rule), that `<section>` elements will allow block quotes, too.
 
 The side effect of such a definition inheritance is that now `<blockQuote>` is allowed in `<blockQuote>` which needs to be resolved by a callback which will disallow this specific structure.
 
 <info-box>
-	You can read more about the format of an item definition in {@link module:engine/model/schema~SchemaItemDefinition}.
+	You can read more about the format of the item definition in {@link module:engine/model/schema~SchemaItemDefinition}.
 </info-box>
 
-## Defining advanced rules in `checkChild()`'s callbacks
+## Defining advanced rules in `checkChild()` callbacks
 
 The {@link module:engine/model/schema~Schema#checkChild `Schema#checkChild()`} method which is the base method used to check whether some element is allowed in a given structure is {@link module:utils/observablemixin~ObservableMixin#decorate a decorated method}. It means that you can add listeners to implement your specific rules which are not limited by the {@link module:engine/model/schema~SchemaItemDefinition declarative `SchemaItemDefinition` API}.
 
-Those listeners can be added either by listening directly to the {@link module:engine/model/schema~Schema#event:checkChild} event or by using the handy {@link module:engine/model/schema~Schema#addChildCheck `Schema#addChildCheck()`} method.
+These listeners can be added either by listening directly to the {@link module:engine/model/schema~Schema#event:checkChild} event or by using the handy {@link module:engine/model/schema~Schema#addChildCheck `Schema#addChildCheck()`} method.
 
 For instance, the block quote feature defines such a listener to disallow nested `<blockQuote>` structures:
 
 ```js
 schema.addChildCheck( context, childDefinition ) => {
-	// Note that context is automatically normalized to SchemaContext instance and
-	// child to its definition (SchemaCompiledItemDefinition).
+	// Note that the context is automatically normalized to a SchemaContext instance and
+	// the child to its definition (SchemaCompiledItemDefinition).
 
 	// If checkChild() is called with a context that ends with blockQuote and blockQuote as a child
 	// to check, make the checkChild() method return false.
@@ -126,14 +124,15 @@ schema.addChildCheck( context, childDefinition ) => {
 	}
 } );
 ```
-
+<!--
 ## Defining attributes
 
 TODO
+-->
 
 ## Implementing additional constraints
 
-Schema's capabilities are limited to simple (and atomic) {@link module:engine/model/schema~Schema#checkChild `Schema#checkChild()`} and {@link module:engine/model/schema~Schema#checkAttribute `Schema#checkAttribute()`} checks on purpose. One may imagine that schema should support defining more complex rules such as "element `<x>` must be always followed by `<y>`". While it is feasible to create an API which would enable feeding the schema with such definitions, it is unfortunately unrealistic to then expect that every editing feature will consider those rules when processing the model. It is also unrealistic to expect that it will be done automatically by the schema and the editing engine themselves.
+Schema's capabilities are limited to simple (and atomic) {@link module:engine/model/schema~Schema#checkChild `Schema#checkChild()`} and {@link module:engine/model/schema~Schema#checkAttribute `Schema#checkAttribute()`} checks on purpose. One may imagine that the schema should support defining more complex rules such as "element `<x>` must be always followed by `<y>`". While it is feasible to create an API that would enable feeding the schema with such definitions, it is unfortunately unrealistic to then expect that every editing feature will consider these rules when processing the model. It is also unrealistic to expect that it will be done automatically by the schema and the editing engine themselves.
 
 For instance, let's get back to the "element `<x>` must be always followed by `<y>`" rule and this initial content:
 
@@ -145,7 +144,7 @@ For instance, let's get back to the "element `<x>` must be always followed by `<
 </$root>
 ```
 
-Now, imagine that the user presses the "block quote" button. Usually it would wrap the two selected blocks (`<y>` and `<z>`) with a `<blockQuote>` element:
+Now imagine that the user presses the "Block quote" button. Normally it would wrap the two selected blocks (`<y>` and `<z>`) with a `<blockQuote>` element:
 
 ```xml
 <$root>
@@ -157,16 +156,15 @@ Now, imagine that the user presses the "block quote" button. Usually it would wr
 </$root>
 ```
 
-But it turns out that this creates an incorrect structure  `<x>` is not followed by `<y>` anymore.
+But it turns out that this creates an incorrect structure &mdash; `<x>` is not followed by `<y>` anymore.
 
 What should happen instead? There are at least 4 possible solutions: the block quote feature should not be applicable in such a context, someone should create a new `<y>` right after `<x>`, `<x>` should be moved inside `<blockQuote>` together with `<y>` or vice versa.
 
-While this is a relatively simple scenario (unlike most real-time collaboration scenarios),
-it turns out that it is already hard to say what should happen and who should react to fix this content.
+While this is a relatively simple scenario (unlike most real-time collaborative editing scenarios), it turns out that it is already hard to say what should happen and who should react to fix this content.
 
-Therefore, if your editor needs to implement such rules, you should do that through {@link module:engine/model/document~Document#registerPostFixer model's post-fixers} fixing incorrect content or actively prevent such situations (e.g. by disabling certain features). It means that those constraints will be defined specifically for your scenario by your code which makes their implementation much easier.
+Therefore, if your editor needs to implement such rules, you should do that through {@link module:engine/model/document~Document#registerPostFixer model's post-fixers} fixing incorrect content or actively prevent such situations (e.g. by disabling certain features). It means that these constraints will be defined specifically for your scenario by your code which makes their implementation much easier.
 
-To sum up, the answer to who and how should implement additional constraints is: your features or your editor through CKEditor 5's API.
+To sum up, the answer to who and how should implement additional constraints is: Your features or your editor through the CKEditor 5 API.
 
 ## Who checks the schema?
 
@@ -174,30 +172,30 @@ The CKEditor 5 API exposes many ways to work on (change) the model. It can be do
 
 ### Low-level APIs
 
-The lowest-level API is the writer (to be precise, there are also raw operations below, but they are used for very special cases only). It allows applying atomic changes to the content like inserting/removing/moving/splitting nodes, setting/removing an attribute, etc. It is important to know that the **writer does not prevent from applying changes which violates rules defined in the schema**.
+The lowest-level API is the writer (to be precise, there are also raw operations below, but they are used for very special cases only). It allows applying atomic changes to the content like inserting, removing, moving or splitting nodes, setting and removing an attribute, etc. It is important to know that the **writer does not prevent from applying changes that violate rules defined in the schema**.
 
-The reason for that is that when you implement a command or any other feature you may need to perform multiple operations to do all the necessary changes. The state in the meantime (between these atomic operations) may be incorrect. The writer must allow that.
+The reason for this is that when you implement a command or any other feature you may need to perform multiple operations to do all the necessary changes. The state in the meantime (between these atomic operations) may be incorrect. The writer must allow that.
 
-For instance, you need to move `<foo>` from `<$root>` to `<bar>` and (at the same time) rename it to `<oof>`. But the schema defines that `<oof>` is not allowed in `<$root>` and `<foo>` in `<bar>`. If the writer would check schema it would complain regardless of the order of `rename` and `move` operations.
+For instance, you need to move `<foo>` from `<$root>` to `<bar>` and (at the same time) rename it to `<oof>`. But the schema defines that `<oof>` is not allowed in `<$root>` and `<foo>` is disallowed in `<bar>`. If the writer checked the schema, it would complain regardless of the order of `rename` and `move` operations.
 
-You can argue that the engine could handle this by checking the schema at the end of a {@link module:engine/model/model~Model#change `Model#change()` block} (it works like a transaction  the state needs to be correct at the end of it). In fact, we [plan to strip disallowed attributes](https://github.com/ckeditor/ckeditor5-engine/issues/1228) at the end of that blocks.
+You can argue that the engine could handle this by checking the schema at the end of a {@link module:engine/model/model~Model#change `Model#change()` block} (it works like a transaction &mdash; the state needs to be correct at the end of it). In fact, we [plan to strip disallowed attributes](https://github.com/ckeditor/ckeditor5-engine/issues/1228) at the end of these blocks.
 
 There are problems, though:
 
-* How to fix the content after a transaction is committed? It is impossible to implement a reasonable heuristic that wouldn't break the content from the user's perspective.
-* The model can become invalid during collaborative changes. Operational Transformation, while implemented by us in a very rich form (with 11 types of operations instead of the base 3) ensures conflict resolution and eventual consistency, but not model's validity.
+* How to fix the content after a transaction is committed? It is impossible to implement a reasonable heuristic that would not break the content from the user perspective.
+* The model can become invalid during real-time collaborative changes. Operational Transformation, while implemented by us in a very rich form (with 11 types of operations instead of the base 3), ensures conflict resolution and eventual consistency, but not the model's validity.
 
-Therefore, we chose to handle such situations per case, using more expressive and flexible {@link module:engine/model/document~Document#registerPostFixer model's post-fixers}. Additionally, we moved the responsibility to check the schema to features. They can make a lot better decisions apriori, before doing changes. You can read more about this in ["Implementing additional constraints"](#implementing-additional-constraints) section above.
+Therefore, we chose to handle such situations on a case-by-case basis, using more expressive and flexible {@link module:engine/model/document~Document#registerPostFixer model's post-fixers}. Additionally, we moved the responsibility to check the schema to features. They can make a lot better decisions a priori, before doing changes. You can read more about this in the ["Implementing additional constraints"](#implementing-additional-constraints) section above.
 
 ### High-level APIs
 
 What about other, higher-level methods? **We recommend that all APIs built on top of the writer should check the schema.**
 
-For instance, the {@link module:engine/model/model~Model#insertContent `Model#insertContent()`} method will make sure that inserted nodes are allowed in the place of their insertion. It may also attempt splitting the insertion container (if allowed by the schema) if that will make the element to insert allowed, and so on.
+For instance, the {@link module:engine/model/model~Model#insertContent `Model#insertContent()`} method will make sure that inserted nodes are allowed in the place of their insertion. It may also attempt to split the insertion container (if allowed by the schema) if that will make the element to insert allowed, and so on.
 
-Similarly, commands, if implemented correctly, {@link module:core/command~Command#isEnabled get disabled} if they should not be executed in the current place.
+Similarly, commands &mdash; if implemented correctly &mdash; {@link module:core/command~Command#isEnabled get disabled} if they should not be executed in the current place.
 
-Finally, the schema plays a crucial role during the conversion from the view to the model (called also "upcasting"). During this process converters make decisions whether they can convert specific view elements or attributes to the given positions in the model. Thanks to that if you would try to load an incorrect data to the editor or when you paste a content copied from another website, the structure and attributes of these data get adjusted to the current schema rules.
+Finally, the schema plays a crucial role during the conversion from the view to the model (also called "upcasting"). During this process converters decide whether they can convert specific view elements or attributes to the given positions in the model. Thanks to that if you tried to load incorrect data to the editor or when you paste content copied from another website, the structure and attributes of the data get adjusted to the current schema rules.
 
 <info-box>
 	Some features may miss schema checks. If you happen to find such a scenario, do not hesitate to [report it to us](https://github.com/ckeditor/ckeditor5/issues).

+ 1 - 1
packages/ckeditor5-engine/src/model/differ.js

@@ -423,7 +423,7 @@ export default class Differ {
 			// If change happens at the same position...
 			if ( a.position.isEqual( b.position ) ) {
 				// Keep chronological order of operations.
-				return a.changeCount < b.changeCount ? -1 : 1;
+				return a.changeCount - b.changeCount;
 			}
 
 			// If positions differ, position "on the left" should be earlier in the result.

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

@@ -352,7 +352,6 @@ export default class Model {
 	 * @fires deleteContent
 	 * @param {module:engine/model/selection~Selection|module:engine/model/documentselection~DocumentSelection} selection
 	 * Selection of which the content should be deleted.
-	 * @param {module:engine/model/batch~Batch} batch Batch to which the operations will be added.
 	 * @param {Object} [options]
 	 * @param {Boolean} [options.leaveUnmerged=false] Whether to merge elements after removing the content of the selection.
 	 *

+ 92 - 20
packages/ckeditor5-engine/src/model/operation/transform.js

@@ -449,6 +449,8 @@ class ContextFactory {
 							this._setRelation( opA, opB, 'insertAtSource' );
 						} else if ( opA.targetPosition.isEqual( opB.deletionPosition ) ) {
 							this._setRelation( opA, opB, 'insertBetween' );
+						} else if ( opA.targetPosition.isAfter( opB.sourcePosition ) ) {
+							this._setRelation( opA, opB, 'moveTargetAfter' );
 						}
 
 						break;
@@ -503,6 +505,12 @@ class ContextFactory {
 
 						break;
 					}
+
+					case SplitOperation: {
+						if ( opA.sourcePosition.isEqual( opB.splitPosition ) ) {
+							this._setRelation( opA, opB, 'splitAtSource' );
+						}
+					}
 				}
 
 				break;
@@ -1157,7 +1165,10 @@ setTransformation( MergeOperation, MergeOperation, ( a, b, context ) => {
 	//
 	// If neither or both operations point to graveyard, then let `aIsStrong` decide.
 	//
-	if ( a.sourcePosition.isEqual( b.sourcePosition ) && !a.targetPosition.isEqual( b.targetPosition ) && !context.bWasUndone ) {
+	if (
+		a.sourcePosition.isEqual( b.sourcePosition ) && !a.targetPosition.isEqual( b.targetPosition ) &&
+		!context.bWasUndone && context.abRelation != 'splitAtSource'
+	) {
 		const aToGraveyard = a.targetPosition.root.rootName == '$graveyard';
 		const bToGraveyard = b.targetPosition.root.rootName == '$graveyard';
 
@@ -1336,7 +1347,7 @@ setTransformation( MergeOperation, SplitOperation, ( a, b, context ) => {
 	// In this scenario the merge operation is now transformed by the split which has undone the previous merge operation.
 	// So now we are fixing situation which was skipped in `MergeOperation` x `MergeOperation` case.
 	//
-	if ( a.sourcePosition.isEqual( b.splitPosition ) && context.abRelation == 'mergeSameElement' ) {
+	if ( a.sourcePosition.isEqual( b.splitPosition ) && ( context.abRelation == 'mergeSameElement' || a.sourcePosition.offset > 0 ) ) {
 		a.sourcePosition = Position._createAt( b.moveTargetPosition );
 		a.targetPosition = a.targetPosition._getTransformedBySplitOperation( b );
 
@@ -1394,9 +1405,9 @@ setTransformation( MoveOperation, MoveOperation, ( a, b, context ) => {
 	let insertBefore = !context.aIsStrong;
 
 	// If the relation is set, then use it to decide nodes order.
-	if ( context.abRelation == 'insertBefore' ) {
+	if ( context.abRelation == 'insertBefore' || context.baRelation == 'insertAfter' ) {
 		insertBefore = true;
-	} else if ( context.abRelation == 'insertAfter' ) {
+	} else if ( context.abRelation == 'insertAfter' || context.baRelation == 'insertBefore' ) {
 		insertBefore = false;
 	}
 
@@ -1568,7 +1579,7 @@ setTransformation( MoveOperation, SplitOperation, ( a, b, context ) => {
 	// Do not transform if target position is same as split insertion position and this split comes from undo.
 	// This should be done on relations but it is too much work for now as it would require relations working in collaboration.
 	// We need to make a decision how we will resolve such conflict and this is less harmful way.
-	if ( !a.targetPosition.isEqual( b.insertionPosition ) || !b.graveyardPosition ) {
+	if ( !a.targetPosition.isEqual( b.insertionPosition ) || !b.graveyardPosition || context.abRelation == 'moveTargetAfter' ) {
 		newTargetPosition = a.targetPosition._getTransformedBySplitOperation( b );
 	}
 
@@ -1658,12 +1669,21 @@ setTransformation( MoveOperation, SplitOperation, ( a, b, context ) => {
 	// The default case.
 	//
 	const transformed = moveRange._getTransformedBySplitOperation( b );
+	const ranges = [ transformed ];
 
-	a.sourcePosition = transformed.start;
-	a.howMany = transformed.end.offset - transformed.start.offset;
-	a.targetPosition = newTargetPosition;
+	// Case 5:
+	//
+	// Moved range contains graveyard element used by split operation. Add extra move operation to the result.
+	//
+	if ( b.graveyardPosition ) {
+		const movesGraveyardElement = moveRange.start.isEqual( b.graveyardPosition ) || moveRange.containsPosition( b.graveyardPosition );
 
-	return [ a ];
+		if ( a.howMany > 1 && movesGraveyardElement ) {
+			ranges.push( Range.createFromPositionAndShift( b.insertionPosition, 1 ) );
+		}
+	}
+
+	return _makeMoveOperationsFromRanges( ranges, newTargetPosition );
 } );
 
 setTransformation( MoveOperation, MergeOperation, ( a, b, context ) => {
@@ -1681,7 +1701,30 @@ setTransformation( MoveOperation, MergeOperation, ( a, b, context ) => {
 			// removed nodes might be unexpected. This means that in this scenario we will reverse merging and remove the element.
 			//
 			if ( !context.aWasUndone ) {
-				return [ b.getReversed(), a ];
+				const results = [];
+
+				let gyMoveSource = Position.createFromPosition( b.graveyardPosition );
+				let splitNodesMoveSource = Position.createFromPosition( b.targetPosition );
+
+				if ( a.howMany > 1 ) {
+					results.push( new MoveOperation( a.sourcePosition, a.howMany - 1, a.targetPosition, 0 ) );
+					gyMoveSource = gyMoveSource._getTransformedByInsertion( a.targetPosition, a.howMany - 1 );
+					splitNodesMoveSource = splitNodesMoveSource._getTransformedByMove( a.sourcePosition, a.targetPosition, a.howMany - 1 );
+				}
+
+				const gyMoveTarget = b.deletionPosition._getCombined( a.sourcePosition, a.targetPosition );
+				const gyMove = new MoveOperation( gyMoveSource, 1, gyMoveTarget, 0 );
+
+				const targetPositionPath = gyMove.getMovedRangeStart().path.slice();
+				targetPositionPath.push( 0 );
+
+				const splitNodesMoveTarget = new Position( gyMove.targetPosition.root, targetPositionPath );
+				const splitNodesMove = new MoveOperation( splitNodesMoveSource, b.howMany, splitNodesMoveTarget, 0 );
+
+				results.push( gyMove );
+				results.push( splitNodesMove );
+
+				return results;
 			}
 		} else {
 			// Case 2:
@@ -1909,11 +1952,32 @@ setTransformation( SplitOperation, MergeOperation, ( a, b, context ) => {
 } );
 
 setTransformation( SplitOperation, MoveOperation, ( a, b, context ) => {
+	const rangeToMove = Range._createFromPositionAndShift( b.sourcePosition, b.howMany );
+
 	if ( a.graveyardPosition ) {
+		// Case 1:
+		//
+		// Split operation graveyard node was moved. In this case move operation is stronger. Since graveyard element
+		// is already moved to the correct position, we need to only move the nodes after the split position.
+		// This will be done by `MoveOperation` instead of `SplitOperation`.
+		//
+		if ( rangeToMove.start.isEqual( a.graveyardPosition ) || rangeToMove.containsPosition( a.graveyardPosition ) ) {
+			const sourcePosition = a.splitPosition._getTransformedByMoveOperation( b );
+
+			const newParentPosition = a.graveyardPosition._getTransformedByMoveOperation( b );
+			const newTargetPath = newParentPosition.path.slice();
+			newTargetPath.push( 0 );
+
+			const newTargetPosition = new Position( newParentPosition.root, newTargetPath );
+			const moveOp = new MoveOperation( sourcePosition, a.howMany, newTargetPosition, 0 );
+
+			return [ moveOp ];
+		}
+
 		a.graveyardPosition = a.graveyardPosition._getTransformedByMoveOperation( b );
 	}
 
-	// Case 1:
+	// Case 2:
 	//
 	// If the split position is inside the moved range, we need to shift the split position to a proper place.
 	// The position cannot be moved together with moved range because that would result in splitting of an incorrect element.
@@ -1930,8 +1994,6 @@ setTransformation( SplitOperation, MoveOperation, ( a, b, context ) => {
 	// After split:
 	// <paragraph>A</paragraph><paragraph>d</paragraph><paragraph>Xbcyz</paragraph>
 	//
-	const rangeToMove = Range._createFromPositionAndShift( b.sourcePosition, b.howMany );
-
 	if ( a.splitPosition.hasSameParentAs( b.sourcePosition ) && rangeToMove.containsPosition( a.splitPosition ) ) {
 		const howManyRemoved = b.howMany - ( a.splitPosition.offset - b.sourcePosition.offset );
 		a.howMany -= howManyRemoved;
@@ -1946,7 +2008,7 @@ setTransformation( SplitOperation, MoveOperation, ( a, b, context ) => {
 		return [ a ];
 	}
 
-	// Case 2:
+	// Case 3:
 	//
 	// Split is at a position where nodes were moved.
 	//
@@ -1964,13 +2026,16 @@ setTransformation( SplitOperation, MoveOperation, ( a, b, context ) => {
 	}
 
 	// The default case.
+	// Don't change `howMany` if move operation does not really move anything.
 	//
-	if ( a.splitPosition.hasSameParentAs( b.sourcePosition ) && a.splitPosition.offset <= b.sourcePosition.offset ) {
-		a.howMany -= b.howMany;
-	}
+	if ( !b.sourcePosition.isEqual( b.targetPosition ) ) {
+		if ( a.splitPosition.hasSameParentAs( b.sourcePosition ) && a.splitPosition.offset <= b.sourcePosition.offset ) {
+			a.howMany -= b.howMany;
+		}
 
-	if ( a.splitPosition.hasSameParentAs( b.targetPosition ) && a.splitPosition.offset < b.targetPosition.offset ) {
-		a.howMany += b.howMany;
+		if ( a.splitPosition.hasSameParentAs( b.targetPosition ) && a.splitPosition.offset < b.targetPosition.offset ) {
+			a.howMany += b.howMany;
+		}
 	}
 
 	// Change position stickiness to force a correct transformation.
@@ -2137,7 +2202,14 @@ function _makeMoveOperationsFromRanges( ranges, targetPosition ) {
 	for ( let i = 0; i < ranges.length; i++ ) {
 		// Create new operation out of a range and target position.
 		const range = ranges[ i ];
-		const op = new MoveOperation( range.start, range.end.offset - range.start.offset, targetPosition, 0 );
+		const op = new MoveOperation(
+			range.start,
+			range.end.offset - range.start.offset,
+			// If the target is the end of the move range this operation doesn't really move anything.
+			// In this case, it is better for OT to use range start instead of range end.
+			targetPosition.isEqual( range.end ) ? range.start : targetPosition,
+			0
+		);
 
 		operations.push( op );
 

+ 8 - 4
packages/ckeditor5-engine/src/model/range.js

@@ -527,13 +527,17 @@ export default class Range {
 	 */
 	_getTransformedBySplitOperation( operation ) {
 		const start = this.start._getTransformedBySplitOperation( operation );
-
-		let end;
+		let end = this.end._getTransformedBySplitOperation( operation );
 
 		if ( this.end.isEqual( operation.insertionPosition ) ) {
 			end = this.end.getShiftedBy( 1 );
-		} else {
-			end = this.end._getTransformedBySplitOperation( operation );
+		}
+
+		// Below may happen when range contains graveyard element used by split operation.
+		if ( start.root != end.root ) {
+			// End position was next to the moved graveyard element and was moved with it.
+			// Fix it by using old `end` which has proper `root`.
+			end = this.end.getShiftedBy( -1 );
 		}
 
 		return new Range( start, end );

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

@@ -117,10 +117,8 @@ function mergeBranches( writer, startPos, endPos ) {
 		return;
 	}
 
-	// If one of the positions is a root, then there's nothing more to merge (at least in the current state of implementation).
-	// Theoretically in this case we could unwrap the <p>: <$root>x[]<p>{}y</p></$root>, but we don't need to support it yet
-	// so let's just abort.
-	if ( !startParent.parent || !endParent.parent ) {
+	// If one of the positions is a limit element, then there's nothing to merge because we don't want to cross the limit boundaries.
+	if ( writer.model.schema.isLimit( startParent ) || writer.model.schema.isLimit( endParent ) ) {
 		return;
 	}
 

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

@@ -204,7 +204,7 @@ export default class Writer {
 					markerRange.end._getCombined( rangeRootPosition, position )
 				);
 
-				this.addMarker( markerName, { range, usingOperation: true } );
+				this.addMarker( markerName, { range, usingOperation: true, affectsData: true } );
 			}
 		}
 	}

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

@@ -174,9 +174,7 @@ describe( 'transform', () => {
 				kate.undo();
 				syncClients();
 
-				expectClients(
-					'<paragraph>Foo</paragraph>'
-				);
+				expectClients( '<paragraph>Foo</paragraph>' );
 			} );
 		} );
 
@@ -222,6 +220,40 @@ describe( 'transform', () => {
 
 				expectClients( '<paragraph>FooBar</paragraph>' );
 			} );
+
+			it( 'remove merged element then undo #3', () => {
+				john.setData( '[<paragraph>A</paragraph><paragraph>B</paragraph>]<paragraph>C</paragraph>' );
+				kate.setData( '<paragraph>A</paragraph>[]<paragraph>B</paragraph><paragraph>C</paragraph>' );
+
+				kate.merge();
+				john.remove();
+
+				syncClients();
+				expectClients( '<paragraph>C</paragraph>' );
+
+				john.undo();
+				kate.undo();
+
+				syncClients();
+				expectClients( '<paragraph>A</paragraph><paragraph>B</paragraph><paragraph>C</paragraph>' );
+			} );
+
+			it( 'remove merged element then undo #4', () => {
+				john.setData( '<paragraph>A</paragraph>[<paragraph>B</paragraph><paragraph>C</paragraph>]' );
+				kate.setData( '<paragraph>A</paragraph>[]<paragraph>B</paragraph><paragraph>C</paragraph>' );
+
+				kate.merge();
+				john.remove();
+
+				syncClients();
+				expectClients( '<paragraph>A</paragraph>' );
+
+				john.undo();
+				kate.undo();
+
+				syncClients();
+				expectClients( '<paragraph>A</paragraph><paragraph>B</paragraph><paragraph>C</paragraph>' );
+			} );
 		} );
 
 		describe( 'by delete', () => {
@@ -409,5 +441,23 @@ describe( 'transform', () => {
 				);
 			} );
 		} );
+
+		describe( 'by split', () => {
+			it( 'merge element which got split (the element is in blockquote) and undo', () => {
+				john.setData( '<paragraph>Foo</paragraph><blockQuote><paragraph>[]Bar</paragraph></blockQuote>' );
+				kate.setData( '<paragraph>Foo</paragraph><blockQuote><paragraph>B[]ar</paragraph></blockQuote>' );
+
+				john._processExecute( 'delete' );
+				kate.split();
+
+				syncClients();
+				expectClients( '<paragraph>FooB</paragraph><paragraph>ar</paragraph>' );
+
+				john.undo();
+
+				syncClients();
+				expectClients( '<paragraph>Foo</paragraph><blockQuote><paragraph>B</paragraph><paragraph>ar</paragraph></blockQuote>' );
+			} );
+		} );
 	} );
 } );

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

@@ -238,7 +238,7 @@ describe( 'transform', () => {
 		expectClients( '<paragraph>Foo</paragraph><paragraph>Bar</paragraph>' );
 	} );
 
-	it.skip( 'delete split paragraphs', () => {
+	it( 'delete split paragraphs', () => {
 		john.setData( '<paragraph>Foo</paragraph><paragraph>B[]ar</paragraph>' );
 
 		john.split();
@@ -276,7 +276,7 @@ describe( 'transform', () => {
 		expectClients( '<paragraph>Foo</paragraph>' );
 	} );
 
-	it( 'undo pasting', () => {
+	it( 'pasting on collapsed selection undo and redo', () => {
 		john.setData( '<paragraph>Foo[]Bar</paragraph>' );
 
 		// Below simulates pasting.
@@ -297,8 +297,16 @@ describe( 'transform', () => {
 		expectClients( '<paragraph>Foo1</paragraph><paragraph>2Bar</paragraph>' );
 
 		john.undo();
+		expectClients( '<paragraph>FooBar</paragraph>' );
+
+		john.redo();
+		expectClients( '<paragraph>Foo1</paragraph><paragraph>2Bar</paragraph>' );
 
+		john.undo();
 		expectClients( '<paragraph>FooBar</paragraph>' );
+
+		john.redo();
+		expectClients( '<paragraph>Foo1</paragraph><paragraph>2Bar</paragraph>' );
 	} );
 
 	it( 'selection attribute setting: split, bold, merge, undo, undo, undo', () => {
@@ -321,4 +329,63 @@ describe( 'transform', () => {
 		john.undo();
 		expectClients( '<paragraph>Foo</paragraph><paragraph>Bar</paragraph>' );
 	} );
+
+	// https://github.com/ckeditor/ckeditor5/issues/1288
+	it( 'remove two groups of blocks then undo, undo', () => {
+		john.setData(
+			'<paragraph>X</paragraph><paragraph>A</paragraph><paragraph>B[</paragraph><paragraph>C</paragraph><paragraph>D]</paragraph>'
+		);
+
+		john.delete();
+		john.setSelection( [ 0, 1 ], [ 2, 1 ] );
+		john.delete();
+
+		expectClients( '<paragraph>X</paragraph>' );
+
+		john.undo();
+
+		expectClients( '<paragraph>X</paragraph><paragraph>A</paragraph><paragraph>B</paragraph>' );
+
+		john.undo();
+
+		expectClients(
+			'<paragraph>X</paragraph><paragraph>A</paragraph><paragraph>B</paragraph><paragraph>C</paragraph><paragraph>D</paragraph>'
+		);
+	} );
+
+	// https://github.com/ckeditor/ckeditor5/issues/1287 TC1
+	it( 'pasting on non-collapsed selection undo and redo', () => {
+		john.setData( '<paragraph>Fo[o</paragraph><paragraph>B]ar</paragraph>' );
+
+		// Below simulates pasting.
+		john.editor.model.change( () => {
+			john.editor.model.deleteContent( john.document.selection );
+
+			john.setSelection( [ 0, 2 ] );
+			john.split();
+
+			john.setSelection( [ 1 ] );
+			john.insert( '<paragraph>1</paragraph>' );
+
+			john.setSelection( [ 1 ] );
+			john.merge();
+
+			john.setSelection( [ 1 ] );
+			john.insert( '<paragraph>2</paragraph>' );
+
+			john.setSelection( [ 2 ] );
+			john.merge();
+		} );
+
+		expectClients( '<paragraph>Fo1</paragraph><paragraph>2ar</paragraph>' );
+
+		john.undo();
+		expectClients( '<paragraph>Foo</paragraph><paragraph>Bar</paragraph>' );
+
+		john.redo();
+		expectClients( '<paragraph>Fo1</paragraph><paragraph>2ar</paragraph>' );
+
+		john.undo();
+		expectClients( '<paragraph>Foo</paragraph><paragraph>Bar</paragraph>' );
+	} );
 } );

+ 58 - 0
packages/ckeditor5-engine/tests/model/utils/deletecontent.js

@@ -772,6 +772,10 @@ describe( 'DataController utils', () => {
 				schema.extend( '$block', { allowIn: 'blockLimit' } );
 
 				schema.register( 'paragraph', { inheritAllFrom: '$block' } );
+				schema.register( 'blockQuote', {
+					allowWhere: '$block',
+					allowContentOf: '$root'
+				} );
 			} );
 
 			test(
@@ -804,6 +808,60 @@ describe( 'DataController utils', () => {
 				'<blockLimit><paragraph>foo [bar</paragraph></blockLimit><blockLimit><paragraph>baz] qux</paragraph></blockLimit>',
 				'<blockLimit><paragraph>foo []</paragraph></blockLimit><blockLimit><paragraph> qux</paragraph></blockLimit>'
 			);
+
+			// See: https://github.com/ckeditor/ckeditor5/issues/1265.
+			it( 'should proper merge two elements which are inside limit element', () => {
+				setData( model,
+					'<blockLimit>' +
+						'<blockQuote>' +
+							'<paragraph>Foo</paragraph>' +
+						'</blockQuote>' +
+						'<paragraph>[]Bar</paragraph>' +
+					'</blockLimit>'
+				);
+
+				model.modifySelection( doc.selection, { direction: 'backward' } );
+				deleteContent( model, doc.selection );
+
+				expect( getData( model ) ).to.equal(
+					'<blockLimit>' +
+						'<blockQuote>' +
+							'<paragraph>Foo[]Bar</paragraph>' +
+						'</blockQuote>' +
+					'</blockLimit>' );
+			} );
+
+			it( 'should proper merge elements which are inside limit element (nested elements)', () => {
+				setData( model,
+					'<blockQuote>' +
+						'<blockLimit>' +
+							'<blockQuote>' +
+								'<paragraph>Foo.</paragraph>' +
+								'<blockQuote>' +
+									'<paragraph>Foo</paragraph>' +
+								'</blockQuote>' +
+							'</blockQuote>' +
+							'<paragraph>[]Bar</paragraph>' +
+						'</blockLimit>' +
+					'</blockQuote>'
+				);
+
+				model.modifySelection( doc.selection, { direction: 'backward' } );
+				deleteContent( model, doc.selection );
+
+				expect( getData( model ) ).to.equal(
+					'<blockQuote>' +
+						'<blockLimit>' +
+							'<blockQuote>' +
+								'<paragraph>Foo.</paragraph>' +
+								'<blockQuote>' +
+									'<paragraph>Foo[]Bar</paragraph>' +
+								'</blockQuote>' +
+							'</blockQuote>' +
+						'</blockLimit>' +
+					'</blockQuote>'
+				);
+			} );
 		} );
 
 		describe( 'should leave a paragraph if the entire content was selected', () => {

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

@@ -310,10 +310,13 @@ describe( 'Writer', () => {
 
 			expect( Array.from( model.markers ).length ).to.equal( 1 );
 
-			const range = model.markers.get( 'marker' ).getRange();
+			const modelMarker = model.markers.get( 'marker' );
+			const range = modelMarker.getRange();
 			expect( range.root ).to.equal( root );
 			expect( range.start.path ).to.deep.equal( [ 2, 1 ] );
 			expect( range.end.path ).to.deep.equal( [ 2, 5 ] );
+			expect( modelMarker.managedUsingOperations ).to.be.true;
+			expect( modelMarker.affectsData ).to.be.true;
 		} );
 
 		it( 'should throw when trying to use detached writer', () => {