Inconsistent behavior when modifying tag attributes of resulting block outputs among different blocks

[mipon]

Describe the bug
Inconsistent behavior occurs when modifying tag attributes of resulting block outputs. Some tags like p do not show the message, This block contains unexpected or invalid content. while other tags do when the plugin which performs the modification is deactivated.

To Reproduce
Steps to reproduce the behavior:

1. Activate a plugin with the following sample JavaScript code.

const { assign } = lodash;
const { __ } = wp.i18n;
const { Fragment } = wp.element;
const { addFilter } = wp.hooks;
const { TextControl, PanelBody } = wp.components;
const { createHigherOrderComponent } = wp.compose;
const { InspectorControls } = wp.editor;


export const addMyCustomBlockControls = createHigherOrderComponent( ( BlockEdit ) => {

    return ( props ) => {

        if ( isAllowedBlockType( props.name ) && props.isSelected ) {
            return (
                <Fragment>
                    <BlockEdit { ...props } />
                    <InspectorControls>
                        <PanelBody title={ __( 'Bug Report' ) }>
							<TextControl
						  		label={ __( 'Test' ) }
								value={ props.attributes.test || '' }
   						  		onChange={ ( nextValue ) => {
									props.setAttributes( {
										test: nextValue,
									} );
						  		} }
						  	/>
                        </PanelBody>
                    </InspectorControls>
                </Fragment>
            );
        }
        return <BlockEdit { ...props } />;
    };
}, 'addMyCustomBlockControls' );
addFilter( 'editor.BlockEdit', 'custom-gutenberg-sidebar/my-control', addMyCustomBlockControls );

function isAllowedBlockType( name ) {

    const allowedBlockTypes = [
        'core/paragraph',
        'core/image',
        'core/heading',
        'core/code',
    ];

    return allowedBlockTypes.includes( name );

}// end isAllowedBlockType()

export function addAttribute( settings ) {

	if ( ! isAllowedBlockType( settings.name ) ) {
		return settings;
	}

	settings.attributes = assign( settings.attributes, {
		test: {
			type: 'string',
		},
	} );
	return settings;

}// end addAttribute()
addFilter( 'blocks.registerBlockType', 'custom-gutenberg-sidebar/add-attr', addAttribute );

export function addSaveProps( extraProps, blockType, attributes ) {

	// If the current block is not valid, add our prop.
	if ( ! isAllowedBlockType( blockType.name ) ) {
		return extraProps;
	}

	extraProps.test  = attributes.test;
	return extraProps;

}// end addSaveProps()
addFilter( 'blocks.getSaveContent.extraProps', 'custom-gutenberg-sidebar/add-props', addSaveProps );

2. Create two blocks. One is a paragraph block by setting an arbitrary text value in the added test text inspect control. The other is a code block and also enter an arbitrary value in the same control.

3. Confirm the front-end block outputs have a added custom attribute.

4. Deactivate the plugin and reload the post editing page.

Now you should see the message, This block contains unexpected or invalid content. only for the code block. The paragraph block does not produce this message.

Expected behavior
The both of the blocks should behave the same, either show the same message or do not show the message.

Desktop (please complete the following information):

• OS: Windows 7
• Browser Chrome
• Version 71

Additional context

• WordPress 5.0.3

Notes

Also clicking on the Resolve button, it is supposed to show the difference between the current and converted outputs. However, the both seem the same.

I prefer not to show the message. One use case is give expiration time to blocks per individual block basis by setting custom attributes through a plugin. Another is for code syntax highlighter. Some engines accept options with custom attributes in the <pre> tag. With the current behavior, when the plugin is deactivated, end users with little knowledge would assume something possibly critical got broken. This leaves unpleasant feelings and is definitely not a good user experience.

[youknowriad]

That behavior is expected, that's why the recommendation is to avoid building plugins altering the "save" function of the blocks. Instead you could leverage the render_block filter to add change the frontend representation of a block.

https://developer.wordpress.org/reference/hooks/render_block/

[mipon]

@youknowriad Are you serious? Explain why the paragraph block does not show the message while the code block does.

[youknowriad]

It is actually surprising that the paragraph block doesn't show the message. For me both are invalid and I do see the invalid message in the console for the paragraph block too but it seems that we have a "deprecated" version of the paragraph block considering this as valid which is causing the message to not appear there.

cc @aduth thoughts here? Do you think we should remove this deprecated version to ensure the paragraph block validation behaves similarly to other blocks?

[aduth]

See #4874 / #589 for context.

That specific deprecation entry for the paragraph block is meant to handle "removep"-formatted text, where the wrapping paragraph tags are omitted. An unintended consequence of this change is that it's tolerant to most any markup change.

It should be noted, however, that while the paragraph isn't flagged as being invalid, depending on the type of change you make, the resulting markup is likely not what you expect, where the invalid/revised content will have been naively wrapped with a new p element.

[mipon]

.... so it's intended.

One more thing, the Current and After Conversion outputs are literally the same, shown with the Resolve button. Why is it flagged as invalid then?

[aduth]

The "After Conversion" diff shows that the test="foo" attribute would be removed, which is the reason for the block being marked as invalid. While the paragraph block is not shown to the user as being invalid, if you look at the markup, you can see it produces an unexpected result like <p><p test="hello">Example</p></p>. This is something where the paragraph block is being too forgiving, and should be marked as invalid when encountering the paragraph tag with unexpected attributes.

Other blocks are marked as invalid because the editor sees that it would cause an attribute to be removed, and thus prompts the user in order to avoid unintended content loss. More information about this process can be found at:

https://wordpress.org/gutenberg/handbook/designers-developers/developers/block-api/block-edit-save/#validation

[mipon]

I see. Highlighted text in red means to be removed. Now it makes sense. Thanks for the explanation.