Fix copy paste and delete error core paragraph with locking

[jorgefilipecosta]

Description

Fixes: #12774

Rich text is able to handle the situation of onReplace and onRemove not existing.
When locking exist onReplace prop is not passed to blocks.
Previously onReplace prop was passed directly passed from paragraph to rich text. Now that's not the case and we have a middle function that may call the onReplace prop. If the onReplace property is not set (e.g we have locking) we should not pass the onReplace to rich text.

How has this been tested?

I added the following CPT with locking:

function gutenberg_test_cpt_locking() {
	register_post_type(
		'locked-all-post',
		array(
			'public'        => true,
			'label'         => 'Locked All Post',
			'show_in_rest'  => true,
			'template'      => $template,
			'template_lock' => 'all',
		)
	);
	register_post_type(
		'locked-insert-post',
		array(
			'public'        => true,
			'label'         => 'Locked Insert Post',
			'show_in_rest'  => true,
			'template'      => $template,
			'template_lock' => 'insert',
		)
	);
}

add_action( 'init', 'gutenberg_test_cpt_locking' );

I verified I can paste in the paragraph.
I verified I can type something and delete all the contents again, and when I press backspace when the paragraph is empty no errors happen.
I repeated the same tests for the paragraph inside the following block:
( function() {
const { registerBlockType } = wp.blocks;
const { createElement: el } = wp.element;
const { InnerBlocks } = wp.editor;

registerBlockType( 'acme/product', {
	title: 'Product',
	icon: 'carrot',
	category: 'common',

	edit() {
		return el( 'div', { className: 'product', style: { outline: '1px solid gray', padding: 5 } },
			// Only paragraphs, images, and products:
			el(
                InnerBlocks,
                { template: [
                    [ 'core/heading', { placeholder: 'Slide heading...', level: 2 } ],
                    [ 'core/paragraph', { placeholder: 'Slide content...' } ],
                    [ 'core/button', { text: 'Read More', className: 'btn-lg' } ],
                ],
                templateLock: 'all'
                }
            )
			// Everything and products:
			//el( InnerBlocks )
		);
	},

	save() {
		return el( 'div', { className: 'product', style: { outline: '1px solid gray', padding: 5 } },
			el( InnerBlocks.Content )
		);
	},
} );

} )();

I verified that If I revert the fix in the paragraph block the tests start failing.

[aduth]

When locking exist onReplace prop is not passed to blocks.

Could we address this by tackling #6587 as the same time?

[jorgefilipecosta]

Hi @aduth following #6587 would not solve this bug.
RichText would assume onReplace is available when it is not and the copy paste on empty locked paragraphs would not work.
It is possible to simulate that by changing the paragraph code to:

onReplace={ this.props.onReplace ? this.onReplace : () => {} }
onRemove={ this.props.onReplace ? ( () => onReplace( [] ) ) : () => {} }

That said one option I see to go with #6587 while solving this bug without big changes to rich text would be to have a BLOCK_EDITOR_NOOP variable referencing a function that is passed when the operation is not available. RichText would then be able to easily check if the action is not possible by comparing against BLOCK_EDITOR_NOOP.

[aduth]

I added the following CPT with locking:

There's an unset variable $template in this snippet.

[aduth]

copy paste on empty locked paragraphs would not work.

This wasn't clear, but I tested and I see what you mean now: RichText changes its behavior based on whether it receives an onReplace, so it could handle internally to substitute the text when it's not available.

gutenberg/packages/block-editor/src/components/rich-text/index.js

Line 286 in 5b3b3ab

const shouldReplace = this.props.onReplace && this.isEmpty();

Alternatively, we could revise the implementation so that we always pass an onReplace function which is not quite a noop: For a locked template, it would allow a replacement for when the substituted value would still satisfy the template (i.e. replacing one paragraph with another).

That being said, this is still an improvement, and allows us some time to consider whether we want to guarantee a function to be passed always (#6587) separately.