Merge pull request #1260 from ckeditor/t/730

Fix: `Model#insertContent()` will not merge nodes if the model after the merge would violate schema rules. Closes ckeditor/ckeditor5#730.

src/model/utils/insertcontent.js

@@ -165,6 +165,7 @@ class Insertion {
 	}
 
 	/**
+	 * @private
 	 * Handles insertion of a single node.
 	 *
 	 * @param {module:engine/model/node~Node} node
@@ -211,6 +212,7 @@ class Insertion {
 	}
 
 	/**
+	 * @private
 	 * @param {module:engine/model/element~Element} node The object element.
 	 * @param {Object} context
 	 */
@@ -226,6 +228,7 @@ class Insertion {
 	}
 
 	/**
+	 * @private
 	 * @param {module:engine/model/node~Node} node The disallowed node which needs to be handled.
 	 * @param {Object} context
 	 */
@@ -241,6 +244,7 @@ class Insertion {
 	}
 
 	/**
+	 * @private
 	 * @param {module:engine/model/node~Node} node The node to insert.
 	 */
 	_insert( node ) {
@@ -274,6 +278,7 @@ class Insertion {
 	}
 
 	/**
+	 * @private
 	 * @param {module:engine/model/node~Node} node The node which could potentially be merged.
 	 * @param {Object} context
 	 */
@@ -282,8 +287,8 @@ class Insertion {
 			return;
 		}
 
-		const mergeLeft = context.isFirst && ( node.previousSibling instanceof Element ) && this.canMergeWith.has( node.previousSibling );
-		const mergeRight = context.isLast && ( node.nextSibling instanceof Element ) && this.canMergeWith.has( node.nextSibling );
+		const mergeLeft = this._canMergeLeft( node, context );
+		const mergeRight = this._canMergeRight( node, context );
 		const mergePosLeft = LivePosition.createBefore( node );
 		const mergePosRight = LivePosition.createAfter( node );
 
@@ -329,9 +334,44 @@ class Insertion {
 		mergePosRight.detach();
 	}
 
+	/**
+	 * Checks whether specified node can be merged with previous sibling element.
+	 *
+	 * @private
+	 * @param {module:engine/model/node~Node} node The node which could potentially be merged.
+	 * @param {Object} context
+	 * @returns {Boolean}
+	 */
+	_canMergeLeft( node, context ) {
+		const previousSibling = node.previousSibling;
+
+		return context.isFirst &&
+			( previousSibling instanceof Element ) &&
+			this.canMergeWith.has( previousSibling ) &&
+			this.model.schema.checkMerge( previousSibling, node );
+	}
+
+	/**
+	 * Checks whether specified node can be merged with next sibling element.
+	 *
+	 * @private
+	 * @param {module:engine/model/node~Node} node The node which could potentially be merged.
+	 * @param {Object} context
+	 * @returns {Boolean}
+	 */
+	_canMergeRight( node, context ) {
+		const nextSibling = node.nextSibling;
+
+		return context.isLast &&
+			( nextSibling instanceof Element ) &&
+			this.canMergeWith.has( nextSibling ) &&
+			this.model.schema.checkMerge( node, nextSibling );
+	}
+
 	/**
 	 * Tries wrapping the node in a new paragraph and inserting it this way.
 	 *
+	 * @private
 	 * @param {module:engine/model/node~Node} node The node which needs to be autoparagraphed.
 	 * @param {Object} context
 	 */
@@ -348,6 +388,7 @@ class Insertion {
 	}
 
 	/**
+	 * @private
 	 * @param {module:engine/model/node~Node} node
 	 * @returns {Boolean} Whether an allowed position was found.
 	 * `false` is returned if the node isn't allowed at any position up in the tree, `true` if was.
@@ -393,6 +434,7 @@ class Insertion {
 	/**
 	 * Gets the element in which the given node is allowed. It checks the passed element and all its ancestors.
 	 *
+	 * @private
 	 * @param {module:engine/model/node~Node} node The node to check.
 	 * @param {module:engine/model/element~Element} element The element in which the node's correctness should be checked.
 	 * @returns {module:engine/model/element~Element|null}

tests/model/utils/insertcontent.js

@@ -407,6 +407,63 @@ describe( 'DataController utils', () => {
 						'<paragraph>b</paragraph>'
 					);
 				} );
+
+				it( 'should not merge a paragraph wrapped in blockQuote with list item (checking left merge)', () => {
+					model.schema.register( 'blockQuote', {
+						allowWhere: '$block',
+						allowContentOf: '$root',
+					} );
+
+					setData( model, '<listItem>fo[]o</listItem>' );
+
+					insertHelper( '<blockQuote><paragraph>xxx</paragraph></blockQuote><heading1>yyy</heading1>' );
+
+					expect( getData( model ) ).to.equal(
+						'<listItem>fo</listItem>' +
+						'<blockQuote>' +
+						'<paragraph>xxx</paragraph>' +
+						'</blockQuote>' +
+						'<heading1>yyy[]o</heading1>'
+					);
+				} );
+
+				it( 'should not merge a paragraph wrapped in blockQuote with list item (checking right merge)', () => {
+					model.schema.register( 'blockQuote', {
+						allowWhere: '$block',
+						allowContentOf: '$root',
+					} );
+
+					setData( model, '<listItem>fo[]o</listItem>' );
+
+					insertHelper( '<heading1>yyy</heading1><blockQuote><paragraph>xxx</paragraph></blockQuote>' );
+
+					expect( getData( model ) ).to.equal(
+						'<listItem>foyyy</listItem>' +
+						'<blockQuote>' +
+						'<paragraph>xxx</paragraph>' +
+						'</blockQuote>' +
+						'<listItem>[]o</listItem>'
+					);
+				} );
+
+				it( 'should not merge a paragraph wrapped in blockQuote with list item (checking both merges)', () => {
+					model.schema.register( 'blockQuote', {
+						allowWhere: '$block',
+						allowContentOf: '$root',
+					} );
+
+					setData( model, '<listItem>fo[]o</listItem>' );
+
+					insertHelper( '<blockQuote><paragraph>xxx</paragraph></blockQuote>' );
+
+					expect( getData( model ) ).to.equal(
+						'<listItem>fo</listItem>' +
+						'<blockQuote>' +
+						'<paragraph>xxx</paragraph>' +
+						'</blockQuote>' +
+						'<listItem>[]o</listItem>'
+					);
+				} );
 			} );
 
 			describe( 'mixed content to block', () => {