Serialized code block is only half escaped

[notnownikki]

Description

While fixing errors related to kses stripping out attributes and tags for non-admin users, I found that the seralized HTML fixture for the code block contains characters that should be htmlentities, but aren't "Fixing" the fixture makes the test fail. Unfortunately, kses does the right thing and converts to entities, so the saved version of the block doesn't match the version the serializer produces.

This branch should pass, shouldn't it?

[mcsf]

This should be intentional per work on our element serializer: #5897

[mcsf]

To be a bit more precise: IIRC the serializer aims to strictly follow the spec

the text must not contain the character U+003C LESS-THAN SIGN (<) or an ambiguous ampersand.

gutenberg/element/serialize.js

Lines 238 to 253 in f496200

	/**
	* Returns an escaped HTML element value.
	*
	* @link https://w3c.github.io/html/syntax.html#writing-html-documents-elements
	* @link https://w3c.github.io/html/syntax.html#ambiguous-ampersand
	*
	* "the text must not contain the character U+003C LESS-THAN SIGN (<) or an
	* ambiguous ampersand."
	*
	* @param {string} value Element value.
	*
	* @return {string} Escaped HTML element value.
	*/
	function escapeHTML( value ) {
	return value.replace( /&/g, '&amp;' ).replace( /</g, '&lt;' );
	}

[notnownikki]

Ah, gotcha. So when a non-admin user saves a code block, it ends up fully escaped because kses does escaping. It doesn't seem to matter when loading the block back... I guess we're ok with that behaviour?

If so, we'll need to exclude the code block fixtures from the kses test, because we know they're going to be different.

[aduth]

Is there a practical impact from the difference here? Considering that the block validation treats them equivalent:

wp.blocks.isEquivalentHTML(
	'return <Button>Click Me!</Button>;',
	'return <Button>Click Me!</Button>;'
);
// true

(Exposing isEquivalentHTML requires some hackery)

[notnownikki]

There's no difference in actual use. Saving a loading a code block as an author works, so the only real impact here is that a test that makes sure that kses isn't altering things will have to skip the code block, or do something slightly different for the code block.

[aduth]

Previously: #9875

[mcsf]

It seems, from the discussion, that this pull request can be safely closed. Correct?

[notnownikki]

Yes, seems so!