瀏覽代碼

Merge branch 'master' into i/7285

Aleksander Nowodzinski 5 年之前
父節點
當前提交
80de7a91ac

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

@@ -46,7 +46,7 @@ While this would be incorrect:
 
 ## Defining additional semantics
 
-In addition to setting allowed structures, the schema can also define additional traits of model elements. By using the `is*` properties a feature author may declare how a certain element should be treated by other features and the engine.
+In addition to setting allowed structures, the schema can also define additional traits of model elements. By using the `is*` properties, a feature author may declare how a certain element should be treated by other features and the engine.
 
 ### Limit elements
 
@@ -55,9 +55,9 @@ Consider a feature like an image caption. The caption text area should construct
 * A selection that starts inside should not end outside.
 * Pressing <kbd>Backspace</kbd> or <kbd>Delete</kbd> should not delete the area. Pressing <kbd>Enter</kbd> should not split the area.
 
-It should also act as a boundary for external actions. This is mostly enforced by a selection post-fixer that ensures that a selection that starts outside, should not end inside. That means that most actions will either apply to the "outside" of such an element or to a content inside it.
+It should also act as a boundary for external actions. This is mostly enforced by a selection post-fixer that ensures that a selection that starts outside, should not end inside. It means that most actions will either apply to the "outside" of such an element or to the content inside it.
 
