Fix unable to remove empty blocks on merge (#65262)

Co-authored-by: kevin940726 <[email protected]>
Co-authored-by: ntsekouras <[email protected]>
Co-authored-by: talldan <[email protected]>
Co-authored-by: andrewserong <[email protected]>
Co-authored-by: youknowriad <[email protected]>
Co-authored-by: getdave <[email protected]>
Co-authored-by: Mamaduka <[email protected]>
Co-authored-by: ramonjd <[email protected]>
Co-authored-by: kspilarski <[email protected]>
Co-authored-by: ndiego <[email protected]>
Co-authored-by: richtabor <[email protected]>
* Fix unable to remove empty blocks on merge

* Update to the stabilized API

* Rename the utils

packages/block-editor/src/components/block-list/block.js

@@ -25,6 +25,7 @@ import {
 	getBlockDefaultClassName,
 	hasBlockSupport,
 	store as blocksStore,
+	privateApis as blocksPrivateApis,
 } from '@wordpress/blocks';
 import { withFilters } from '@wordpress/components';
 import { withDispatch, useDispatch, useSelect } from '@wordpress/data';
@@ -46,6 +47,8 @@ import { PrivateBlockContext } from './private-block-context';
 
 import { unlock } from '../../lock-unlock';
 
+const { isUnmodifiedBlockContent } = unlock( blocksPrivateApis );
+
 /**
  * Merges wrapper props with special handling for classNames and styles.
  *
@@ -350,34 +353,66 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps, registry ) => {
 					removeBlock( _clientId );
 				} else {
 					registry.batch( () => {
+						const firstBlock = getBlock( firstClientId );
+						const isFirstBlockContentUnmodified =
+							isUnmodifiedBlockContent( firstBlock );
+						const defaultBlockName = getDefaultBlockName();
+						const replacement = switchToBlockType(
+							firstBlock,
+							defaultBlockName
+						);
+						const canTransformToDefaultBlock =
+							!! replacement?.length &&
+							replacement.every( ( block ) =>
+								canInsertBlockType( block.name, _clientId )
+							);
+
 						if (
+							isFirstBlockContentUnmodified &&
+							canTransformToDefaultBlock
+						) {
+							// Step 1: If the block is empty and can be transformed to the default block type.
+							replaceBlocks(
+								firstClientId,
+								replacement,
+								changeSelection
+							);
+						} else if (
+							isFirstBlockContentUnmodified &&
+							firstBlock.name === defaultBlockName
+						) {
+							// Step 2: If the block is empty and is already the default block type.
+							removeBlock( firstClientId );
+							const nextBlockClientId =
+								getNextBlockClientId( clientId );
+							if ( nextBlockClientId ) {
+								selectBlock( nextBlockClientId );
+							}
+						} else if (
 							canInsertBlockType(
-								getBlockName( firstClientId ),
+								firstBlock.name,
 								targetRootClientId
 							)
 						) {
+							// Step 3: If the block can be moved up.
 							moveBlocksToPosition(
 								[ firstClientId ],
 								_clientId,
 								targetRootClientId,
 								getBlockIndex( _clientId )
 							);
 						} else {
-							const replacement = switchToBlockType(
-								getBlock( firstClientId ),
-								getDefaultBlockName()
-							);
-
-							if (
-								replacement &&
-								replacement.length &&
+							const canLiftAndTransformToDefaultBlock =
+								!! replacement?.length &&
 								replacement.every( ( block ) =>
 									canInsertBlockType(
 										block.name,
 										targetRootClientId
 									)
-								)
-							) {
+								);
+
+							if ( canLiftAndTransformToDefaultBlock ) {
+								// Step 4: If the block can be transformed to the default block type and moved up.
 								insertBlocks(
 									replacement,
 									getBlockIndex( _clientId ),
@@ -386,6 +421,7 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps, registry ) => {
 								);
 								removeBlock( firstClientId, false );
 							} else {
+								// Step 5: Continue the default behavior.
 								switchToDefaultOrRemove();
 							}
 						}

packages/blocks/src/api/index.js

@@ -8,6 +8,7 @@ import {
 	getBlockBindingsSource,
 	getBlockBindingsSources,
 } from './registration';
+import { isUnmodifiedBlockContent } from './utils';
 
 // The blocktype is the most important concept within the block API. It defines
 // all aspects of the block configuration and its interfaces, including `edit`
@@ -182,4 +183,5 @@ lock( privateApis, {
 	unregisterBlockBindingsSource,
 	getBlockBindingsSource,
 	getBlockBindingsSources,
+	isUnmodifiedBlockContent,
 } );

packages/blocks/src/api/utils.js

@@ -29,6 +29,30 @@ extend( [ namesPlugin, a11yPlugin ] );
  */
 const ICON_COLORS = [ '#191e23', '#f8f9f9' ];
 
+/**
+ * Determines whether the block's attribute is equal to the default attribute
+ * which means the attribute is unmodified.
+ * @param {Object} attributeDefinition The attribute's definition of the block type.
+ * @param {*}      value               The attribute's value.
+ * @return {boolean} Whether the attribute is unmodified.
+ */
+function isUnmodifiedAttribute( attributeDefinition, value ) {
+	// Every attribute that has a default must match the default.
+	if ( attributeDefinition.hasOwnProperty( 'default' ) ) {
+		return value === attributeDefinition.default;
+	}
+
+	// The rich text type is a bit different from the rest because it
+	// has an implicit default value of an empty RichTextData instance,
+	// so check the length of the value.
+	if ( attributeDefinition.type === 'rich-text' ) {
+		return ! value?.length;
+	}
+
+	// Every attribute that doesn't have a default should be undefined.
+	return value === undefined;
+}
+
 /**
  * Determines whether the block's attributes are equal to the default attributes
  * which means the block is unmodified.
@@ -42,20 +66,7 @@ export function isUnmodifiedBlock( block ) {
 		( [ key, definition ] ) => {
 			const value = block.attributes[ key ];
 
-			// Every attribute that has a default must match the default.
-			if ( definition.hasOwnProperty( 'default' ) ) {
-				return value === definition.default;
-			}
-
-			// The rich text type is a bit different from the rest because it
-			// has an implicit default value of an empty RichTextData instance,
-			// so check the length of the value.
-			if ( definition.type === 'rich-text' ) {
-				return ! value?.length;
-			}
-
-			// Every attribute that doesn't have a default should be undefined.
-			return value === undefined;
+			return isUnmodifiedAttribute( definition, value );
 		}
 	);
 }
@@ -72,6 +83,35 @@ export function isUnmodifiedDefaultBlock( block ) {
 	return block.name === getDefaultBlockName() && isUnmodifiedBlock( block );
 }
 
+/**
+ * Determines whether the block content is unmodified. A block content is
+ * considered unmodified if all the attributes that have a role of 'content'
+ * are equal to the default attributes (or undefined).
+ * If the block does not have any attributes with a role of 'content', it
+ * will be considered unmodified if all the attributes are equal to the default
+ * attributes (or undefined).
+ *
+ * @param {WPBlock} block Block Object
+ * @return {boolean} Whether the block content is unmodified.
+ */
+export function isUnmodifiedBlockContent( block ) {
+	const contentAttributes = getBlockAttributesNamesByRole(
+		block.name,
+		'content'
+	);
+
+	if ( contentAttributes.length === 0 ) {
+		return isUnmodifiedBlock( block );
+	}
+
+	return contentAttributes.every( ( key ) => {
+		const definition = getBlockType( block.name )?.attributes[ key ];
+		const value = block.attributes[ key ];
+
+		return isUnmodifiedAttribute( definition, value );
+	} );
+}
+
 /**
  * Function that checks if the parameter is a valid icon.
  *

test/e2e/specs/editor/various/splitting-merging.spec.js

@@ -373,6 +373,103 @@ test.describe( 'splitting and merging blocks (@firefox, @webkit)', () => {
 		);
 	} );
 
+	// Fix for https://github.com/WordPress/gutenberg/issues/65174.
+	test( 'should handle unwrapping and merging blocks with empty contents', async ( {
+		editor,
+		page,
+	} ) => {
+		const emptyAlignedParagraph = {
+			name: 'core/paragraph',
+			attributes: { content: '', align: 'center', dropCap: false },
+			innerBlocks: [],
+		};
+		const emptyAlignedHeading = {
+			name: 'core/heading',
+			attributes: { content: '', textAlign: 'center', level: 2 },
+			innerBlocks: [],
+		};
+		const headingWithContent = {
+			name: 'core/heading',
+			attributes: { content: 'heading', level: 2 },
+			innerBlocks: [],
+		};
+		const placeholderBlock = { name: 'core/separator' };
+		await editor.insertBlock( {
+			name: 'core/group',
+			innerBlocks: [
+				emptyAlignedParagraph,
+				emptyAlignedHeading,
+				headingWithContent,
+				placeholderBlock,
+			],
+		} );
+		await editor.canvas
+			.getByRole( 'document', { name: 'Empty block' } )
+			.focus();
+
+		await page.keyboard.press( 'Backspace' );
+		await expect
+			.poll( editor.getBlocks, 'Remove the default empty block' )
+			.toEqual( [
+				{
+					name: 'core/group',
+					attributes: { tagName: 'div' },
+					innerBlocks: [
+						emptyAlignedHeading,
+						headingWithContent,
+						expect.objectContaining( placeholderBlock ),
+					],
+				},
+			] );
+
+		// Move the caret to the beginning of the empty heading block.
+		await page.keyboard.press( 'ArrowDown' );
+		await page.keyboard.press( 'Backspace' );
+		await expect
+			.poll(
+				editor.getBlocks,
+				'Convert the non-default empty block to a default block'
+			)
+			.toEqual( [
+				{
+					name: 'core/group',
+					attributes: { tagName: 'div' },
+					innerBlocks: [
+						emptyAlignedParagraph,
+						headingWithContent,
+						expect.objectContaining( placeholderBlock ),
+					],
+				},
+			] );
+		await page.keyboard.press( 'Backspace' );
+		await expect.poll( editor.getBlocks ).toEqual( [
+			{
+				name: 'core/group',
+				attributes: { tagName: 'div' },
+				innerBlocks: [
+					headingWithContent,
+					expect.objectContaining( placeholderBlock ),
+				],
+			},
+		] );
+
+		// Move the caret to the beginning of the "heading" heading block.
+		await page.keyboard.press( 'ArrowDown' );
+		await page.keyboard.press( 'Backspace' );
+		await expect
+			.poll( editor.getBlocks, 'Lift the non-empty non-default block' )
+			.toEqual( [
+				headingWithContent,
+				{
+					name: 'core/group',
+					attributes: { tagName: 'div' },
+					innerBlocks: [
+						expect.objectContaining( placeholderBlock ),
+					],
+				},
+			] );
+	} );
+
 	test.describe( 'test restore selection when merge produces more than one block', () => {
 		const snap1 = [
 			{