Browse Source

Merge branch 'master' into t/ckeditor5/1243

Maciej Gołaszewski 7 năm trước cách đây
mục cha
commit
9df0bb8929

+ 206 - 0
packages/ckeditor5-engine/docs/framework/guides/deep-dive/schema.md

@@ -0,0 +1,206 @@
+---
+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"}.
+
+## 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.
+
+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.
+
+Elements and attributes are checked by features separately by using the {@link module:engine/model/schema~Schema#checkChild `Schema#checkChild()`} and {@link module:engine/model/schema~Schema#checkAttribute `Schema#checkAttribute()`} methods.
+
+## 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:
+
+```js
+schema.register( 'myElement', {
+	allowIn: '$root'
+} );
+```
+
+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.
+
+In other words, this would be correct:
+
+```xml
+<$root>
+	<myElement></myElement>
+</$root>
+```
+
+While this would not be correct:
+
+```js
+<$root>
+	<foo>
+		<myElement></myElement>
+	</foo>
+</$root>
+```
+
+## Generic items
+
+There are three basic generic items: `$root`, `$block` and `$text`. They are defined as follows:
+
+```js
+schema.register( '$root', {
+	isLimit: true
+} );
+schema.register( '$block', {
+	allowIn: '$root',
+	isBlock: true
+} );
+schema.register( '$text', {
+	allowIn: '$block'
+} );
+```
+
+These definitions can then be reused by features to create their own definitions in a more extensible way. For example, the {@link module:paragraph/paragraph~Paragraph} feature will define its item as:
+
+```js
+schema.register( 'paragraph', {
+	inheritAllFrom: '$block'
+} );
+```
+
+Which translates to:
+
+```js
+schema.register( 'paragraph', {
+	allowWhere: '$block',
+	allowContentOf: '$block',
+	allowAttributesOf: '$block',
+	inheritTypesFrom: '$block'
+} );
+```
+
+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 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:
+
+```js
+schema.register( 'blockQuote', {
+	allowWhere: '$block',
+	allowContentOf: '$root'
+} );
+```
+
+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.
+
+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}.
+</info-box>
+
+## Defining advanced rules in `checkChild()`'s 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.
+
+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).
+
+	// If checkChild() is called with a context that ends with blockQuote and blockQuote as a child
+	// to check, make the checkChild() method return false.
+	if ( context.endsWith( 'blockQuote' ) && childDefinition.name == 'blockQuote' ) {
+		return false;
+	}
+} );
+```
+
+## 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.
+
+For instance, let's get back to the "element `<x>` must be always followed by `<y>`" rule and this initial content:
+
+```xml
+<$root>
+	<x>foo</x>
+	<y>bar[bom</y>
+	<z>bom]bar</z>
+</$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:
+
+```xml
+<$root>
+	<x>foo</x>
+	<blockQuote>
+		<y>bar[bom</y>
+		<z>bom]bar</z>
+	</blockQuote>
+</$root>
+```
+
+But it turns out that this creates an incorrect structure – `<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.
+
+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.
+
+To sum up, the answer to who and how should implement additional constraints is: your features or your editor through CKEditor 5's API.
+
+## Who checks the schema?
+
+The CKEditor 5 API exposes many ways to work on (change) the model. It can be done {@link framework/guides/architecture/editing-engine#changing-the-model through the writer}, via methods like {@link module:engine/model/model~Model#insertContent `Model#insertContent()`}, via commands and so on.
+
+### 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 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.
+
+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.
+
+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.
+
+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.
+
+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.
+
+### 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.
+
+Similarly, commands, if implemented correctly, {@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.
+
+<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).
+</info-box>
+
+

+ 98 - 31
packages/ckeditor5-engine/src/model/operation/transform.js