-Taken these characteristics, the image caption should be defined as limit element by using the {@link module:engine/model/schema~SchemaItemDefinition#isLimit `isLimit`} property.
+Taken these characteristics, the image caption should be defined as a limit element by using the {@link module:engine/model/schema~SchemaItemDefinition#isLimit `isLimit`} property.
 
 ```js
 schema.register( 'myCaption', {
@@ -73,9 +73,9 @@ The engine and various features then check it via {@link module:engine/model/sch
 
 ### Object elements
 
-For the image caption as in the example above it does not make much sense to select the caption box, then copy or drag it somewhere else.
+For an image caption like in the example above it does not make much sense to select the caption box, then copy or drag it somewhere else.
 
-A caption without the image that it describes does not make much sense. However, the image is more self-sufficient. Most likely users should be able to select the entire image (with all its internals), then copy or move it around. {@link module:engine/model/schema~SchemaItemDefinition#isObject `isObject`} should be used to mark such behavior.
+A caption without the image that it describes makes little sense. However, the image is more self-sufficient. Most likely users should be able to select the entire image (with all its internals), then copy or move it around. The {@link module:engine/model/schema~SchemaItemDefinition#isObject `isObject`} property should be used to mark such behavior.
 
 ```js
 schema.register( 'myImage', {
@@ -95,7 +95,7 @@ The {@link module:engine/model/schema~Schema#isObject `Schema#isObject()`} can l
 
 Generally speaking, content is usually made out of blocks like paragraphs, list items, images, headings, etc. All these elements should be marked as blocks by using {@link module:engine/model/schema~SchemaItemDefinition#isBlock `isBlock`}.
 
-It is important to remember that a block should not allow another block inside. Container elements like `<blockQuote>` which can contain other block elements should not be marked as blocks.
+It is important to remember that a block should not allow another block inside. Container elements like `<blockQuote>`, which can contain other block elements, should not be marked as blocks.
 
 <info-box>
 	There is also the `$block` generic item which has `isBlock` set to `true`. Most block type items will inherit from `$block` (through `inheritAllFrom`).
@@ -103,11 +103,11 @@ It is important to remember that a block should not allow another block inside.
 
 ### Inline elements
 
-In the editor, all HTML formatting elements such as `<strong>` or `<code>` are represented by text attributes. Therefore, inline model elements are not to be used for this scenarios.
+In the editor, all HTML formatting elements such as `<strong>` or `<code>` are represented by text attributes. Therefore, inline model elements are not supposed to be used for these scenarios.
 
 Currently, the {@link module:engine/model/schema~SchemaItemDefinition#isInline `isInline`} property is used for the `$text` token (so, text nodes) and elements such as `<softBreak>` or placeholder elements such as in the {@link framework/guides/tutorials/implementing-an-inline-widget Implementing an inline widget} tutorial.
 
-The support for inline elements in CKEditor 5 is so far limited to self-contained elements. This is &mdash; all elements marked with `isInline` should also be marked with `isObject`.
+The support for inline elements in CKEditor 5 is so far limited to self-contained elements. Because of this, all elements marked with `isInline` should also be marked with `isObject`.
 
 ## Generic items
 

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

@@ -232,7 +232,7 @@ export default class Element extends Node {
 
 	/**
 	 * Creates a copy of this element and returns it. Created element has the same name and attributes as the original element.
-	 * If clone is deep, the original element's children are also cloned. If not, then empty element is removed.
+	 * If clone is deep, the original element's children are also cloned. If not, then empty element is returned.
 	 *
 	 * @protected
 	 * @param {Boolean} [deep=false] If set to `true` clones element and all its children recursively. When set to `false`,

+ 39 - 39
packages/ckeditor5-engine/src/model/schema.js

@@ -26,9 +26,9 @@ import TreeWalker from './treewalker';
  *
  * Read more about the schema in:
  *
- * * {@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.
+ * * {@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
  */
@@ -171,7 +171,7 @@ export default class Schema {
 	}
 
 	/**
-	 * Returns a definition of the given item or `undefined` if item is not registered.
+	 * Returns a definition of the given item or `undefined` if an item is not registered.
 	 *
 	 * This method should normally be used for reflection purposes (e.g. defining a clone of a certain element,
 	 * checking a list of all block elements, etc).
@@ -212,7 +212,7 @@ export default class Schema {
 
 	/**
 	 * Returns `true` if the given item is defined to be
-	 * a block by {@link module:engine/model/schema~SchemaItemDefinition}'s `isBlock` property.
+	 * a block by the {@link module:engine/model/schema~SchemaItemDefinition}'s `isBlock` property.
 	 *
 	 *		schema.isBlock( 'paragraph' ); // -> true
 	 *		schema.isBlock( '$root' ); // -> false
@@ -220,7 +220,7 @@ export default class Schema {
 	 *		const paragraphElement = writer.createElement( 'paragraph' );
 	 *		schema.isBlock( paragraphElement ); // -> true
 	 *
-	 * See the {@glink framework/guides/deep-dive/schema#block-elements Block elements} section of the "Schema" deep dive}
+	 * See the {@glink framework/guides/deep-dive/schema#block-elements Block elements} section of the Schema deep dive}
 	 * guide for more details.
 	 *
 	 * @param {module:engine/model/item~Item|module:engine/model/schema~SchemaContextItem|String} item
@@ -234,18 +234,18 @@ export default class Schema {
 	/**
 	 * Returns `true` if the given item should be treated as a limit element.
 	 *
-	 * It considers the item to be a limit element if its
+	 * It considers an item to be a limit element if its
 	 * {@link module:engine/model/schema~SchemaItemDefinition}'s
 	 * {@link module:engine/model/schema~SchemaItemDefinition#isLimit `isLimit`} or
 	 * {@link module:engine/model/schema~SchemaItemDefinition#isObject `isObject`} property
-	 * were set to `true`.
+	 * was set to `true`.
 	 *
 	 *		schema.isLimit( 'paragraph' ); // -> false
 	 *		schema.isLimit( '$root' ); // -> true
 	 *		schema.isLimit( editor.model.document.getRoot() ); // -> true
 	 *		schema.isLimit( 'image' ); // -> true
 	 *
-	 * See the {@glink framework/guides/deep-dive/schema#limit-elements Limit elements} section of the "Schema" deep dive}
+	 * See the {@glink framework/guides/deep-dive/schema#limit-elements Limit elements} section of the Schema deep dive}
 	 * guide for more details.
 	 *
 	 * @param {module:engine/model/item~Item|module:engine/model/schema~SchemaContextItem|String} item
@@ -261,12 +261,12 @@ export default class Schema {
 	}
 
 	/**
-	 * Returns `true` if the given item is should be treated as an object element.
+	 * Returns `true` if the given item should be treated as an object element.
 	 *
-	 * It considers the item to be an object element if its
+	 * It considers an item to be an object element if its
 	 * {@link module:engine/model/schema~SchemaItemDefinition}'s
 	 * {@link module:engine/model/schema~SchemaItemDefinition#isObject `isObject`} property
-	 * were set to `true`.
+	 * was set to `true`.
 	 *
 	 *		schema.isObject( 'paragraph' ); // -> false
 	 *		schema.isObject( 'image' ); // -> true
@@ -274,7 +274,7 @@ export default class Schema {
 	 *		const imageElement = writer.createElement( 'image' );
 	 *		schema.isObject( imageElement ); // -> true
 	 *
-	 * See the {@glink framework/guides/deep-dive/schema#object-elements Object elements} section of the "Schema" deep dive}
+	 * See the {@glink framework/guides/deep-dive/schema#object-elements Object elements} section of the Schema deep dive}
 	 * guide for more details.
 	 *
 	 * @param {module:engine/model/item~Item|module:engine/model/schema~SchemaContextItem|String} item
@@ -287,7 +287,7 @@ export default class Schema {
 
 	/**
 	 * Returns `true` if the given item is defined to be
-	 * an inline element by {@link module:engine/model/schema~SchemaItemDefinition}'s `isInline` property.
+	 * an inline element by the {@link module:engine/model/schema~SchemaItemDefinition}'s `isInline` property.
 	 *
 	 *		schema.isInline( 'paragraph' ); // -> false
 	 *		schema.isInline( 'softBreak' ); // -> true
@@ -295,7 +295,7 @@ export default class Schema {
 	 *		const text = writer.createText('foo' );
 	 *		schema.isInline( text ); // -> true
 	 *
-	 * See the {@glink framework/guides/deep-dive/schema#inline-elements Inline elements} section of the "Schema" deep dive}
+	 * See the {@glink framework/guides/deep-dive/schema#inline-elements Inline elements} section of the Schema deep dive}
 	 * guide for more details.
 	 *
 	 * @param {module:engine/model/item~Item|module:engine/model/schema~SchemaContextItem|String} item
@@ -737,13 +737,13 @@ export default class Schema {
 	}
 
 	/**
-	 * Tries to find position ancestors that allows to insert given node.
+	 * Tries to find position ancestors that allow to insert a given node.
 	 * It starts searching from the given position and goes node by node to the top of the model tree
-	 * as long as {@link module:engine/model/schema~Schema#isLimit limit element},
-	 * {@link module:engine/model/schema~Schema#isObject object element} or top-most ancestor won't be reached.
+	 * as long as a {@link module:engine/model/schema~Schema#isLimit limit element}, an
+	 * {@link module:engine/model/schema~Schema#isObject object element} or a topmost ancestor is not reached.
 	 *
-	 * @param {module:engine/model/position~Position} position Position from searching will start.
-	 * @param {module:engine/model/node~Node|String} node Node for which allowed parent should be found or its name.
+	 * @param {module:engine/model/position~Position} position The position that the search will start from.
+	 * @param {module:engine/model/node~Node|String} node The node for which an allowed parent should be found or its name.
 	 * @returns {module:engine/model/element~Element|null} element Allowed parent or null if nothing was found.
 	 */
 	findAllowedParent( position, node ) {
@@ -873,7 +873,7 @@ export default class Schema {
 	 * This is a helper function for {@link ~Schema#getValidRanges}.
 	 *
 	 * @private
-	 * @param {module:engine/model/range~Range} range Range to process.
+	 * @param {module:engine/model/range~Range} range The range to process.
 	 * @param {String} attribute The name of the attribute to check.
 	 * @returns {Iterable.<module:engine/model/range~Range>} Ranges in which the attribute is allowed.
 	 */
@@ -907,7 +907,7 @@ mix( Schema, ObservableMixin );
 
 /**
  * Event fired when the {@link #checkChild} method is called. It allows plugging in
- * additional behavior – e.g. implementing rules which cannot be defined using the declarative
+ * additional behavior, for example implementing rules which cannot be defined using the declarative
  * {@link module:engine/model/schema~SchemaItemDefinition} interface.
  *
  * **Note:** The {@link #addChildCheck} method is a more handy way to register callbacks. Internally,
@@ -916,7 +916,7 @@ mix( Schema, ObservableMixin );
  *
  * The {@link #checkChild} method fires an event because it is
  * {@link module:utils/observablemixin~ObservableMixin#decorate decorated} with it. Thanks to that you can
- * use this event in a various way, but the most important use case is overriding standard behaviour of the
+ * use this event in various ways, but the most important use case is overriding standard behavior of the
  * `checkChild()` method. Let's see a typical listener template:
  *
  *		schema.on( 'checkChild', ( evt, args ) => {
@@ -927,12 +927,12 @@ mix( Schema, ObservableMixin );
  * The listener is added with a `high` priority to be executed before the default method is really called. The `args` callback
  * parameter contains arguments passed to `checkChild( context, child )`. However, the `context` parameter is already
  * normalized to a {@link module:engine/model/schema~SchemaContext} instance and `child` to a
- * {@link module:engine/model/schema~SchemaCompiledItemDefinition} instance, so you don't have to worry about
+ * {@link module:engine/model/schema~SchemaCompiledItemDefinition} instance, so you do not have to worry about
  * the various ways how `context` and `child` may be passed to `checkChild()`.
  *
  * **Note:** `childDefinition` may be `undefined` if `checkChild()` was called with a non-registered element.
  *
- * So, in order to implement a rule "disallow `heading1` in `blockQuote`" you can add such a listener:
+ * So, in order to implement a rule "disallow `heading1` in `blockQuote`", you can add such a listener:
  *
  *		schema.on( 'checkChild', ( evt, args ) => {
  *			const context = args[ 0 ];
@@ -946,8 +946,8 @@ mix( Schema, ObservableMixin );
  *			}
  *		}, { priority: 'high' } );
  *
- * Allowing elements in specific contexts will be a far less common use case, because it's normally handled by
- * `allowIn` rule from {@link module:engine/model/schema~SchemaItemDefinition} but if you have a complex scenario
+ * Allowing elements in specific contexts will be a far less common use case, because it is normally handled by the
+ * `allowIn` rule from {@link module:engine/model/schema~SchemaItemDefinition}. But if you have a complex scenario
  * where `listItem` should be allowed only in element `foo` which must be in element `bar`, then this would be the way:
  *
  *		schema.on( 'checkChild', ( evt, args ) => {
@@ -968,16 +968,16 @@ mix( Schema, ObservableMixin );
 
 /**
  * Event fired when the {@link #checkAttribute} method is called. It allows plugging in
- * additional behavior – e.g. implementing rules which cannot be defined using the declarative
+ * additional behavior, for example implementing rules which cannot be defined using the declarative
  * {@link module:engine/model/schema~SchemaItemDefinition} interface.
  *
  * **Note:** The {@link #addAttributeCheck} method is a more handy way to register callbacks. Internally,
  * it registers a listener to this event but comes with a simpler API and it is the recommended choice
  * in most of the cases.
  *
- * The {@link #checkAttribute} method fires an event because it's
+ * The {@link #checkAttribute} method fires an event because it is
  * {@link module:utils/observablemixin~ObservableMixin#decorate decorated} with it. Thanks to that you can
- * use this event in a various way, but the most important use case is overriding standard behaviour of the
+ * use this event in various ways, but the most important use case is overriding the standard behavior of the
  * `checkAttribute()` method. Let's see a typical listener template:
  *
  *		schema.on( 'checkAttribute', ( evt, args ) => {
@@ -987,10 +987,10 @@ mix( Schema, ObservableMixin );
  *
  * The listener is added with a `high` priority to be executed before the default method is really called. The `args` callback
  * parameter contains arguments passed to `checkAttribute( context, attributeName )`. However, the `context` parameter is already
- * normalized to a {@link module:engine/model/schema~SchemaContext} instance, so you don't have to worry about
+ * normalized to a {@link module:engine/model/schema~SchemaContext} instance, so you do not have to worry about
  * the various ways how `context` may be passed to `checkAttribute()`.
  *
- * So, in order to implement a rule "disallow `bold` in a text which is in a `heading1` you can add such a listener:
+ * So, in order to implement a rule "disallow `bold` in a text which is in a `heading1`, you can add such a listener:
  *
  *		schema.on( 'checkAttribute', ( evt, args ) => {
  *			const context = args[ 0 ];
@@ -1004,8 +1004,8 @@ mix( Schema, ObservableMixin );
  *			}
  *		}, { priority: 'high' } );
  *
- * Allowing attributes in specific contexts will be a far less common use case, because it's normally handled by
- * `allowAttributes` rule from {@link module:engine/model/schema~SchemaItemDefinition} but if you have a complex scenario
+ * Allowing attributes in specific contexts will be a far less common use case, because it is normally handled by the
+ * `allowAttributes` rule from {@link module:engine/model/schema~SchemaItemDefinition}. But if you have a complex scenario
  * where `bold` should be allowed only in element `foo` which must be in element `bar`, then this would be the way:
  *
  *		schema.on( 'checkAttribute', ( evt, args ) => {
@@ -1056,7 +1056,7 @@ mix( Schema, ObservableMixin );
  * {@link module:engine/model/schema~Schema#isLimit `isLimit()`} returns `true` for object elements automatically.
  *
  * Read more about the meaning of these types in the
- * {@glink framework/guides/deep-dive/schema#defining-additional-semantics Dedicated section of the "Schema" deep dive} guide.
+ * {@glink framework/guides/deep-dive/schema#defining-additional-semantics dedicated section of the Schema deep dive} guide.
  *
  * # Generic items
  *
@@ -1159,14 +1159,14 @@ mix( Schema, ObservableMixin );
  * Most block type items will inherit from `$block` (through `inheritAllFrom`).
  *
  * Read more about the block elements in the
- * {@glink framework/guides/deep-dive/schema#block-elements Block elements} section of the "Schema" deep dive} guide.
+ * {@glink framework/guides/deep-dive/schema#block-elements Block elements} section of the Schema deep dive} guide.
  *
  * @property {Boolean} isInline
  * Whether an item is "text-like" and should be treated as an inline node. Examples of inline elements:
  * `$text`, `softBreak` (`<br>`), etc.
  *
  * Read more about the inline elements in the
- * {@glink framework/guides/deep-dive/schema#inline-elements Inline elements} section of the "Schema" deep dive} guide.
+ * {@glink framework/guides/deep-dive/schema#inline-elements Inline elements} section of the Schema deep dive} guide.
  *
  * @property {Boolean} isLimit
  * It can be understood as whether this element should not be split by <kbd>Enter</kbd>.
@@ -1174,7 +1174,7 @@ mix( Schema, ObservableMixin );
  * a limit element are limited to its content.
  *
  * Read more about the limit elements in the
- * {@glink framework/guides/deep-dive/schema#limit-elements Limit elements} section of the "Schema" deep dive} guide.
+ * {@glink framework/guides/deep-dive/schema#limit-elements Limit elements} section of the Schema deep dive} guide.
  *
  * @property {Boolean} isObject
  * Whether an item is "self-contained" and should be treated as a whole. Examples of object elements:
@@ -1184,7 +1184,7 @@ mix( Schema, ObservableMixin );
  * {@link module:engine/model/schema~Schema#isLimit `isLimit()`} returns `true` for object elements automatically.
  *
  * Read more about the object elements in the
- * {@glink framework/guides/deep-dive/schema#object-elements Object elements} section of the "Schema" deep dive} guide.
+ * {@glink framework/guides/deep-dive/schema#object-elements Object elements} section of the Schema deep dive} guide.
  */
 
 /**

+ 297 - 47
packages/ckeditor5-engine/src/model/utils/deletecontent.js

@@ -80,8 +80,8 @@ export default function deleteContent( model, selection, options = {} ) {
 			return;
 		}
 
-		const startPos = selRange.start;
-		const endPos = LivePosition.fromPosition( selRange.end, 'toNext' );
+		// Get the live positions for the range adjusted to span only blocks selected from the user perspective.
+		const [ startPosition, endPosition ] = getLivePositionsForSelectedBlocks( selRange );
 
 		// 2. Remove the content if there is any.
 		if ( !selRange.start.isTouching( selRange.end ) ) {
@@ -97,7 +97,7 @@ export default function deleteContent( model, selection, options = {} ) {
 		// as it's hard to imagine what should actually be the default behavior. Usually, specific features will
 		// want to override that behavior anyway.
 		if ( !options.leaveUnmerged ) {
-			mergeBranches( writer, startPos, endPos );
+			mergeBranches( writer, startPosition, endPosition );
 
 			// TMP this will be replaced with a postfixer.
 			// We need to check and strip disallowed attributes in all nested nodes because after merge
@@ -105,81 +105,331 @@ export default function deleteContent( model, selection, options = {} ) {
 			//
 			// e.g. bold is disallowed for <H1>
 			// <h1>Fo{o</h1><p>b}a<b>r</b><p> -> <h1>Fo{}a<b>r</b><h1> -> <h1>Fo{}ar<h1>.
-			schema.removeDisallowedAttributes( startPos.parent.getChildren(), writer );
+			schema.removeDisallowedAttributes( startPosition.parent.getChildren(), writer );
 		}
 
-		collapseSelectionAt( writer, selection, startPos );
+		collapseSelectionAt( writer, selection, startPosition );
 
 		// 4. Add a paragraph to set selection in it.
 		// Check if a text is allowed in the new container. If not, try to create a new paragraph (if it's allowed here).
 		// If autoparagraphing is off, we assume that you know what you do so we leave the selection wherever it was.
-		if ( !options.doNotAutoparagraph && shouldAutoparagraph( schema, startPos ) ) {
-			insertParagraph( writer, startPos, selection );
+		if ( !options.doNotAutoparagraph && shouldAutoparagraph( schema, startPosition ) ) {
+			insertParagraph( writer, startPosition, selection );
 		}
 
-		endPos.detach();
+		startPosition.detach();
+		endPosition.detach();
 	} );
 }
 
+// Returns the live positions for the range adjusted to span only blocks selected from the user perspective. Example:
+//
+//     <heading1>[foo</heading1>
+//     <paragraph>bar</paragraph>
+//     <heading1>]abc</heading1>  <-- this block is not considered as selected
+//
+// This is the same behavior as in Selection#getSelectedBlocks() "special case".
+function getLivePositionsForSelectedBlocks( range ) {
+	const model = range.root.document.model;
+
+	const startPosition = range.start;
+	let endPosition = range.end;
+
+	// If the end of selection is at the start position of last block in the selection, then
+	// shrink it to not include that trailing block. Note that this should happen only for not empty selection.
+	if ( model.hasContent( range ) ) {
+		const endBlock = getParentBlock( endPosition );
+
+		if ( endBlock && endPosition.isTouching( model.createPositionAt( endBlock, 0 ) ) ) {
+			// Create forward selection as a probe to find a valid position after excluding last block from the range.
+			const selection = model.createSelection( range );
+
+			// Modify the forward selection in backward direction to shrink it and remove first position of following block from it.
+			// This is how modifySelection works and here we are making use of it.
+			model.modifySelection( selection, { direction: 'backward' } );
+
+			endPosition = selection.getLastPosition();
+		}
+	}
+
+	return [
+		LivePosition.fromPosition( startPosition, 'toPrevious' ),
+		LivePosition.fromPosition( endPosition, 'toNext' )
+	];
+}
+
+// Finds the lowest element in position's ancestors which is a block.
+// Returns null if a limit element is encountered before reaching a block element.
+function getParentBlock( position ) {
+	const element = position.parent;
+	const schema = element.root.document.model.schema;
+	const ancestors = element.getAncestors( { parentFirst: true, includeSelf: true } );
+
+	for ( const element of ancestors ) {
+		if ( schema.isLimit( element ) ) {
+			return null;
+		}
+
+		if ( schema.isBlock( element ) ) {
+			return element;
+		}
+	}
+}
+
 // This function is a result of reaching the Ballmer's peak for just the right amount of time.
 // Even I had troubles documenting it after a while and after reading it again I couldn't believe that it really works.
-function mergeBranches( writer, startPos, endPos ) {
-	const startParent = startPos.parent;
-	const endParent = endPos.parent;
+function mergeBranches( writer, startPosition, endPosition ) {
+	const model = writer.model;
 
-	// If both positions ended up in the same parent, then there's nothing more to merge:
-	// <$root><p>x[]</p><p>{}y</p></$root> => <$root><p>xy</p>[]{}</$root>
-	if ( startParent == endParent ) {
+	// Verify if there is a need and possibility to merge.
+	if ( !checkShouldMerge( writer.model.schema, startPosition, endPosition ) ) {
 		return;
 	}
 
-	// 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;
+	// If the start element on the common ancestor level is empty, and the end element on the same level is not empty
+	// then merge those to the right element so that it's properties are preserved (name, attributes).
+	// Because of OT merging is used instead of removing elements.
+	//
+	// Merge left:
+	//     <heading1>foo[</heading1>    ->  <heading1>foo[]bar</heading1>
+	//     <paragraph>]bar</paragraph>  ->               --^
+	//
+	// Merge right:
+	//     <heading1>[</heading1>       ->
+	//     <paragraph>]bar</paragraph>  ->  <paragraph>[]bar</paragraph>
+	//
+	// Merge left:
+	//     <blockQuote>                     ->  <blockQuote>
+	//         <heading1>foo[</heading1>    ->      <heading1>foo[]bar</heading1>
+	//         <paragraph>]bar</paragraph>  ->                   --^
+	//     </blockQuote>                    ->  </blockQuote>
+	//
+	// Merge right:
+	//     <blockQuote>                     ->  <blockQuote>
+	//         <heading1>[</heading1>       ->
+	//         <paragraph>]bar</paragraph>  ->      <paragraph>[]bar</paragraph>
+	//     </blockQuote>                    ->  </blockQuote>
+
+	// Merging should not go deeper than common ancestor.
+	const [ startAncestor, endAncestor ] = getAncestorsJustBelowCommonAncestor( startPosition, endPosition );
+
+	if ( !model.hasContent( startAncestor ) && model.hasContent( endAncestor ) ) {
+		mergeBranchesRight( writer, startPosition, endPosition, startAncestor.parent );
+	} else {
+		mergeBranchesLeft( writer, startPosition, endPosition, startAncestor.parent );
 	}
+}
 
-	// Check if operations we'll need to do won't need to cross object or limit boundaries.
-	// E.g., we can't merge endParent into startParent in this case:
-	// <limit><startParent>x[]</startParent></limit><endParent>{}</endParent>
-	if ( !checkCanBeMerged( startPos, endPos, writer.model.schema ) ) {
+// Merging blocks to the left (properties of the left block are preserved).
+// Simple example:
+//     <heading1>foo[</heading1>    ->  <heading1>foo[bar</heading1>]
+//     <paragraph>]bar</paragraph>  ->              --^
+//
+// Nested example:
+//     <blockQuote>                     ->  <blockQuote>
+//         <heading1>foo[</heading1>    ->      <heading1>foo[bar</heading1>
+//     </blockQuote>                    ->  </blockQuote>]    ^
+//     <blockBlock>                     ->                    |
+//         <paragraph>]bar</paragraph>  ->                 ---
+//     </blockBlock>                    ->
+//
+function mergeBranchesLeft( writer, startPosition, endPosition, commonAncestor ) {
+	const startElement = startPosition.parent;
+	const endElement = endPosition.parent;
+
+	// Merging reached the common ancestor element, stop here.
+	if ( startElement == commonAncestor || endElement == commonAncestor ) {
 		return;
 	}
 
-	// Remember next positions to merge. For example:
-	// <a><b>x[]</b></a><c><d>{}y</d></c>
-	// will become:
-	// <a><b>xy</b>[]</a><c>{}</c>
-	startPos = writer.createPositionAfter( startParent );
-	endPos = writer.createPositionBefore( endParent );
+	// Remember next positions to merge in next recursive step (also used as modification points pointers).
+	startPosition = writer.createPositionAfter( startElement );
+	endPosition = writer.createPositionBefore( endElement );
 
-	if ( !endPos.isEqual( startPos ) ) {
-		// In this case, before we merge, we need to move `endParent` to the `startPos`:
-		// <a><b>x[]</b></a><c><d>{}y</d></c>
-		// becomes:
-		// <a><b>x</b>[]<d>y</d></a><c>{}</c>
-		writer.insert( endParent, startPos );
+	// Move endElement just after startElement if they aren't siblings.
+	if ( !endPosition.isEqual( startPosition ) ) {
+		//
+		//     <blockQuote>                     ->  <blockQuote>
+		//         <heading1>foo[</heading1>    ->      <heading1>foo</heading1>[<paragraph>bar</paragraph>
+		//     </blockQuote>                    ->  </blockQuote>                ^
+		//     <blockBlock>                     ->  <blockBlock>                 |
+		//         <paragraph>]bar</paragraph>  ->      ]                     ---
+		//     </blockBlock>                    ->  </blockBlock>
+		//
+		writer.insert( endElement, startPosition );
 	}
 
-	// Merge two siblings:
-	// <a>x</a>[]<b>y</b> -> <a>xy</a> (the usual case)
-	// <a><b>x</b>[]<d>y</d></a><c></c> -> <a><b>xy</b>[]</a><c></c> (this is the "move parent" case shown above)
-	writer.merge( startPos );
+	// Merge two siblings (nodes on sides of startPosition):
+	//
+	//     <blockQuote>                                             ->  <blockQuote>
+	//         <heading1>foo</heading1>[<paragraph>bar</paragraph>  ->      <heading1>foo[bar</heading1>
+	//     </blockQuote>                                            ->  </blockQuote>
+	//     <blockBlock>                                             ->  <blockBlock>
+	//         ]                                                    ->      ]
+	//     </blockBlock>                                            ->  </blockBlock>
+	//
+	// Or in simple case (without moving elements in above if):
+	//     <heading1>foo</heading1>[<paragraph>bar</paragraph>]  ->  <heading1>foo[bar</heading1>]
+	//
+	writer.merge( startPosition );
 
 	// Remove empty end ancestors:
-	// <a>fo[o</a><b><a><c>bar]</c></a></b>
-	// becomes:
-	// <a>fo[]</a><b><a>{}</a></b>
-	// So we can remove <a> and <b>.
-	while ( endPos.parent.isEmpty ) {
-		const parentToRemove = endPos.parent;
+	//
+	//     <blockQuote>                      ->  <blockQuote>
+	//         <heading1>foo[bar</heading1>  ->      <heading1>foo[bar</heading1>
+	//     </blockQuote>                     ->  </blockQuote>
+	//     <blockBlock>                      ->
+	//         ]                             ->  ]
+	//     </blockBlock>                     ->
+	//
+	while ( endPosition.parent.isEmpty ) {
+		const parentToRemove = endPosition.parent;
+
+		endPosition = writer.createPositionBefore( parentToRemove );
 
-		endPos = writer.createPositionBefore( parentToRemove );
+		writer.remove( parentToRemove );
+	}
+
+	// Verify if there is a need and possibility to merge next level.
+	if ( !checkShouldMerge( writer.model.schema, startPosition, endPosition ) ) {
+		return;
+	}
+
+	// Continue merging next level (blockQuote with blockBlock in the examples above if it would not be empty and got removed).
+	mergeBranchesLeft( writer, startPosition, endPosition, commonAncestor );
+}
+
+// Merging blocks to the right (properties of the right block are preserved).
+// Simple example:
+//     <heading1>foo[</heading1>    ->            --v
+//     <paragraph>]bar</paragraph>  ->  [<paragraph>foo]bar</paragraph>
+//
+// Nested example:
+//     <blockQuote>                     ->
+//         <heading1>foo[</heading1>    ->              ---
+//     </blockQuote>                    ->                 |
+//     <blockBlock>                     ->  [<blockBlock>  v
+//         <paragraph>]bar</paragraph>  ->      <paragraph>foo]bar</paragraph>
+//     </blockBlock>                    ->  </blockBlock>
+//
+function mergeBranchesRight( writer, startPosition, endPosition, commonAncestor ) {
+	const startElement = startPosition.parent;
+	const endElement = endPosition.parent;
+
+	// Merging reached the common ancestor element, stop here.
+	if ( startElement == commonAncestor || endElement == commonAncestor ) {
+		return;
+	}
+
+	// Remember next positions to merge in next recursive step (also used as modification points pointers).
+	startPosition = writer.createPositionAfter( startElement );
+	endPosition = writer.createPositionBefore( endElement );
+
+	// Move startElement just before endElement if they aren't siblings.
+	if ( !endPosition.isEqual( startPosition ) ) {
+		//
+		//     <blockQuote>                     ->  <blockQuote>
+		//         <heading1>foo[</heading1>    ->      [                   ---
+		//     </blockQuote>                    ->  </blockQuote>              |
+		//     <blockBlock>                     ->  <blockBlock>               v
+		//         <paragraph>]bar</paragraph>  ->      <heading1>foo</heading1>]<paragraph>bar</paragraph>
+		//     </blockBlock>                    ->  </blockBlock>
+		//
+		writer.insert( startElement, endPosition );
+	}
+
+	// Remove empty end ancestors:
+	//
+	//     <blockQuote>                                             ->
+	//         [                                                    ->  [
+	//     </blockQuote>                                            ->
+	//     <blockBlock>                                             ->  <blockBlock>
+	//         <heading1>foo</heading1>]<paragraph>bar</paragraph>  ->      <heading1>foo</heading1>]<paragraph>bar</paragraph>
+	//     </blockBlock>                                            ->  </blockBlock>
+	//
+	while ( startPosition.parent.isEmpty ) {
+		const parentToRemove = startPosition.parent;
+
+		startPosition = writer.createPositionBefore( parentToRemove );
 
 		writer.remove( parentToRemove );
 	}
 
-	// Continue merging next level.
-	mergeBranches( writer, startPos, endPos );
+	// Update endPosition after inserting and removing elements.
+	endPosition = writer.createPositionBefore( endElement );
+
+	// Merge right two siblings (nodes on sides of endPosition):
+	//                                                              ->
+	//     [                                                        ->  [
+	//                                                              ->
+	//     <blockBlock>                                             ->  <blockBlock>
+	//         <heading1>foo</heading1>]<paragraph>bar</paragraph>  ->      <paragraph>foo]bar</paragraph>
+	//     </blockBlock>                                            ->  </blockBlock>
+	//
+	// Or in simple case (without moving elements in above if):
+	//     [<heading1>foo</heading1>]<paragraph>bar</paragraph>  ->  [<heading1>foo]bar</heading1>
+	//
+	mergeRight( writer, endPosition );
+
+	// Verify if there is a need and possibility to merge next level.
+	if ( !checkShouldMerge( writer.model.schema, startPosition, endPosition ) ) {
+		return;
+	}
+
+	// Continue merging next level (blockQuote with blockBlock in the examples above if it would not be empty and got removed).
+	mergeBranchesRight( writer, startPosition, endPosition, commonAncestor );
+}
+
+// There is no right merge operation so we need to simulate it.
+function mergeRight( writer, position ) {
+	const startElement = position.nodeBefore;
+	const endElement = position.nodeAfter;
+
+	if ( startElement.name != endElement.name ) {
+		writer.rename( startElement, endElement.name );
+	}
+
+	writer.clearAttributes( startElement );
+	writer.setAttributes( Object.fromEntries( endElement.getAttributes() ), startElement );
+
+	writer.merge( position );
+}
+
+// Verifies if merging is needed and possible. It's not needed if both positions are in the same element
+// and it's not possible if some element is a limit or the range crosses a limit element.
+function checkShouldMerge( schema, startPosition, endPosition ) {
+	const startElement = startPosition.parent;
+	const endElement = endPosition.parent;
+
+	// If both positions ended up in the same parent, then there's nothing more to merge:
+	// <$root><p>x[</p><p>]y</p></$root> => <$root><p>xy</p>[]</$root>
+	if ( startElement == endElement ) {
+		return false;
+	}
+
+	// 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 ( schema.isLimit( startElement ) || schema.isLimit( endElement ) ) {
+		return false;
+	}
+
+	// Check if operations we'll need to do won't need to cross object or limit boundaries.
+	// E.g., we can't merge endElement into startElement in this case:
+	// <limit><startElement>x[</startElement></limit><endElement>]</endElement>
+	return isCrossingLimitElement( startPosition, endPosition, schema );
+}
+
+// Returns the elements that are the ancestors of the provided positions that are direct children of the common ancestor.
+function getAncestorsJustBelowCommonAncestor( positionA, positionB ) {
+	const ancestorsA = positionA.getAncestors();
+	const ancestorsB = positionB.getAncestors();
+
+	let i = 0;
+
+	while ( ancestorsA[ i ] && ancestorsA[ i ] == ancestorsB[ i ] ) {
+		i++;
+	}
+
+	return [ ancestorsA[ i ], ancestorsB[ i ] ];
 }
 
 function shouldAutoparagraph( schema, position ) {
@@ -195,7 +445,7 @@ function shouldAutoparagraph( schema, position ) {
 // E.g. in <bQ><p>x[]</p></bQ><widget><caption>{}</caption></widget>
 // we'll check <p>, <bQ>, <widget> and <caption>.
 // Usually, widget and caption are marked as objects/limits in the schema, so in this case merging will be blocked.
-function checkCanBeMerged( leftPos, rightPos, schema ) {
+function isCrossingLimitElement( leftPos, rightPos, schema ) {
 	const rangeToCheck = new Range( leftPos, rightPos );
 
 	for ( const value of rangeToCheck.getWalker() ) {

+ 1 - 1
packages/ckeditor5-engine/src/model/utils/getselectedcontent.js

@@ -73,7 +73,7 @@ export default function getSelectedContent( model, selection ) {
 			if ( item.is( 'textProxy' ) ) {
 				writer.appendText( item.data, item.getAttributes(), frag );
 			} else {
-				writer.append( item._clone( true ), frag );
+				writer.append( writer.cloneElement( item, true ), frag );
 			}
 		}
 

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

@@ -117,6 +117,18 @@ export default class Writer {
 	}
 
 	/**
+	 * Creates a copy of the element and returns it. Created element has the same name and attributes as the original element.
+	 * If clone is deep, the original element's children are also cloned. If not, then empty element is returned.
+	 *
+	 * @param {module:engine/model/element~Element} element The element to clone.
+	 * @param {Boolean} [deep=true] If set to `true` clones element and all its children recursively. When set to `false`,
+	 * element will be cloned without any child.
+	 */
+	cloneElement( element, deep = true ) {
+		return element._clone( deep );
+	}
+
+	/**
 	 * Inserts item on given position.
 	 *
 	 *		const paragraph = writer.createElement( 'paragraph' );

+ 249 - 97
packages/ckeditor5-engine/tests/model/utils/deletecontent.js

@@ -216,11 +216,13 @@ describe( 'DataController utils', () => {
 					allowIn: 'pparent',
 					allowAttributes: 'align'
 				} );
-				schema.register( 'heading1', { inheritAllFrom: '$block' } );
+				schema.register( 'heading1', { inheritAllFrom: '$block', allowIn: 'pparent' } );
 				schema.register( 'image', { inheritAllFrom: '$text' } );
 				schema.register( 'pchild', { allowIn: 'paragraph' } );
 				schema.register( 'pparent', { allowIn: '$root' } );
-				schema.extend( '$text', { allowIn: [ 'pchild', 'pparent' ] } );
+				schema.register( 'hchild', { allowIn: 'heading1' } );
+				schema.register( 'widget', { isObject: true, allowWhere: '$text', isInline: true } );
+				schema.extend( '$text', { allowIn: [ 'pchild', 'pparent', 'hchild' ] } );
 			} );
 
 			test(
@@ -236,6 +238,30 @@ describe( 'DataController utils', () => {
 			);
 
 			test(
+				'merges first element into the second element (it would become empty but second element would not) (same name)',
+				'<paragraph>x</paragraph><paragraph>[foo</paragraph><paragraph>b]ar</paragraph><paragraph>y</paragraph>',
+				'<paragraph>x</paragraph><paragraph>[]ar</paragraph><paragraph>y</paragraph>'
+			);
+
+			test(
+				'do not remove end block if selection ends at start position of it',
+				'<paragraph>x</paragraph><paragraph>[foo</paragraph><paragraph>]bar</paragraph><paragraph>y</paragraph>',
+				'<paragraph>x</paragraph><paragraph>[]</paragraph><paragraph>bar</paragraph><paragraph>y</paragraph>'
+			);
+
+			test(
+				'removes empty element (merges it into second element)',
+				'<paragraph>x</paragraph><paragraph>[</paragraph><paragraph>]bar</paragraph><paragraph>y</paragraph>',
+				'<paragraph>x</paragraph><paragraph>[]bar</paragraph><paragraph>y</paragraph>'
+			);
+
+			test(
+				'treats inline widget elements as content so parent element is not considered as empty after merging (same name)',
+				'<paragraph>x</paragraph><paragraph><widget></widget>[foo</paragraph><paragraph>b]ar</paragraph><paragraph>y</paragraph>',
+				'<paragraph>x</paragraph><paragraph><widget></widget>[]ar</paragraph><paragraph>y</paragraph>'
+			);
+
+			test(
 				'does not merge second element into the first one (same name, !option.merge)',
 				'<paragraph>x</paragraph><paragraph>fo[o</paragraph><paragraph>b]ar</paragraph><paragraph>y</paragraph>',
 				'<paragraph>x</paragraph><paragraph>fo[]</paragraph><paragraph>ar</paragraph><paragraph>y</paragraph>',
@@ -243,11 +269,24 @@ describe( 'DataController utils', () => {
 			);
 
 			test(
+				'does not remove first empty element when it\'s empty but second element is not empty (same name, !option.merge)',
+				'<paragraph>x</paragraph><paragraph>[foo</paragraph><paragraph>b]ar</paragraph><paragraph>y</paragraph>',
+				'<paragraph>x</paragraph><paragraph>[]</paragraph><paragraph>ar</paragraph><paragraph>y</paragraph>',
+				{ leaveUnmerged: true }
+			);
+
+			test(
 				'merges second element into the first one (different name)',
 				'<paragraph>x</paragraph><heading1>fo[o</heading1><paragraph>b]ar</paragraph><paragraph>y</paragraph>',
 				'<paragraph>x</paragraph><heading1>fo[]ar</heading1><paragraph>y</paragraph>'
 			);
 
+			test(
+				'removes first element when it\'s empty but second element is not empty (different name)',
+				'<paragraph>x</paragraph><heading1>[foo</heading1><paragraph>b]ar</paragraph><paragraph>y</paragraph>',
+				'<paragraph>x</paragraph><paragraph>[]ar</paragraph><paragraph>y</paragraph>'
+			);
+
 			// Note: in all these cases we ignore the direction of merge.
 			// If https://github.com/ckeditor/ckeditor5-engine/issues/470 was fixed we could differently treat
 			// forward and backward delete.
@@ -270,9 +309,9 @@ describe( 'DataController utils', () => {
 			);
 
 			test(
-				'merges second element to an empty first element',
+				'removes empty first element',
 				'<paragraph>x</paragraph><heading1>[</heading1><paragraph>fo]o</paragraph><paragraph>y</paragraph>',
-				'<paragraph>x</paragraph><heading1>[]o</heading1><paragraph>y</paragraph>'
+				'<paragraph>x</paragraph><paragraph>[]o</paragraph><paragraph>y</paragraph>'
 			);
 
 			test(
@@ -283,8 +322,20 @@ describe( 'DataController utils', () => {
 
 			test(
 				'leaves just one element when all selected',
+				'<paragraph>[x</paragraph><paragraph>foo</paragraph><paragraph>y]bar</paragraph>',
+				'<paragraph>[]bar</paragraph>'
+			);
+
+			test(
+				'leaves just one (last) element when all selected (first block would become empty) (different name)',
 				'<heading1>[x</heading1><paragraph>foo</paragraph><paragraph>y]bar</paragraph>',
-				'<heading1>[]bar</heading1>'
+				'<paragraph>[]bar</paragraph>'
+			);
+
+			test(
+				'leaves just one (first) element when all selected (first block would not become empty) (different name)',
+				'<heading1>foo[x</heading1><paragraph>bar</paragraph><paragraph>y]</paragraph>',
+				'<heading1>foo[]</heading1>'
 			);
 
 			it( 'uses merge operation even if merged element is empty', () => {
@@ -317,6 +368,36 @@ describe( 'DataController utils', () => {
 				expect( mergeSpy.called ).to.be.true;
 			} );
 
+			it( 'uses "merge" operation (from OT) if first element is empty (because of content delete) and last is not', () => {
+				let mergeSpy;
+
+				setData( model, '<paragraph>[abcd</paragraph><paragraph>ef]gh</paragraph>' );
+
+				model.change( writer => {
+					mergeSpy = sinon.spy( writer, 'merge' );
+					deleteContent( model, doc.selection );
+				} );
+
+				expect( getData( model ) ).to.equal( '<paragraph>[]gh</paragraph>' );
+
+				expect( mergeSpy.called ).to.be.true;
+			} );
+
+			it( 'uses merge operation if first element is empty and last is not', () => {
+				let mergeSpy;
+
+				setData( model, '<paragraph>[</paragraph><paragraph>ef]gh</paragraph>' );
+
+				model.change( writer => {
+					mergeSpy = sinon.spy( writer, 'merge' );
+					deleteContent( model, doc.selection );
+				} );
+
+				expect( getData( model ) ).to.equal( '<paragraph>[]gh</paragraph>' );
+
+				expect( mergeSpy.called ).to.be.true;
+			} );
+
 			it( 'does not try to move the second block if not needed', () => {
 				let mergeSpy, moveSpy;
 
@@ -344,47 +425,41 @@ describe( 'DataController utils', () => {
 					'<paragraph>x<pchild>fo[]ar</pchild>y</paragraph>'
 				);
 
-				it( 'merges elements when deep nested (3rd level)', () => {
-					const root = doc.getRoot();
-
-					// We need to use the raw API due to https://github.com/ckeditor/ckeditor5-engine/issues/905.
-					// <pparent>x<paragraph>x<pchild>fo[o</pchild></paragraph></pparent>
-					// <pparent><paragraph><pchild>b]ar</pchild>y</paragraph>y</pparent>
-
-					root._appendChild(
-						new Element( 'pparent', null, [
-							'x',
-							new Element( 'paragraph', null, [
-								'x',
-								new Element( 'pchild', null, 'foo' )
-							] )
-						] )
-					);
+				test(
+					'merges block element to the right (with nested element)',
+					'<paragraph><pchild>[foo</pchild></paragraph><paragraph><pchild>b]ar</pchild></paragraph>',
+					'<paragraph><pchild>[]ar</pchild></paragraph>'
+				);
 
-					root._appendChild(
-						new Element( 'pparent', null, [
-							new Element( 'paragraph', null, [
-								new Element( 'pchild', null, 'bar' ),
-								'y'
-							] ),
-							'y'
-						] )
-					);
+				test(
+					'does not remove block element with nested element and object',
+					'<paragraph><pchild><widget></widget>[foo</pchild></paragraph><paragraph><pchild>b]ar</pchild></paragraph>',
+					'<paragraph><pchild><widget></widget>[]ar</pchild></paragraph>'
+				);
 
-					const range = new Range(
-						new Position( doc.getRoot(), [ 0, 1, 1, 2 ] ), // fo[o
-						new Position( doc.getRoot(), [ 1, 0, 0, 1 ] ) // b]ar
-					);
+				test(
+					'merges nested elements',
+					'<heading1><hchild>x[foo</hchild></heading1><paragraph><pchild>b]ar</pchild></paragraph>',
+					'<heading1><hchild>x[]ar</hchild></heading1>'
+				);
 
-					model.change( writer => {
-						writer.setSelection( range );
-					} );
+				test(
+					'merges nested elements on multiple levels',
+					'<heading1><hchild>x[foo</hchild></heading1><paragraph><pchild>b]ar</pchild>abc</paragraph>',
+					'<heading1><hchild>x[]ar</hchild>abc</heading1>'
+				);
 
-					deleteContent( model, doc.selection );
+				test(
+					'merges nested elements to the right if left side element would become empty',
+					'<heading1><hchild>[foo</hchild></heading1><paragraph><pchild>b]ar</pchild></paragraph>',
+					'<paragraph><pchild>[]ar</pchild></paragraph>'
+				);
 
-					expect( getData( model ) )
-						.to.equal( '<pparent>x<paragraph>x<pchild>fo[]ar</pchild>y</paragraph>y</pparent>' );
-				} );
+				test(
+					'merges to the left if first element contains object (considers it as a content of that element)',
+					'<heading1><hchild><widget></widget>[foo</hchild></heading1><paragraph><pchild>b]ar</pchild></paragraph>',
+					'<heading1><hchild><widget></widget>[]ar</hchild></heading1>'
+				);
 
 				test(
 					'merges elements when left end deep nested',
@@ -393,89 +468,166 @@ describe( 'DataController utils', () => {
 				);
 
 				test(
+					'merges nested elements to the right (on multiple levels) if left side element would become empty',
+					'<heading1><hchild>[foo</hchild></heading1><paragraph><pchild>b]ar</pchild>abc</paragraph>',
+					'<paragraph><pchild>[]ar</pchild>abc</paragraph>'
+				);
+
+				test(
+					'merges to the right element when left end deep nested and will be empty',
+					'<paragraph><pchild>[foo</pchild></paragraph><paragraph>b]ar</paragraph><paragraph>x</paragraph>',
+					'<paragraph>[]ar</paragraph><paragraph>x</paragraph>'
+				);
+
+				test(
 					'merges elements when right end deep nested',
 					'<paragraph>x</paragraph><paragraph>fo[o</paragraph><paragraph><pchild>b]ar</pchild>x</paragraph>',
 					'<paragraph>x</paragraph><paragraph>fo[]ar</paragraph><paragraph>x</paragraph>'
 				);
 
-				it( 'merges elements when left end deep nested (3rd level)', () => {
-					const root = doc.getRoot();
+				test(
+					'removes element when right end deep nested but left end would be empty',
+					'<paragraph>x</paragraph><paragraph>[foo</paragraph><paragraph><pchild>b]ar</pchild></paragraph>',
+					'<paragraph>x</paragraph><paragraph><pchild>[]ar</pchild></paragraph>'
+				);
 
-					// We need to use the raw API due to https://github.com/ckeditor/ckeditor5-engine/issues/905.
-					// <pparent>x<paragraph>foo<pchild>ba[r</pchild></paragraph></pparent><paragraph>b]om</paragraph>
+				test(
+					'merges elements when right end deep nested (in an empty container)',
+					'<paragraph>fo[o</paragraph><paragraph><pchild>bar]</pchild></paragraph>',
+					'<paragraph>fo[]</paragraph>'
+				);
 
-					root._appendChild(
-						new Element( 'pparent', null, [
-							'x',
-							new Element( 'paragraph', null, [
-								'foo',
-								new Element( 'pchild', null, 'bar' )
-							] )
-						] )
-					);
+				test(
+					'removes elements when left end deep nested (in an empty container)',
+					'<paragraph><pchild>[foo</pchild></paragraph><paragraph>b]ar</paragraph><paragraph>x</paragraph>',
+					'<paragraph>[]ar</paragraph><paragraph>x</paragraph>'
+				);
 
-					root._appendChild(
-						new Element( 'paragraph', null, 'bom' )
+				describe( 'with 3rd level of nesting', () => {
+					test(
+						'merges elements when deep nested (same name)',
+						'<pparent>x<paragraph>x<pchild>fo[o</pchild></paragraph></pparent>' +
+						'<pparent><paragraph><pchild>b]ar</pchild>y</paragraph>y</pparent>',
+						'<pparent>x<paragraph>x<pchild>fo[]ar</pchild>y</paragraph>y</pparent>'
 					);
 
-					const range = new Range(
-						new Position( doc.getRoot(), [ 0, 1, 3, 2 ] ), // ba[r
-						new Position( doc.getRoot(), [ 1, 1 ] ) // b]om
+					test(
+						'removes elements when deep nested (same name)',
+						'<pparent><paragraph><pchild>[foo</pchild></paragraph></pparent>' +
+						'<pparent><paragraph><pchild>b]ar</pchild>y</paragraph>y</pparent>',
+						'<pparent><paragraph><pchild>[]ar</pchild>y</paragraph>y</pparent>'
 					);
 
-					model.change( writer => {
-						writer.setSelection( range );
-					} );
+					test(
+						'removes elements up to common ancestor when deep nested (same name)',
+						'<pparent>' +
+							'<paragraph><pchild>[foo</pchild></paragraph>' +
+							'<paragraph><pchild>b]ar</pchild>y</paragraph>y' +
+						'</pparent>',
+						'<pparent><paragraph><pchild>[]ar</pchild>y</paragraph>y</pparent>'
+					);
 
-					deleteContent( model, doc.selection );
+					test(
+						'merges elements when deep nested (different name)',
+						'<pparent>x<heading1>x<hchild>fo[o</hchild></heading1></pparent>' +
+						'<pparent><paragraph><pchild>b]ar</pchild>y</paragraph>y</pparent>',
+						'<pparent>x<heading1>x<hchild>fo[]ar</hchild>y</heading1>y</pparent>'
+					);
 
-					expect( getData( model ) )
-						.to.equal( '<pparent>x<paragraph>foo<pchild>ba[]om</pchild></paragraph></pparent>' );
-				} );
+					test(
+						'removes elements when deep nested (different name)',
+						'<pparent><heading1><hchild>[foo</hchild></heading1></pparent>' +
+						'<pparent><paragraph><pchild>b]ar</pchild>y</paragraph>y</pparent>',
+						'<pparent><paragraph><pchild>[]ar</pchild>y</paragraph>y</pparent>'
+					);
 
-				test(
-					'merges elements when right end deep nested (in an empty container)',
-					'<paragraph>fo[o</paragraph><paragraph><pchild>bar]</pchild></paragraph>',
-					'<paragraph>fo[]</paragraph>'
-				);
+					test(
+						'merges elements up to common ancestor when deep nested (different names)',
+						'<pparent>' +
+							'<heading1><hchild>fo[o</hchild></heading1>' +
+							'<paragraph><pchild>b]ar</pchild></paragraph>' +
+						'</pparent>',
+						'<pparent><heading1><hchild>fo[]ar</hchild></heading1></pparent>'
+					);
 
-				test(
-					'merges elements when left end deep nested (in an empty container)',
-					'<paragraph><pchild>[foo</pchild></paragraph><paragraph>b]ar</paragraph><paragraph>x</paragraph>',
-					'<paragraph><pchild>[]ar</pchild></paragraph><paragraph>x</paragraph>'
-				);
+					test(
+						'removes elements up to common ancestor when deep nested (different names)',
+						'<pparent>' +
+							'<heading1><hchild>[foo</hchild></heading1>' +
+							'<paragraph><pchild>b]ar</pchild>y</paragraph>y' +
+						'</pparent>',
+						'<pparent><paragraph><pchild>[]ar</pchild>y</paragraph>y</pparent>'
+					);
+				} );
 
-				it( 'merges elements when right end deep nested (3rd level)', () => {
-					const root = doc.getRoot();
+				describe( 'with 3rd level of nesting o the left end', () => {
+					test(
+						'merges elements',
+						'<pparent>x<paragraph>foo<pchild>ba[r</pchild></paragraph></pparent>' +
+						'<paragraph>b]om</paragraph>',
+						'<pparent>x<paragraph>foo<pchild>ba[]om</pchild></paragraph></pparent>'
+					);
 
-					// We need to use the raw API due to https://github.com/ckeditor/ckeditor5-engine/issues/905.
-					// <paragraph>fo[o</paragraph><pparent><paragraph><pchild>bar]</pchild></paragraph></pparent>
+					test(
+						'merges elements (different names)',
+						'<pparent>x<heading1>foo<hchild>ba[r</hchild></heading1></pparent>' +
+						'<paragraph>b]om</paragraph>',
+						'<pparent>x<heading1>foo<hchild>ba[]om</hchild></heading1></pparent>'
+					);
 
-					root._appendChild(
-						new Element( 'paragraph', null, 'foo' )
+					test(
+						'removes elements',
+						'<pparent><paragraph><pchild>[bar</pchild></paragraph></pparent>' +
+						'<paragraph>b]om</paragraph>',
+						'<paragraph>[]om</paragraph>'
 					);
 
-					root._appendChild(
-						new Element( 'pparent', null, [
-							new Element( 'paragraph', null, [
-								new Element( 'pchild', null, 'bar' )
-							] )
-						] )
+					test(
+						'removes elements up to common ancestor (different names)',
+						'<pparent>' +
+							'<heading1><hchild>[foo</hchild></heading1>' +
+							'<paragraph>b]ar</paragraph>y' +
+						'</pparent>',
+						'<pparent><paragraph>[]ar</paragraph>y</pparent>'
 					);
+				} );
 
-					const range = new Range(
-						new Position( doc.getRoot(), [ 0, 2 ] ), // f[oo
-						new Position( doc.getRoot(), [ 1, 0, 0, 3 ] ) // bar]
+				describe( 'with 3rd level of nesting o the right end', () => {
+					test(
+						'merges elements',
+						'<paragraph>b[om</paragraph>' +
+						'<pparent><paragraph><pchild>ba]r</pchild></paragraph></pparent>',
+						'<paragraph>b[]r</paragraph>'
 					);
 
-					model.change( writer => {
-						writer.setSelection( range );
-					} );
+					test(
+						'merges elements (different names)',
+						'<paragraph>bo[m</paragraph>' +
+						'<pparent><heading1><hchild>b]ar</hchild></heading1></pparent>',
+						'<paragraph>bo[]ar</paragraph>'
+					);
+					test(
+						'merges elements (different names, reversed)',
+						'<heading1>bo[m</heading1>' +
+						'<pparent><paragraph><pchild>b]ar</pchild></paragraph></pparent>',
+						'<heading1>bo[]ar</heading1>'
+					);
 
-					deleteContent( model, doc.selection );
+					test(
+						'removes elements',
+						'<paragraph>[bom</paragraph>' +
+						'<pparent><paragraph><pchild>b]ar</pchild></paragraph></pparent>',
+						'<pparent><paragraph><pchild>[]ar</pchild></paragraph></pparent>'
+					);
 
-					expect( getData( model ) )
-						.to.equal( '<paragraph>fo[]</paragraph>' );
+					test(
+						'removes elements up to common ancestor (different names)',
+						'<pparent>' +
+							'<heading1>[bar</heading1>y' +
+							'<paragraph><pchild>f]oo</pchild></paragraph>' +
+						'</pparent>',
+						'<pparent><paragraph><pchild>[]oo</pchild></paragraph></pparent>'
+					);
 				} );
 			} );
 

+ 35 - 0
packages/ckeditor5-engine/tests/model/writer.js

@@ -86,6 +86,35 @@ describe( 'Writer', () => {
 		} );
 	} );
 
+	describe( 'cloneElement()', () => {
+		it( 'should make deep copy of element', () => {
+			const element = createElement( 'foo', { 'abc': '123' } );
+
+			insertElement( createElement( 'bar', { 'xyz': '789' } ), element );
+
+			const clonedElement = cloneElement( element );
+
+			expect( clonedElement ).to.not.equal( element );
+			expect( clonedElement.getChild( 0 ) ).to.not.equal( element.getChild( 0 ) );
+			expect( clonedElement.toJSON() ).to.deep.equal( element.toJSON() );
+		} );
+
+		it( 'should make shallow copy of element', () => {
+			const element = createElement( 'foo', { 'abc': '123' } );
+
+			insertElement( createElement( 'bar', { 'xyz': '789' } ), element );
+
+			const elementJson = element.toJSON();
+			delete elementJson.children;
+
+			const clonedElement = cloneElement( element, false );
+
+			expect( clonedElement ).to.not.equal( element );
+			expect( clonedElement.childCount ).to.equal( 0 );
+			expect( clonedElement.toJSON() ).to.deep.equal( elementJson );
+		} );
+	} );
+
 	describe( 'insert()', () => {
 		it( 'should insert node at given position', () => {
 			const parent = createDocumentFragment();
@@ -2902,6 +2931,12 @@ describe( 'Writer', () => {
 		} );
 	}
 
+	function cloneElement( element, deep ) {
+		return model.change( writer => {
+			return writer.cloneElement( element, deep );
+		} );
+	}
+
 	function insert( item, itemOrPosition, offset ) {
 		model.enqueueChange( batch, writer => {
 			writer.insert( item, itemOrPosition, offset );

+ 5 - 6
packages/ckeditor5-table/src/commands/mergecellcommand.js

@@ -11,6 +11,7 @@ import Command from '@ckeditor/ckeditor5-core/src/command';
 import TableWalker from '../tablewalker';
 import { getTableCellsContainingSelection } from '../utils/selection';
 import { findAncestor, isHeadingColumnCell } from '../utils/common';
+import { removeEmptyRowsColumns } from '../utils/structure';
 
 /**
  * The merge cell command.
@@ -104,13 +105,11 @@ export default class MergeCellCommand extends Command {
 			writer.setAttribute( spanAttribute, cellSpan + cellToMergeSpan, cellToExpand );
 			writer.setSelection( writer.createRangeIn( cellToExpand ) );
 
-			// Remove empty row after merging.
-			if ( !removedTableCellRow.childCount ) {
-				const tableUtils = this.editor.plugins.get( 'TableUtils' );
-				const table = findAncestor( 'table', removedTableCellRow );
+			const tableUtils = this.editor.plugins.get( 'TableUtils' );
+			const table = findAncestor( 'table', removedTableCellRow );
 
-				tableUtils.removeRows( table, { at: removedTableCellRow.index, batch: writer.batch } );
-			}
+			// Remove empty rows and columns after merging.
+			removeEmptyRowsColumns( table, tableUtils, writer.batch );
 		} );
 	}
 

+ 4 - 12
packages/ckeditor5-table/src/commands/mergecellscommand.js

@@ -11,6 +11,7 @@ import Command from '@ckeditor/ckeditor5-core/src/command';
 import TableUtils from '../tableutils';
 import { getSelectedTableCells, isSelectionRectangular } from '../utils/selection';
 import { findAncestor, updateNumericAttribute } from '../utils/common';
+import { removeEmptyRowsColumns } from '../utils/structure';
 
 /**
  * The merge cells command.
@@ -57,23 +58,14 @@ export default class MergeCellsCommand extends Command {
 			updateNumericAttribute( 'colspan', mergeWidth, firstTableCell, writer );
 			updateNumericAttribute( 'rowspan', mergeHeight, firstTableCell, writer );
 
-			const emptyRowsIndexes = [];
-
 			for ( const tableCell of selectedTableCells ) {
-				const tableRow = tableCell.parent;
-
 				mergeTableCells( tableCell, firstTableCell, writer );
-
-				if ( !tableRow.childCount ) {
-					emptyRowsIndexes.push( tableRow.index );
-				}
 			}
 
-			if ( emptyRowsIndexes.length ) {
-				const table = findAncestor( 'table', firstTableCell );
+			const table = findAncestor( 'table', firstTableCell );
 
-				emptyRowsIndexes.reverse().forEach( row => tableUtils.removeRows( table, { at: row, batch: writer.batch } ) );
-			}
+			// Remove rows and columns that become empty (have no anchored cells).
+			removeEmptyRowsColumns( table, tableUtils, writer.batch );
 
 			writer.setSelection( firstTableCell, 'in' );
 		} );

+ 106 - 101
packages/ckeditor5-table/src/tableclipboard.js

@@ -130,66 +130,18 @@ export default class TableClipboard extends Plugin {
 		evt.stop();
 
 		model.change( writer => {
-			const columnIndexes = getColumnIndexes( selectedTableCells );
-			const rowIndexes = getRowIndexes( selectedTableCells );
-
-			let { first: firstColumnOfSelection, last: lastColumnOfSelection } = columnIndexes;
-			let { first: firstRowOfSelection, last: lastRowOfSelection } = rowIndexes;
-
-			const pasteHeight = tableUtils.getRows( pastedTable );
-			const pasteWidth = tableUtils.getColumns( pastedTable );
+			const pastedDimensions = {
+				width: tableUtils.getColumns( pastedTable ),
+				height: tableUtils.getRows( pastedTable )
+			};
 
-			// Content table to which we insert a pasted table.
-			const selectedTable = findAncestor( 'table', selectedTableCells[ 0 ] );
-
-			// Single cell selected - expand selection to pasted table dimensions.
-			const shouldExpandSelection = selectedTableCells.length === 1;
-
-			if ( shouldExpandSelection ) {
-				lastRowOfSelection += pasteHeight - 1;
-				lastColumnOfSelection += pasteWidth - 1;
-
-				expandTableSize( selectedTable, lastRowOfSelection + 1, lastColumnOfSelection + 1, writer, tableUtils );
-			}
-
-			// In case of expanding selection we do not reset the selection so in this case we will always try to fix selection
-			// like in the case of a non-rectangular area. This might be fixed by re-setting selected cells array but this shortcut is safe.
-			if ( shouldExpandSelection || !isSelectionRectangular( selectedTableCells, tableUtils ) ) {
-				const splitDimensions = {
-					firstRow: firstRowOfSelection,
-					lastRow: lastRowOfSelection,
-					firstColumn: firstColumnOfSelection,
-					lastColumn: lastColumnOfSelection
-				};
-
-				// For a non-rectangular selection (ie in which some cells sticks out from a virtual selection rectangle) we need to create
-				// a table layout that has a rectangular selection. This will split cells so the selection become rectangular.
-				// Beyond this point we will operate on fixed content table.
-				splitCellsToRectangularSelection( selectedTable, splitDimensions, writer );
-			}
-			// However a selected table fragment might be invalid if examined alone. Ie such table fragment:
-			//
-			//    +---+---+---+---+
-			//  0 | a | b | c | d |
-			//    +   +   +---+---+
-			//  1 |   | e | f | g |
-			//    +   +---+   +---+
-			//  2 |   | h |   | i | <- last row, each cell has rowspan = 2,
-			//    +   +   +   +   +    so we need to return 3, not 2
-			//  3 |   |   |   |   |
-			//    +---+---+---+---+
-			//
-			// is invalid as the cells "h" and "i" have rowspans.
-			// This case needs only adjusting the selection dimension as the rest of the algorithm operates on empty slots also.
-			else {
-				lastRowOfSelection = adjustLastRowIndex( selectedTable, rowIndexes, columnIndexes );
-				lastColumnOfSelection = adjustLastColumnIndex( selectedTable, rowIndexes, columnIndexes );
-			}
+			// Prepare the table for pasting.
+			const selection = prepareTableForPasting( selectedTableCells, pastedDimensions, writer, tableUtils );
 
 			// Beyond this point we operate on a fixed content table with rectangular selection and proper last row/column values.
 
-			const selectionHeight = lastRowOfSelection - firstRowOfSelection + 1;
-			const selectionWidth = lastColumnOfSelection - firstColumnOfSelection + 1;
+			const selectionHeight = selection.lastRow - selection.firstRow + 1;
+			const selectionWidth = selection.lastColumn - selection.firstColumn + 1;
 
 			// Crop pasted table if:
 			// - Pasted table dimensions exceeds selection area.
@@ -201,28 +153,86 @@ export default class TableClipboard extends Plugin {
 			const cropDimensions = {
 				startRow: 0,
 				startColumn: 0,
-				endRow: Math.min( selectionHeight - 1, pasteHeight - 1 ),
-				endColumn: Math.min( selectionWidth - 1, pasteWidth - 1 )
+				endRow: Math.min( selectionHeight, pastedDimensions.height ) - 1,
+				endColumn: Math.min( selectionWidth, pastedDimensions.width ) - 1
 			};
 
 			pastedTable = cropTableToDimensions( pastedTable, cropDimensions, writer );
 
-			const pastedDimensions = {
-				width: pasteWidth,
-				height: pasteHeight
-			};
-			const selectionDimensions = {
-				firstColumnOfSelection,
-				firstRowOfSelection,
-				lastColumnOfSelection,
-				lastRowOfSelection
-			};
+			// Content table to which we insert a pasted table.
+			const selectedTable = findAncestor( 'table', selectedTableCells[ 0 ] );
 
-			replaceSelectedCellsWithPasted( pastedTable, pastedDimensions, selectedTable, selectionDimensions, writer );
+			replaceSelectedCellsWithPasted( pastedTable, pastedDimensions, selectedTable, selection, writer );
 		} );
 	}
 }
 
+// Prepares a table for pasting and returns adjusted selection dimensions.
+//
+// @param {Array.<module:engine/model/element~Element>} selectedTableCells
+// @param {Object} pastedDimensions
+// @param {Number} pastedDimensions.height
+// @param {Number} pastedDimensions.width
+// @param {module:engine/model/writer~Writer} writer
+// @param {module:table/tableutils~TableUtils} tableUtils
+// @returns {Object} selection
+// @returns {Number} selection.firstColumn
+// @returns {Number} selection.firstRow
+// @returns {Number} selection.lastColumn
+// @returns {Number} selection.lastRow
+function prepareTableForPasting( selectedTableCells, pastedDimensions, writer, tableUtils ) {
+	const selectedTable = findAncestor( 'table', selectedTableCells[ 0 ] );
+
+	const columnIndexes = getColumnIndexes( selectedTableCells );
+	const rowIndexes = getRowIndexes( selectedTableCells );
+
+	const selection = {
+		firstColumn: columnIndexes.first,
+		lastColumn: columnIndexes.last,
+		firstRow: rowIndexes.first,
+		lastRow: rowIndexes.last
+	};
+
+	// Single cell selected - expand selection to pasted table dimensions.
+	const shouldExpandSelection = selectedTableCells.length === 1;
+
+	if ( shouldExpandSelection ) {
+		selection.lastRow += pastedDimensions.height - 1;
+		selection.lastColumn += pastedDimensions.width - 1;
+
+		expandTableSize( selectedTable, selection.lastRow + 1, selection.lastColumn + 1, writer, tableUtils );
+	}
+
+	// In case of expanding selection we do not reset the selection so in this case we will always try to fix selection
+	// like in the case of a non-rectangular area. This might be fixed by re-setting selected cells array but this shortcut is safe.
+	if ( shouldExpandSelection || !isSelectionRectangular( selectedTableCells, tableUtils ) ) {
+		// For a non-rectangular selection (ie in which some cells sticks out from a virtual selection rectangle) we need to create
+		// a table layout that has a rectangular selection. This will split cells so the selection become rectangular.
+		// Beyond this point we will operate on fixed content table.
+		splitCellsToRectangularSelection( selectedTable, selection, writer );
+	}
+	// However a selected table fragment might be invalid if examined alone. Ie such table fragment:
+	//
+	//    +---+---+---+---+
+	//  0 | a | b | c | d |
+	//    +   +   +---+---+
+	//  1 |   | e | f | g |
+	//    +   +---+   +---+
+	//  2 |   | h |   | i | <- last row, each cell has rowspan = 2,
+	//    +   +   +   +   +    so we need to return 3, not 2
+	//  3 |   |   |   |   |
+	//    +---+---+---+---+
+	//
+	// is invalid as the cells "h" and "i" have rowspans.
+	// This case needs only adjusting the selection dimension as the rest of the algorithm operates on empty slots also.
+	else {
+		selection.lastRow = adjustLastRowIndex( selectedTable, selection );
+		selection.lastColumn = adjustLastColumnIndex( selectedTable, selection );
+	}
+
+	return selection;
+}
+
 // Replaces the part of selectedTable with pastedTable.
 //
 // @param {module:engine/model/element~Element} pastedTable
@@ -230,28 +240,23 @@ export default class TableClipboard extends Plugin {
 // @param {Number} pastedDimensions.height
 // @param {Number} pastedDimensions.width
 // @param {module:engine/model/element~Element} selectedTable
-// @param {Object} selectionDimensions
-// @param {Number} selectionDimensions.firstColumnOfSelection
-// @param {Number} selectionDimensions.firstRowOfSelection
-// @param {Number} selectionDimensions.lastColumnOfSelection
-// @param {Number} selectionDimensions.lastRowOfSelection
+// @param {Object} selection
+// @param {Number} selection.firstColumn
+// @param {Number} selection.firstRow
+// @param {Number} selection.lastColumn
+// @param {Number} selection.lastRow
 // @param {module:engine/model/writer~Writer} writer
-function replaceSelectedCellsWithPasted( pastedTable, pastedDimensions, selectedTable, selectionDimensions, writer ) {
-	const {
-		firstColumnOfSelection, lastColumnOfSelection,
-		firstRowOfSelection, lastRowOfSelection
-	} = selectionDimensions;
-
+function replaceSelectedCellsWithPasted( pastedTable, pastedDimensions, selectedTable, selection, writer ) {
 	const { width: pastedWidth, height: pastedHeight } = pastedDimensions;
 
 	// Holds two-dimensional array that is addressed by [ row ][ column ] that stores cells anchored at given location.
 	const pastedTableLocationMap = createLocationMap( pastedTable, pastedWidth, pastedHeight );
 
 	const selectedTableMap = [ ...new TableWalker( selectedTable, {
-		startRow: firstRowOfSelection,
-		endRow: lastRowOfSelection,
-		startColumn: firstColumnOfSelection,
-		endColumn: lastColumnOfSelection,
+		startRow: selection.firstRow,
+		endRow: selection.lastRow,
+		startColumn: selection.firstColumn,
+		endColumn: selection.lastColumn,
 		includeAllSlots: true
 	} ) ];
 
@@ -271,7 +276,7 @@ function replaceSelectedCellsWithPasted( pastedTable, pastedDimensions, selected
 		const { row, column, cell, isAnchor } = tableSlot;
 
 		// Save the insert position for current row start.
-		if ( column === firstColumnOfSelection ) {
+		if ( column === selection.firstColumn ) {
 			insertPosition = tableSlot.getPositionBefore();
 		}
 
@@ -284,8 +289,8 @@ function replaceSelectedCellsWithPasted( pastedTable, pastedDimensions, selected
 		}
 
 		// Map current table slot location to an pasted table slot location.
-		const pastedRow = row - firstRowOfSelection;
-		const pastedColumn = column - firstColumnOfSelection;
+		const pastedRow = row - selection.firstRow;
+		const pastedColumn = column - selection.firstColumn;
 		const pastedCell = pastedTableLocationMap[ pastedRow % pastedHeight ][ pastedColumn % pastedWidth ];
 
 		// There is no cell to insert (might be spanned by other cell in a pasted table) - advance to the next content table slot.
@@ -295,10 +300,10 @@ function replaceSelectedCellsWithPasted( pastedTable, pastedDimensions, selected
 
 		// Clone cell to insert (to duplicate its attributes and children).
 		// Cloning is required to support repeating pasted table content when inserting to a bigger selection.
-		const cellToInsert = pastedCell._clone( true );
+		const cellToInsert = writer.cloneElement( pastedCell );
 
 		// Trim the cell if it's row/col-spans would exceed selection area.
-		trimTableCellIfNeeded( cellToInsert, row, column, lastRowOfSelection, lastColumnOfSelection, writer );
+		trimTableCellIfNeeded( cellToInsert, row, column, selection.lastRow, selection.lastColumn, writer );
 
 		writer.insert( cellToInsert, insertPosition );
 		cellsToSelect.push( cellToInsert );
@@ -498,23 +503,23 @@ function isAffectedBySelection( index, span, limit ) {
 //    +   +   +   +   +    so we need to return 3, not 2
 //  3 |   |   |   |   |
 //    +---+---+---+---+
-function adjustLastRowIndex( table, rowIndexes, columnIndexes ) {
+function adjustLastRowIndex( table, dimensions ) {
 	const lastRowMap = Array.from( new TableWalker( table, {
-		startColumn: columnIndexes.first,
-		endColumn: columnIndexes.last,
-		row: rowIndexes.last
+		startColumn: dimensions.firstColumn,
+		endColumn: dimensions.lastColumn,
+		row: dimensions.lastRow
 	} ) );
 
 	const everyCellHasSingleRowspan = lastRowMap.every( ( { cellHeight } ) => cellHeight === 1 );
 
 	// It is a "flat" row, so the last row index is OK.
 	if ( everyCellHasSingleRowspan ) {
-		return rowIndexes.last;
+		return dimensions.lastRow;
 	}
 
 	// Otherwise get any cell's rowspan and adjust the last row index.
 	const rowspanAdjustment = lastRowMap[ 0 ].cellHeight - 1;
-	return rowIndexes.last + rowspanAdjustment;
+	return dimensions.lastRow + rowspanAdjustment;
 }
 
 // Returns adjusted last column index if selection covers part of a column with empty slots (spanned by other cells).
@@ -534,21 +539,21 @@ function adjustLastRowIndex( table, rowIndexes, columnIndexes ) {
 // +---+---+---+---+
 //           ^
 //          last column, each cell has colspan = 2, so we need to return 3, not 2
-function adjustLastColumnIndex( table, rowIndexes, columnIndexes ) {
+function adjustLastColumnIndex( table, dimensions ) {
 	const lastColumnMap = Array.from( new TableWalker( table, {
-		startRow: rowIndexes.first,
-		endRow: rowIndexes.last,
-		column: columnIndexes.last
+		startRow: dimensions.firstRow,
+		endRow: dimensions.lastRow,
+		column: dimensions.lastColumn
 	} ) );
 
 	const everyCellHasSingleColspan = lastColumnMap.every( ( { cellWidth } ) => cellWidth === 1 );
 
 	// It is a "flat" column, so the last column index is OK.
 	if ( everyCellHasSingleColspan ) {
-		return columnIndexes.last;
+		return dimensions.lastColumn;
 	}
 
 	// Otherwise get any cell's colspan and adjust the last column index.
 	const colspanAdjustment = lastColumnMap[ 0 ].cellWidth - 1;
-	return columnIndexes.last + colspanAdjustment;
+	return dimensions.lastColumn + colspanAdjustment;
 }

+ 7 - 12
packages/ckeditor5-table/src/tableutils.js

@@ -11,6 +11,7 @@ import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
 
 import TableWalker from './tablewalker';
 import { createEmptyTableCell, updateNumericAttribute } from './utils/common';
+import { removeEmptyColumns, removeEmptyRows } from './utils/structure';
 
 /**
  * The table utilities plugin.
@@ -336,7 +337,10 @@ export default class TableUtils extends Plugin {
 				updateNumericAttribute( 'rowspan', rowspan, cell, writer );
 			}
 
-			// 2d. Adjust heading rows if removed rows were in a heading section.
+			// 2d. Remove empty columns (without anchored cells) if there are any.
+			removeEmptyColumns( table, this );
+
+			// 2e. Adjust heading rows if removed rows were in a heading section.
 			updateHeadingRows( table, first, last, model, batch );
 		} );
 	}
@@ -379,29 +383,20 @@ export default class TableUtils extends Plugin {
 		model.change( writer => {
 			adjustHeadingColumns( table, { first, last }, writer );
 
-			const emptyRowsIndexes = [];
-
 			for ( let removedColumnIndex = last; removedColumnIndex >= first; removedColumnIndex-- ) {
 				for ( const { cell, column, cellWidth } of [ ...new TableWalker( table ) ] ) {
 					// If colspaned cell overlaps removed column decrease its span.
 					if ( column <= removedColumnIndex && cellWidth > 1 && column + cellWidth > removedColumnIndex ) {
 						updateNumericAttribute( 'colspan', cellWidth - 1, cell, writer );
 					} else if ( column === removedColumnIndex ) {
-						const cellRow = cell.parent;
-
 						// The cell in removed column has colspan of 1.
 						writer.remove( cell );
-
-						// If the cell was the last one in the row, get rid of the entire row.
-						// https://github.com/ckeditor/ckeditor5/issues/6429
-						if ( !cellRow.childCount ) {
-							emptyRowsIndexes.push( cellRow.index );
-						}
 					}
 				}
 			}
 
-			emptyRowsIndexes.reverse().forEach( row => this.removeRows( table, { at: row, batch: writer.batch } ) );
+			// Remove empty rows that could appear after removing columns.
+			removeEmptyRows( table, this, writer.batch );
 		} );
 	}
 

+ 134 - 1
packages/ckeditor5-table/src/utils/structure.js

@@ -75,7 +75,7 @@ export function cropTableToDimensions( sourceTable, cropDimensions, writer ) {
 		}
 		// Otherwise clone the cell with all children and trim if it exceeds cropped area.
 		else {
-			const tableCellCopy = tableCell._clone( true );
+			const tableCellCopy = writer.cloneElement( tableCell );
 
 			writer.append( tableCellCopy, row );
 
@@ -305,3 +305,136 @@ function addHeadingsToCroppedTable( croppedTable, sourceTable, startRow, startCo
 		updateNumericAttribute( 'headingColumns', headingColumnsInCrop, croppedTable, writer, 0 );
 	}
 }
+
+/**
+ * Removes columns that have no cells anchored.
+ *
+ * In table below:
+ *
+ *     +----+----+----+----+----+----+----+
+ *     | 00 | 01      | 03 | 04      | 06 |
+ *     +----+----+----+----+         +----+
+ *     | 10 | 11      | 13 |         | 16 |
+ *     +----+----+----+----+----+----+----+
+ *     | 20 | 21      | 23 | 24      | 26 |
+ *     +----+----+----+----+----+----+----+
+ *                  ^--- empty ---^
+ *
+ * Will remove columns 2 and 5.
+ *
+ * **Note:** This is a low-level helper method for clearing invalid model state when doing table modifications.
+ * To remove a column from a table use {@link module:table/tableutils~TableUtils#removeColumns `TableUtils.removeColumns()`}.
+ *
+ * @protected
+ * @param {module:engine/model/element~Element} table
+ * @param {module:table/tableutils~TableUtils} tableUtils
+ * @return {Boolean} True if removed some columns.
+ */
+export function removeEmptyColumns( table, tableUtils ) {
+	const width = tableUtils.getColumns( table );
+	const columnsMap = new Array( width ).fill( 0 );
+
+	for ( const { column } of new TableWalker( table ) ) {
+		columnsMap[ column ]++;
+	}
+
+	const emptyColumns = columnsMap.reduce( ( result, cellsCount, column ) => {
+		return cellsCount ? result : [ ...result, column ];
+	}, [] );
+
+	// @if CK_DEBUG_TABLE // emptyColumns.length > 0 && console.log( `Removing empty columns: ${ emptyColumns.join( ', ' ) }.` );
+
+	emptyColumns.reverse().forEach( column => {
+		tableUtils.removeColumns( table, { at: column } );
+	} );
+
+	return emptyColumns.length > 0;
+}
+
+/**
+ * Removes rows that have no cells anchored.
+ *
+ * In table below:
+ *
+ *     +----+----+----+
+ *     | 00 | 01 | 02 |
+ *     +----+----+----+
+ *     | 10 | 11 | 12 |
+ *     +    +    +    +
+ *     |    |    |    | <-- empty
+ *     +----+----+----+
+ *     | 30 | 31 | 32 |
+ *     +----+----+----+
+ *     | 40      | 42 |
+ *     +         +    +
+ *     |         |    | <-- empty
+ *     +----+----+----+
+ *     | 60 | 61 | 62 |
+ *     +----+----+----+
+ *
+ * Will remove rows 2 and 5.
+ *
+ * **Note:** This is a low-level helper method for clearing invalid model state when doing table modifications.
+ * To remove a row from a table use {@link module:table/tableutils~TableUtils#removeRows `TableUtils.removeRows()`}.
+ *
+ * @protected
+ * @param {module:engine/model/element~Element} table
+ * @param {module:table/tableutils~TableUtils} tableUtils
+ * @param {module:engine/model/batch~Batch|null} [batch] Batch that should be used for removing empty rows.
+ * @return {Boolean} True if removed some rows.
+ */
+export function removeEmptyRows( table, tableUtils, batch ) {
+	const emptyRows = [];
+
+	for ( let rowIndex = 0; rowIndex < table.childCount; rowIndex++ ) {
+		const tableRow = table.getChild( rowIndex );
+
+		if ( tableRow.isEmpty ) {
+			emptyRows.push( rowIndex );
+		}
+	}
+
+	// @if CK_DEBUG_TABLE // emptyRows.length > 0 && console.log( `Removing empty rows: ${ emptyRows.join( ', ' ) }.` );
+
+	emptyRows.reverse().forEach( row => {
+		tableUtils.removeRows( table, { at: row, batch } );
+	} );
+
+	return emptyRows.length > 0;
+}
+
+/**
+ * Removes rows and columns that have no cells anchored.
+ *
+ * In table below:
+ *
+ *     +----+----+----+----+
+ *     | 00      | 02      |
+ *     +----+----+         +
+ *     | 10      |         |
+ *     +----+----+----+----+
+ *     | 20      | 22 | 23 |
+ *     +         +    +    +
+ *     |         |    |    | <-- empty row
+ *     +----+----+----+----+
+ *             ^--- empty column
+ *
+ * Will remove row 3 and column 1.
+ *
+ * **Note:** This is a low-level helper method for clearing invalid model state when doing table modifications.
+ * To remove a rows from a table use {@link module:table/tableutils~TableUtils#removeRows `TableUtils.removeRows()`} and
+ * {@link module:table/tableutils~TableUtils#removeColumns `TableUtils.removeColumns()`} to remove a column.
+ *
+ * @protected
+ * @param {module:engine/model/element~Element} table
+ * @param {module:table/tableutils~TableUtils} tableUtils
+ * @param {module:engine/model/batch~Batch|null} [batch] Batch that should be used for removing empty rows.
+ */
+export function removeEmptyRowsColumns( table, tableUtils, batch ) {
+	const removedColumns = removeEmptyColumns( table, tableUtils );
+
+	// If there was some columns removed then cleaning empty rows was already triggered.
+	if ( !removedColumns ) {
+		removeEmptyRows( table, tableUtils, batch );
+	}
+}

+ 62 - 18
packages/ckeditor5-table/tests/commands/mergecellcommand.js

@@ -189,49 +189,69 @@ describe( 'MergeCellCommand', () => {
 		describe( 'execute()', () => {
 			it( 'should merge table cells', () => {
 				setData( model, modelTable( [
+					[ '[]00', '01' ],
+					[ '10', '11' ]
+				] ) );
+
+				command.execute();
+
+				assertEqualMarkup( getData( model ), modelTable( [
+					[ { colspan: 2, contents: '<paragraph>[00</paragraph><paragraph>01]</paragraph>' } ],
+					[ '10', '11' ]
+				] ) );
+			} );
+
+			it( 'should merge table cells and remove empty columns', () => {
+				setData( model, modelTable( [
 					[ '[]00', '01' ]
 				] ) );
 
 				command.execute();
 
 				assertEqualMarkup( getData( model ), modelTable( [
-					[ { colspan: 2, contents: '<paragraph>[00</paragraph><paragraph>01]</paragraph>' } ]
+					[ '<paragraph>[00</paragraph><paragraph>01]</paragraph>' ]
 				] ) );
 			} );
 
 			it( 'should result in single empty paragraph if both cells are empty', () => {
 				setData( model, modelTable( [
-					[ '[]', '' ]
+					[ '[]', '' ],
+					[ '10', '11' ]
 				] ) );
 
 				command.execute();
 
 				assertEqualMarkup( getData( model ), modelTable( [
-					[ { colspan: 2, contents: '<paragraph>[]</paragraph>' } ]
+					[ { colspan: 2, contents: '<paragraph>[]</paragraph>' } ],
+					[ '10', '11' ]
 				] ) );
 			} );
 
 			it( 'should result in single paragraph (other cell is empty)', () => {
 				setData( model, modelTable( [
-					[ 'foo[]', '' ]
+					[ 'foo[]', '' ],
+					[ '10', '11' ]
 				] ) );
 
 				command.execute();
 
 				assertEqualMarkup( getData( model ), modelTable( [
-					[ { colspan: 2, contents: '<paragraph>[foo]</paragraph>' } ]
+					[ { colspan: 2, contents: '<paragraph>[foo]</paragraph>' } ],
+					[ '10', '11' ]
 				] ) );
 			} );
 
 			it( 'should result in single paragraph (selection cell is empty)', () => {
 				setData( model, modelTable( [
-					[ '[]', 'foo' ]
+					[ '[]', 'foo' ],
+					[ '10', '11' ]
 				] ) );
 
 				command.execute();
 
 				assertEqualMarkup( getData( model ), modelTable( [
-					[ { colspan: 2, contents: '<paragraph>[foo]</paragraph>' } ]
+					[ { colspan: 2, contents: '<paragraph>[foo]</paragraph>' } ],
+					[ '10', '11' ]
 				] ) );
 			} );
 
@@ -243,13 +263,15 @@ describe( 'MergeCellCommand', () => {
 				} );
 
 				setData( model, modelTable( [
-					[ '<block>[]</block>', '<block></block>' ]
+					[ '<block>[]</block>', '<block></block>' ],
+					[ '10', '11' ]
 				] ) );
 
 				command.execute();
 
 				assertEqualMarkup( getData( model ), modelTable( [
-					[ { colspan: 2, contents: '<block>[</block><block>]</block>' } ]
+					[ { colspan: 2, contents: '<block>[</block><block>]</block>' } ],
+					[ '10', '11' ]
 				] ) );
 			} );
 		} );
@@ -403,49 +425,69 @@ describe( 'MergeCellCommand', () => {
 		describe( 'execute()', () => {
 			it( 'should merge table cells', () => {
 				setData( model, modelTable( [
+					[ '00', '[]01' ],
+					[ '10', '11' ]
+				] ) );
+
+				command.execute();
+
+				assertEqualMarkup( getData( model ), modelTable( [
+					[ { colspan: 2, contents: '<paragraph>[00</paragraph><paragraph>01]</paragraph>' } ],
+					[ '10', '11' ]
+				] ) );
+			} );
+
+			it( 'should merge table cells and remove empty columns', () => {
+				setData( model, modelTable( [
 					[ '00', '[]01' ]
 				] ) );
 
 				command.execute();
 
 				assertEqualMarkup( getData( model ), modelTable( [
-					[ { colspan: 2, contents: '<paragraph>[00</paragraph><paragraph>01]</paragraph>' } ]
+					[ '<paragraph>[00</paragraph><paragraph>01]</paragraph>' ]
 				] ) );
 			} );
 
 			it( 'should result in single empty paragraph if both cells are empty', () => {
 				setData( model, modelTable( [
-					[ '', '[]' ]
+					[ '', '[]' ],
+					[ '10', '11' ]
 				] ) );
 
 				command.execute();
 
 				assertEqualMarkup( getData( model ), modelTable( [
-					[ { colspan: 2, contents: '<paragraph>[]</paragraph>' } ]
+					[ { colspan: 2, contents: '<paragraph>[]</paragraph>' } ],
+					[ '10', '11' ]
 				] ) );
 			} );
 
 			it( 'should result in single paragraph (other cell is empty)', () => {
 				setData( model, modelTable( [
-					[ '', 'foo[]' ]
+					[ '', 'foo[]' ],
+					[ '10', '11' ]
 				] ) );
 
 				command.execute();
 
 				assertEqualMarkup( getData( model ), modelTable( [
-					[ { colspan: 2, contents: '<paragraph>[foo]</paragraph>' } ]
+					[ { colspan: 2, contents: '<paragraph>[foo]</paragraph>' } ],
+					[ '10', '11' ]
 				] ) );
 			} );
 
 			it( 'should result in single paragraph (selection cell is empty)', () => {
 				setData( model, modelTable( [
-					[ 'foo', '[]' ]
+					[ 'foo', '[]' ],
+					[ '10', '11' ]
 				] ) );
 
 				command.execute();
 
 				assertEqualMarkup( getData( model ), modelTable( [
-					[ { colspan: 2, contents: '<paragraph>[foo]</paragraph>' } ]
+					[ { colspan: 2, contents: '<paragraph>[foo]</paragraph>' } ],
+					[ '10', '11' ]
 				] ) );
 			} );
 
@@ -457,13 +499,15 @@ describe( 'MergeCellCommand', () => {
 				} );
 
 				setData( model, modelTable( [
-					[ '<block></block>', '<block>[]</block>' ]
+					[ '<block></block>', '<block>[]</block>' ],
+					[ '10', '11' ]
 				] ) );
 
 				command.execute();
 
 				assertEqualMarkup( getData( model ), modelTable( [
-					[ { colspan: 2, contents: '<block>[</block><block>]</block>' } ]
+					[ { colspan: 2, contents: '<block>[</block><block>]</block>' } ],
+					[ '10', '11' ]
 				] ) );
 			} );
 		} );

+ 54 - 11
packages/ckeditor5-table/tests/commands/mergecellscommand.js

@@ -312,6 +312,25 @@ describe( 'MergeCellsCommand', () => {
 	describe( 'execute()', () => {
 		it( 'should merge simple table cell selection', () => {
 			setData( model, modelTable( [
+				[ '[]00', '01' ],
+				[ '10', '11' ]
+			] ) );
+
+			tableSelection.setCellSelection(
+				root.getNodeByPath( [ 0, 0, 0 ] ),
+				root.getNodeByPath( [ 0, 0, 1 ] )
+			);
+
+			command.execute();
+
+			assertEqualMarkup( getData( model ), modelTable( [
+				[ { colspan: 2, contents: '<paragraph>[00</paragraph><paragraph>01]</paragraph>' } ],
+				[ '10', '11' ]
+			] ) );
+		} );
+
+		it( 'should merge simple table cell selection and remove empty columns', () => {
+			setData( model, modelTable( [
 				[ '[]00', '01' ]
 			] ) );
 
@@ -323,7 +342,7 @@ describe( 'MergeCellsCommand', () => {
 			command.execute();
 
 			assertEqualMarkup( getData( model ), modelTable( [
-				[ { colspan: 2, contents: '<paragraph>[00</paragraph><paragraph>01]</paragraph>' } ]
+				[ '<paragraph>[00</paragraph><paragraph>01]</paragraph>' ]
 			] ) );
 		} );
 
@@ -403,9 +422,17 @@ describe( 'MergeCellsCommand', () => {
 		} );
 
 		it( 'should merge table cells - extend colspan attribute', () => {
+			// +----+----+----+----+
+			// | 00      | 02 | 03 |
+			// +----+----+----+----+
+			// | 10 | 11 | 12 | 13 |
+			// +----+----+----+----+
+			// | 20 | 21 | 22 | 23 |
+			// +----+----+----+----+
 			setData( model, modelTable( [
 				[ { colspan: 2, contents: '00' }, '02', '03' ],
-				[ '10', '11', '12', '13' ]
+				[ '10', '11', '12', '13' ],
+				[ '20', '21', '22', '23' ]
 			] ) );
 
 			selectNodes( [
@@ -415,6 +442,13 @@ describe( 'MergeCellsCommand', () => {
 
 			command.execute();
 
+			// +----+----+----+----+
+			// |              | 03 |
+			// +   (merged)   +----+
+			// |              | 13 |
+			// +----+----+----+----+
+			// | 20 | 21 | 22 | 23 |
+			// +----+----+----+----+
 			assertEqualMarkup( getData( model ), modelTable( [
 				[ {
 					colspan: 3,
@@ -425,13 +459,15 @@ describe( 'MergeCellsCommand', () => {
 						'<paragraph>11</paragraph>' +
 						'<paragraph>12]</paragraph>'
 				}, '03' ],
-				[ '13' ]
+				[ '13' ],
+				[ '20', '21', '22', '23' ]
 			] ) );
 		} );
 
 		it( 'should merge to a single paragraph - every cell is empty', () => {
 			setData( model, modelTable( [
-				[ '[]', '' ]
+				[ '[]', '' ],
+				[ '10', '11' ]
 			] ) );
 
 			selectNodes( [ [ 0, 0, 0 ], [ 0, 0, 1 ] ] );
@@ -439,13 +475,15 @@ describe( 'MergeCellsCommand', () => {
 			command.execute();
 
 			assertEqualMarkup( getData( model ), modelTable( [
-				[ { colspan: 2, contents: '<paragraph>[]</paragraph>' } ]
+				[ { colspan: 2, contents: '<paragraph>[]</paragraph>' } ],
+				[ '10', '11' ]
 			] ) );
 		} );
 
 		it( 'should merge to a single paragraph - merged cell is empty', () => {
 			setData( model, modelTable( [
-				[ 'foo', '' ]
+				[ 'foo', '' ],
+				[ '10', '11' ]
 			] ) );
 
 			selectNodes( [ [ 0, 0, 0 ], [ 0, 0, 1 ] ] );
@@ -453,13 +491,15 @@ describe( 'MergeCellsCommand', () => {
 			command.execute();
 
 			assertEqualMarkup( getData( model ), modelTable( [
-				[ { colspan: 2, contents: '<paragraph>[foo]</paragraph>' } ]
+				[ { colspan: 2, contents: '<paragraph>[foo]</paragraph>' } ],
+				[ '10', '11' ]
 			] ) );
 		} );
 
 		it( 'should merge to a single paragraph - cell to which others are merged is empty', () => {
 			setData( model, modelTable( [
-				[ '', 'foo' ]
+				[ '', 'foo' ],
+				[ '10', '11' ]
 			] ) );
 
 			selectNodes( [ [ 0, 0, 0 ], [ 0, 0, 1 ] ] );
@@ -467,7 +507,8 @@ describe( 'MergeCellsCommand', () => {
 			command.execute();
 
 			assertEqualMarkup( getData( model ), modelTable( [
-				[ { colspan: 2, contents: '<paragraph>[foo]</paragraph>' } ]
+				[ { colspan: 2, contents: '<paragraph>[foo]</paragraph>' } ],
+				[ '10', '11' ]
 			] ) );
 		} );
 
@@ -479,7 +520,8 @@ describe( 'MergeCellsCommand', () => {
 			} );
 
 			setData( model, modelTable( [
-				[ '<block>[]</block>', '<block></block>' ]
+				[ '<block>[]</block>', '<block></block>' ],
+				[ '10', '11' ]
 			] ) );
 
 			selectNodes( [ [ 0, 0, 0 ], [ 0, 0, 1 ] ] );
@@ -487,7 +529,8 @@ describe( 'MergeCellsCommand', () => {
 			command.execute();
 
 			assertEqualMarkup( getData( model ), modelTable( [
-				[ { colspan: 2, contents: '<block>[</block><block>]</block>' } ]
+				[ { colspan: 2, contents: '<block>[</block><block>]</block>' } ],
+				[ '10', '11' ]
 			] ) );
 		} );
 

+ 13 - 0
packages/ckeditor5-table/tests/commands/removerowcommand.js

@@ -535,5 +535,18 @@ describe( 'RemoveRowCommand', () => {
 				[ '30', '31', '32' ]
 			] ) );
 		} );
+
+		it( 'should remove empty columns after removing row', () => {
+			setData( model, modelTable( [
+				[ '00', { contents: '01', colspan: 2 } ],
+				[ '[]10', '11', '12' ]
+			] ) );
+
+			command.execute();
+
+			assertEqualMarkup( getData( model ), modelTable( [
+				[ '[]00', '01' ]
+			] ) );
+		} );
 	} );
 } );

+ 12 - 2
packages/ckeditor5-table/tests/manual/tablemocking.html

@@ -25,9 +25,19 @@
 	#input-status {
 		color: hsl( 0, 90%, 50% );
 	}
+	#tools {
+		margin-bottom: 10px;
+	}
+	#tools input[type="checkbox"] {
+		vertical-align: middle;
+	}
+	#tools label > span {
+		vertical-align: middle;
+		display: inline-block;
+	}
 </style>
 
-<div style="margin-bottom: 10px;">
+<div id="tools">
 	<label for="model-data">Table data as expected by <a href="https://github.com/ckeditor/ckeditor5-table/blob/1004f9106110be9de125825afd491a1618b71271/tests/_utils/utils.js#L48">modelTable</a> helper function:</label>
 	<textarea id="model-data">
 [
@@ -43,7 +53,7 @@
 	<button type="button" id="set-model-data">↓ Set model data ↓</button>
 	<button type="button" id="get-model-data">↑ Get model data ↑</button>
 	<button type="button" id="renumber-cells">Renumber cells</button>
-	<label><input type="checkbox" id="use-letters" checked="false">Use letters</label>
+	<label><input type="checkbox" id="use-letters" checked="false"><span>Use letters</span></label>
 
 	<span id="input-status"></span>
 </div>

+ 8 - 3
packages/ckeditor5-table/tests/tableselection-integration.js

@@ -229,7 +229,10 @@ describe( 'TableSelection - integration', () => {
 
 		// See https://github.com/ckeditor/ckeditor5/issues/6634.
 		it( 'works with merge cells command', () => {
-			setModelData( editor.model, modelTable( [ [ '00', '01' ] ] ) );
+			setModelData( editor.model, modelTable( [
+				[ '00', '01' ],
+				[ '10', '11' ]
+			] ) );
 
 			tableSelection.setCellSelection(
 				modelRoot.getNodeByPath( [ 0, 0, 0 ] ),
@@ -239,13 +242,15 @@ describe( 'TableSelection - integration', () => {
 			editor.execute( 'mergeTableCells' );
 
 			assertEqualMarkup( getModelData( model ), modelTable( [
-				[ { colspan: 2, contents: '<paragraph>[00</paragraph><paragraph>01]</paragraph>' } ]
+				[ { colspan: 2, contents: '<paragraph>[00</paragraph><paragraph>01]</paragraph>' } ],
+				[ '10', '11' ]
 			] ) );
 
 			editor.execute( 'undo' );
 
 			assertEqualMarkup( getModelData( model ), modelTable( [
-				[ '[]00', '01' ]
+				[ '[]00', '01' ],
+				[ '10', '11' ]
 			] ) );
 		} );
 	} );