Templates : HTML comments cause the Site Editor to crash.

[scruffian]

What problem does this address?

When testing #38984, the modified template causes the Site Editor to crash.

To replicate:

1. Add an empty HTML comment to a template file in a block theme:
2. Open the template in the Site Editor
3. The Site Editor displays this error:

What is your proposed solution?

We should allow HTMLs comment in template files!

[mcsf]

Here is what's happening:

• Any HTML comment that isn't a Gutenberg block boundary is parsed by the parser as "freeform" content, in the same way that a stray piece of HTML would be if found outside of any blocks.
• The Post and Site editors are configured to delegate the handling of freeform content to the Classic block.
• The Classic block is an adaptor for an underlying TinyMCE instance.
• TinyMCE is only loaded in the Post environment, so when the Classic block tries to invoke functions in the nonexisting wp.tinymce, we get a TypeError.

A few ideas on how to solve this at different layers. Most of these suggestions are mutually compatible:

• I'd rather not touch the block parser itself. It's doing exactly as it was designed: returning nodes for any non-block piece of HTML it finds.
• In the Site Editor, silently discard freeform nodes in the Site Editor if they only contain an HTML comment. If there's a freeform node with any actual HTML content, that should probably be surfaced up to the user.
• or: In the Classic block, silently render nothing if given just an HTML comment (not sure how I feel about this one).
• In the Classic block, explicitly test for the presence of TinyMCE, and show a dedicated notice if it isn't. This is a much nicer failure mode than the generic block error view.
• In the Site Editor, consider setting the HTML Block as the handler of freeform content, instead of the Classic block.
• I didn't look into this, but look at the initialisation logic in the block library (registerCoreBlocks), because we shouldn't be setting Classic block if the environment isn't complete.

[dmsnell]

thanks @mcsf for jumping in and investigating!

silently discard freeform nodes in the Site Editor if they only contain an HTML comment

sounds great to me! or build a small criterion for discarding them, since we could have the same kind of issues we've seen with extra newlines.

/**
 * Checks if given HTML is likely empty.
 *
 * Naive criteria for whether HTML represents meaningful content:
 *  - contains non-whitespace #text content, e.g. "this bare text implies HTML structure"
 *  - there exist _any_ tags/elements, e.g. "<div class=placeholder></div>"
 *
 * Specifically, these things are not considered meaningful content:
 *  - HTML comments
 *  - CDATA sections
 *  - "blank" #text nodes, e.g. newlines, medium mathematical space, non-breaking space
 */
const htmlLikelyEmpty = html => {
	const div = document.createElement( 'div' );
	div.innerHTML = html;

	// If we have text content then it's probably supposed to render.
	if ( div.innerText.trim() ) {
		return false;
	}

	for ( const { nodeType } of div.childNodes ) {
		// If we have any HTML elements we probably need to render
		// even if the text content is empty.
		if ( Node.ELEMENT_NODE === nodeType ) {
			return false;
		}
	}

	return true;
}

[kevin940726]

This also happens if we just add an arbitrary HTML in the template file. For instance, add a <p>Hello World</p>.

In the Site Editor, consider setting the HTML Block as the handler of freeform content, instead of the Classic block.

I think this might be a better option as it's less destructive, and users can still keep their content no matter if it's accidentally added or not.

Comments-only freeform block can still be handled by not rendering anything in the visual editor, but available in the code editor. In the end, comments are code, not visible to the users, ...I think 😆 .

[oandregal]

There's a follow-up for this issue that is tracked at #48408