@@ -138,7 +138,8 @@ export function transform( a, b, context = {} ) {
  * @param {Array.<module:engine/model/operation/operation~Operation>} operationsB
  * @param {Object} options Additional transformation options.
  * @param {module:engine/model/document~Document|null} options.document Document which the operations change.
- * @param {Boolean} [options.useContext=false] Whether during transformation additional context information should be gathered and used.
+ * @param {Boolean} [options.useRelations=false] Whether during transformation relations should be used (used during undo for
+ * better conflict resolution).
  * @param {Boolean} [options.padWithNoOps=false] Whether additional {@link module:engine/model/operation/nooperation~NoOperation}s
  * should be added to the transformation results to force the same last base version for both transformed sets (in case
  * if some operations got broken into multiple operations during transformation).
@@ -302,7 +303,7 @@ export function transformSets( operationsA, operationsB, options ) {
 		originalOperationsBCount: operationsB.length
 	};
 
-	const contextFactory = new ContextFactory( options.document, options.useContext );
+	const contextFactory = new ContextFactory( options.document, options.useRelations );
 	contextFactory.setOriginalOperations( operationsA );
 	contextFactory.setOriginalOperations( operationsB );
 
@@ -380,13 +381,14 @@ class ContextFactory {
 	// Creates `ContextFactory` instance.
 	//
 	// @param {module:engine/model/document~Document} document Document which the operations change.
-	// @param {Boolean} useContext Whether during transformation additional context information should be gathered and used.
-	constructor( document, useContext ) {
+	// @param {Boolean} useRelations Whether during transformation relations should be used (used during undo for
+	// better conflict resolution).
+	constructor( document, useRelations ) {
 		// `model.History` instance which information about undone operations will be taken from.
 		this._history = document.history;
 
 		// Whether additional context should be used.
-		this._useContext = useContext;
+		this._useRelations = useRelations;
 
 		// For each operation that is created during transformation process, we keep a reference to the original operation
 		// which it comes from. The original operation works as a kind of "identifier". Every contextual information
@@ -508,29 +510,17 @@ class ContextFactory {
 	// @param {module:engine/model/operation/operation~Operation} opB
 	// @returns {module:engine/model/operation/transform~TransformationContext}
 	getContext( opA, opB, aIsStrong ) {
-		if ( !this._useContext ) {
-			return {
-				aIsStrong,
-				aWasUndone: false,
-				bWasUndone: false,
-				abRelation: null,
-				baRelation: null
-			};
-		}
-
 		return {
 			aIsStrong,
 			aWasUndone: this._wasUndone( opA ),
 			bWasUndone: this._wasUndone( opB ),
-			abRelation: this._getRelation( opA, opB ),
-			baRelation: this._getRelation( opB, opA )
+			abRelation: this._useRelations ? this._getRelation( opA, opB ) : null,
+			baRelation: this._useRelations ? this._getRelation( opB, opA ) : null
 		};
 	}
 
 	// Returns whether given operation `op` has already been undone.
 	//
-	// This is only used when additional context mode is on (options.useContext == true).
-	//
 	// Information whether an operation was undone gives more context when making a decision when two operations are in conflict.
 	//
 	// @param {module:engine/model/operation/operation~Operation} op
@@ -542,14 +532,12 @@ class ContextFactory {
 		const originalOp = this._originalOperations.get( op );
 
 		// And check with the document if the original operation was undone.
-		return this._history.isUndoneOperation( originalOp );
+		return originalOp.wasUndone || this._history.isUndoneOperation( originalOp );
 	}
 
 	// Returns a relation between `opA` and an operation which is undone by `opB`. This can be `String` value if a relation
 	// was set earlier or `null` if there was no relation between those operations.
 	//
-	// This is only used when additional context mode is on (options.useContext == true).
-	//
 	// This is a little tricky to understand, so let's compare it to `ContextFactory#_wasUndone`.
 	//
 	// When `wasUndone( opB )` is used, we check if the `opB` has already been undone. It is obvious, that the
@@ -1216,17 +1204,29 @@ setTransformation( MergeOperation, MergeOperation, ( a, b, context ) => {
 	// Both operations have same source and target positions. So the element already got merged and there is
 	// theoretically nothing to do.
 	//
-	// In this case, keep the source operation in the merged element - in the graveyard - and don't change target position.
-	// Doing this instead of returning `NoOperation` allows for a correct undo later.
-	//
 	if ( a.sourcePosition.isEqual( b.sourcePosition ) && a.targetPosition.isEqual( b.targetPosition ) ) {
-		const path = b.graveyardPosition.path.slice();
-		path.push( 0 );
+		// There are two ways that we can provide a do-nothing operation.
+		//
+		// First is simply a NoOperation instance. We will use it if `b` operation was not undone.
+		//
+		// Second is a merge operation that has the source operation in the merged element - in the graveyard -
+		// same target position and `howMany` equal to `0`. So it is basically merging an empty element from graveyard
+		// which is almost the same as NoOperation.
+		//
+		// This way the merge operation can be later transformed by split operation
+		// to provide correct undo. This will be used if `b` operation was undone (only then it is correct).
+		//
+		if ( !context.bWasUndone ) {
+			return [ new NoOperation( 0 ) ];
+		} else {
+			const path = b.graveyardPosition.path.slice();
+			path.push( 0 );
 
-		a.sourcePosition = new Position( b.graveyardPosition.root, path );
-		a.howMany = 0;
+			a.sourcePosition = new Position( b.graveyardPosition.root, path );
+			a.howMany = 0;
 
-		return [ a ];
+			return [ a ];
+		}
 	}
 
 	// The default case.
@@ -2031,7 +2031,74 @@ setTransformation( SplitOperation, InsertOperation, ( a, b ) => {
 	return [ a ];
 } );
 
-setTransformation( SplitOperation, MergeOperation, ( a, b ) => {
+setTransformation( SplitOperation, MergeOperation, ( a, b, context ) => {
+	// Case 1:
+	//
+	// Split element got merged. If two different elements were merged, clients will have different content.
+	//
+	// Example. Merge at `{}`, split at `[]`:
+	// <heading>Foo</heading>{}<paragraph>B[]ar</paragraph>
+	//
+	// On merge side it will look like this:
+	// <heading>FooB[]ar</heading>
+	// <heading>FooB</heading><heading>ar</heading>
+	//
+	// On split side it will look like this:
+	// <heading>Foo</heading>{}<paragraph>B</paragraph><paragraph>ar</paragraph>
+	// <heading>FooB</heading><paragraph>ar</paragraph>
+	//
+	// Clearly, the second element is different for both clients.
+	//
+	// We could use the removed merge element from graveyard as a split element but then clients would have a different
+	// model state (in graveyard), because the split side client would still have an element in graveyard (removed by merge).
+	//
+	// To overcome this, in `SplitOperation` x `MergeOperation` transformation we will add additional `SplitOperation`
+	// in the graveyard, which will actually clone the merged-and-deleted element. Then, that cloned element will be
+	// used for splitting. Example below.
+	//
+	// Original state:
+	// <heading>Foo</heading>{}<paragraph>B[]ar</paragraph>
+	//
+	// Merge side client:
+	//
+	// After merge:
+	// <heading>FooB[]ar</heading>                                 graveyard: <paragraph></paragraph>
+	//
+	// Extra split:
+	// <heading>FooB[]ar</heading>                                 graveyard: <paragraph></paragraph><paragraph></paragraph>
+	//
+	// Use the "cloned" element from graveyard:
+	// <heading>FooB</heading><paragraph>ar</paragraph>            graveyard: <paragraph></paragraph>
+	//
+	// Split side client:
+	//
+	// After split:
+	// <heading>Foo</heading>{}<paragraph>B</paragraph><paragraph>ar</paragraph>
+	//
+	// After merge:
+	// <heading>FooB</heading><paragraph>ar</paragraph>            graveyard: <paragraph></paragraph>
+	//
+	// This special case scenario only applies if the original split operation clones the split element.
+	// If the original split operation has `graveyardPosition` set, it all doesn't have sense because split operation
+	// knows exactly which element it should use. So there would be no original problem with different contents.
+	//
+	// Additionally, the special case applies only if the merge wasn't already undone.
+	//
+	if ( !a.graveyardPosition && !context.bWasUndone && a.position.hasSameParentAs( b.sourcePosition ) ) {
+		const splitPath = b.graveyardPosition.path.slice();
+		splitPath.push( 0 );
+
+		const additionalSplit = new SplitOperation( new Position( b.graveyardPosition.root, splitPath ), 0, null, 0 );
+
+		a.position = a.position._getTransformedByMergeOperation( b );
+		a.graveyardPosition = Position.createFromPosition( additionalSplit.insertionPosition );
+		a.graveyardPosition.stickiness = 'toNext';
+
+		return [ additionalSplit, a ];
+	}
+
+	// The default case.
+	//
 	if ( a.position.hasSameParentAs( b.deletionPosition ) && !a.position.isAfter( b.deletionPosition ) ) {
 		a.howMany--;
 	}

+ 4 - 156
packages/ckeditor5-engine/src/model/schema.js

@@ -23,163 +23,11 @@ import TreeWalker from './treewalker';
  *
  * The instance of schema is available in {@link module:engine/model/model~Model#schema `editor.model.schema`}.
  *
- * # Schema definitions
+ * Read more about the schema in:
  *
- * Schema defines allowed model structures and allowed attributes separately. They are also checked separately
- * by using the {@link ~Schema#checkChild} and {@link ~Schema#checkAttribute} methods.
- *
- * ## 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:
- *
- *		schema.register( 'myElement', {
- *			allowIn: '$root'
- *		} );
- *
- * This lets the schema know that `<myElement>` may be a child of the `<$root>` element. `$root` is one of 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:
- *
- *		<$root><myElement></myElement></$root>
- *
- * While this would not be correct:
- *
- *		<$root><foo><myElement></myElement></foo></$root>
- *
- * ## Generic items
- *
- * There are three basic generic items: `$root`, `$block` and `$text`.
- * They are defined as follows:
- *
- *		this.schema.register( '$root', {
- *			isLimit: true
- *		} );
- *		this.schema.register( '$block', {
- *			allowIn: '$root',
- *			isBlock: true
- *		} );
- *		this.schema.register( '$text', {
- *			allowIn: '$block'
- *		} );
- *
- * These definitions can then be reused by features to create their own definitions in a more extensible way.
- * For example, the {@link module:paragraph/paragraph~Paragraph} feature will define its item as:
- *
- *		schema.register( 'paragraph', {
- *			inheritAllFrom: '$block'
- *		} );
- *
- * Which translates to:
- *
- *		schema.register( 'paragraph', {
- *			allowWhere: '$block',
- *			allowContentOf: '$block',
- *			allowAttributesOf: '$block',
- *			inheritTypesFrom: '$block'
- *		} );
- *
- * 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 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:
- *
- *		schema.register( 'blockQuote', {
- *			allowWhere: '$block',
- *			allowContentOf: '$root'
- *		} );
- *
- * 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. So if anyone will
- * register a `<section>` element (with `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.
- *
- * You can read more about the format of an item definition in {@link module:engine/model/schema~SchemaItemDefinition}.
- *
- * ## Defining advanced rules in `checkChild()`'s callbacks
- *
- * The {@link ~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 declarative
- * {@link module:engine/model/schema~SchemaItemDefinition API}.
- *
- * Those listeners can be added either by listening directly to the {@link ~Schema#event:checkChild} event or
- * by using the handy {@link ~Schema#addChildCheck} method.
- *
- * For instance, the block quote feature defines such a listener to disallow nested `<blockQuote>` structures:
- *
- *		schema.addChildCheck( context, childDefinition ) => {
- *			// Note that context is automatically normalized to SchemaContext instance and
- *			// 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.
- *			if ( context.endsWith( 'blockQuote' ) && childDefinition.name == 'blockQuote' ) {
- *				return false;
- *			}
- *		} );
- *
- * ## Defining attributes
- *
- * TODO
- *
- * ## Implementing additional constraints
- *
- * Schema's capabilities were limited to simple (and atomic) {@link ~Schema#checkChild} and
- * {@link ~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.
- *
- * For instance, let's get back to the "element `<x>` must be always followed by `<y>`" rule and this initial content:
- *
- *		<$root>
- *			<x>foo</x>
- *			<y>bar[bom</y>
- *			<z>bom]bar</z>
- *		</$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:
- *
- *		<$root>
- *			<x>foo</x>
- *			<blockQuote>
- *				<y>bar[bom</y>
- *				<z>bom]bar</z>
- *			</blockQuote>
- *		</$root>
- *
- * But it turns out that this creates an incorrect structure – `<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's 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 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.
- *
- * So the answer for who and how should implement additional constraints is your features or your editor
- * through CKEditor 5's rich and open API.
+ * * {@glink framework/guides/architecture/editing-engine#schema "Schema"} section of the
+ * {@glink framework/guides/architecture/editing-engine Introduction to the "Editing engine architecture"}.
+ * * {@glink framework/guides/deep-dive/schema "Schema" deep dive} guide.
  *
  * @mixes module:utils/observablemixin~ObservableMixin
  */

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

@@ -45,6 +45,10 @@ import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
  * Note that the writer should never be stored and used outside of the `change()` and
  * `enqueueChange()` blocks.
  *
+ * Note that writer's methods do not check the {@link module:engine/model/schema~Schema}. It is possible
+ * to create incorrect model structures by using the writer. Read more about in
+ * {@glink framework/guides/deep-dive/schema#who-checks-the-schema "Who checks the schema?"}.
+ *
  * @see module:engine/model/model~Model#change
  * @see module:engine/model/model~Model#enqueueChange
  */

+ 44 - 0
packages/ckeditor5-engine/tests/model/operation/transform/merge.js

@@ -60,6 +60,50 @@ describe( 'transform', () => {
 			} );
 		} );
 
+		describe( 'by remove', () => {
+			it( 'remove merged element', () => {
+				john.setData( '<paragraph>Foo</paragraph>[]<paragraph>Bar</paragraph>' );
+				kate.setData( '<paragraph>Foo</paragraph>[<paragraph>Bar</paragraph>]' );
+
+				john.merge();
+				kate.remove();
+
+				syncClients();
+
+				expectClients( '<paragraph>Foo</paragraph>' );
+			} );
+
+			it( 'remove merged element then undo #1', () => {
+				john.setData( '<paragraph>Foo</paragraph>[]<paragraph>Bar</paragraph>' );
+				kate.setData( '<paragraph>Foo</paragraph>[<paragraph>Bar</paragraph>]' );
+
+				john.merge();
+				kate.remove();
+
+				syncClients();
+				expectClients( '<paragraph>Foo</paragraph>' );
+
+				kate.undo();
+
+				syncClients();
+
+				expectClients( '<paragraph>Foo</paragraph><paragraph>Bar</paragraph>' );
+			} );
+
+			it( 'remove merged element then undo #2', () => {
+				john.setData( '<paragraph>Foo</paragraph>[]<paragraph>Bar</paragraph>' );
+				kate.setData( '<paragraph>Foo</paragraph>[<paragraph>Bar</paragraph>]' );
+
+				john.merge();
+				kate.remove();
+				kate.undo();
+
+				syncClients();
+
+				expectClients( '<paragraph>FooBar</paragraph>' );
+			} );
+		} );
+
 		describe( 'by delete', () => {
 			it( 'text from two elements', () => {
 				john.setData( '<paragraph>Foo</paragraph>[]<paragraph>Bar</paragraph>' );

+ 69 - 0
packages/ckeditor5-engine/tests/model/operation/transform/split.js

@@ -317,6 +317,25 @@ describe( 'transform', () => {
 				expectClients( '<paragraph>Foo</paragraph>' );
 			} );
 
+			it( 'text in same position, then undo and redo', () => {
+				john.setData( '<paragraph>F[]oo</paragraph>' );
+				kate.setData( '<paragraph>F[]oo</paragraph>' );
+
+				john.split();
+				kate.split();
+
+				syncClients();
+
+				john.undo();
+
+				syncClients();
+
+				kate.undo();
+				kate.redo();
+
+				expectClients( '<paragraph>Foo</paragraph>' );
+			} );
+
 			it( 'text in different path', () => {
 				john.setData( '<paragraph>F[]oo</paragraph><paragraph>Bar</paragraph>' );
 				kate.setData( '<paragraph>Foo</paragraph><paragraph>B[]ar</paragraph>' );
@@ -403,6 +422,56 @@ describe( 'transform', () => {
 				syncClients();
 				expectClients( '<paragraph>Foo</paragraph><paragraph>Bar</paragraph>' );
 			} );
+
+			it( 'element into heading', () => {
+				john.setData( '<heading1>Foo</heading1><paragraph>B[]ar</paragraph>' );
+				kate.setData( '<heading1>Foo</heading1>[]<paragraph>Bar</paragraph>' );
+
+				john.split();
+				kate.merge();
+
+				syncClients();
+				expectClients(
+					'<heading1>FooB</heading1>' +
+					'<paragraph>ar</paragraph>'
+				);
+			} );
+
+			it( 'element into heading with undo #1', () => {
+				john.setData( '<heading1>Foo</heading1><paragraph>B[]ar</paragraph>' );
+				kate.setData( '<heading1>Foo</heading1>[]<paragraph>Bar</paragraph>' );
+
+				john.split();
+				kate.merge();
+
+				syncClients();
+				expectClients(
+					'<heading1>FooB</heading1>' +
+					'<paragraph>ar</paragraph>'
+				);
+
+				john.undo();
+				kate.undo();
+
+				syncClients();
+				expectClients( '<heading1>Foo</heading1><paragraph>Bar</paragraph>' );
+			} );
+
+			it( 'element into heading with undo #2', () => {
+				john.setData( '<heading1>Foo</heading1><paragraph>B[]ar</paragraph>' );
+				kate.setData( '<heading1>Foo</heading1>[]<paragraph>Bar</paragraph>' );
+
+				john.split();
+				kate.merge();
+				kate.undo();
+
+				syncClients();
+				expectClients(
+					'<heading1>Foo</heading1>' +
+					'<paragraph>B</paragraph>' +
+					'<paragraph>ar</paragraph>'
+				);
+			} );
 		} );
 
 		describe( 'by delete', () => {

+ 30 - 7
packages/ckeditor5-engine/tests/model/operation/transform/utils.js

@@ -5,6 +5,7 @@ import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
 import Typing from '@ckeditor/ckeditor5-typing/src/typing';
 import UndoEditing from '@ckeditor/ckeditor5-undo/src/undoediting';
 import BlockQuoteEditing from '@ckeditor/ckeditor5-block-quote/src/blockquoteediting';
+import HeadingEditing from '@ckeditor/ckeditor5-heading/src/headingediting';
 
 import { getData, parse } from '../../../../src/dev-utils/model';
 import { transformSets } from '../../../../src/model/operation/transform';
@@ -29,7 +30,7 @@ export class Client {
 			// Typing is needed for delete command.
 			// UndoEditing is needed for undo command.
 			// Block plugins are needed for proper data serializing.
-			plugins: [ Typing, Paragraph, ListEditing, UndoEditing, BlockQuoteEditing ]
+			plugins: [ Typing, Paragraph, ListEditing, UndoEditing, BlockQuoteEditing, HeadingEditing ]
 		} ).then( editor => {
 			this.editor = editor;
 			this.document = editor.model.document;
@@ -189,6 +190,10 @@ export class Client {
 		this._processExecute( 'undo' );
 	}
 
+	redo() {
+		this._processExecute( 'redo' );
+	}
+
 	_processExecute( commandName, commandArgs ) {
 		const oldVersion = this.document.version;
 
@@ -254,7 +259,7 @@ export class Client {
 }
 
 function bufferOperations( operations, client ) {
-	bufferedOperations.add( { operations: operations.map( operation => JSON.stringify( operation ) ), client } );
+	bufferedOperations.add( { operations, client } );
 }
 
 export function syncClients() {
@@ -277,13 +282,31 @@ export function syncClients() {
 				continue;
 			}
 
-			const remoteOperationsJson = clientsOperations[ remoteClient.name ];
-
-			if ( !remoteOperationsJson ) {
+			if ( !clientsOperations[ remoteClient.name ] ) {
 				continue;
 			}
 
-			const remoteOperations = remoteOperationsJson.map( op => OperationFactory.fromJSON( JSON.parse( op ), localClient.document ) );
+			// Stringify and rebuild operations to simulate sending operations. Set `wasUndone`.
+			const remoteOperationsJson = clientsOperations[ remoteClient.name ].map( operation => {
+				operation.wasUndone = remoteClient.document.history.isUndoneOperation( operation );
+
+				const json = JSON.stringify( operation );
+
+				delete operation.wasUndone;
+
+				return json;
+			} );
+
+			const remoteOperations = remoteOperationsJson.map( json => {
+				const parsedJson = JSON.parse( json );
+				const operation = OperationFactory.fromJSON( parsedJson, localClient.document );
+
+				if ( parsedJson.wasUndone ) {
+					operation.wasUndone = true;
+				}
+
+				return operation;
+			} );
 
 			const localOperations = Array.from( localClient.document.history.getOperations( localClient.syncedVersion ) );
 
@@ -291,7 +314,7 @@ export function syncClients() {
 
 			const options = {
 				document: localClient.document,
-				useContext: false,
+				useRelations: false,
 				padWithNoOps: true
 			};