Refresh: Implement 'the_block_template_html' filter for the block template

[joemcgill]

This refreshes #5188

• Merges trunk and resolves conflicts
• Updates docblocks

Trac ticket: https://core.trac.wordpress.org/ticket/55996
Trac ticket: https://core.trac.wordpress.org/ticket/61446

---

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props flixos90, joemcgill, westonruter, ryelle.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[github-actions]

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

• The Plugin and Theme Directories cannot be accessed within Playground.
• All changes will be lost when closing a tab with a Playground instance.
• All changes will be lost when refreshing the page.
• A fresh instance is created each time the link below is clicked.
• Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
  it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

[westonruter]

@joemcgill This is still pending tests, correct?

[joemcgill]

@westonruter I think adding some tests could be good. However, the way this was implemented seems to be fully compatible with the previous behavior, so previous tests for block-templates should cover this change. I'll spend some time verifying and see if there are some places where additional test coverage should be added.

[westonruter]

The fix here is purely for performance, so there's no bug that can be tested otherwise?

[joemcgill]

As far as I can tell, this wouldn't have any direct impact on performance—if anything, moving from hard coded functions to hooks, is probably slower—but it does provide the opportunity for post processing that needs to be run across the whole template to be handled in one place in the future, which would be better.

[westonruter]

I thought Core-55996 is intended to be a performance fix by avoiding the same functions being run on the content more than once?

[ryelle]

I've manually tested this, and everything still works as expected (embeds, shortcodes, etc) across patterns, templates, and content. I also tried using the the_block_template_html filter for the issue I raised in WordPress/gutenberg#61454, and it works perfectly.

[joemcgill]

@westonruter

I thought Core-55996 is intended to be a performance fix by avoiding the same functions being run on the content more than once?

That's correct. Though, @felixarntz questioned whether running certain filters twice was a bad thing in this comment. This PR does not address that question, but only moves the hard-coded function calls for the block template canvas to a filter. To finish addressing the original ticket, I think we would need to move some post-processing off of the post content block since it is being run on the full template, but that will need a bit more thinking. Either way, having this filter in place is a step towards addressing the original ticket concerns and addresses the use case @ryelle ran into. I'd be happy to open a separate ticket specifically for proposing the filter, but since he already had the implementation here, I just refreshed his PR.

[westonruter]

LGTM

[felixarntz]

@joemcgill This looks solid to me. Sharing a few considerations we may want to think through.

[felixarntz]

Instead of having the order of these two purely defined by the order of these two add_filter() calls, I think we should use priority 12 on wp_filter_content_tags, also for consistency with how it is intentionally done elsewhere as of 6560f46.

Suggested change

	add_filter( 'the_block_template_html', 'wptexturize' );
	add_filter( 'the_block_template_html', 'wp_filter_content_tags' );
	add_filter( 'the_block_template_html', 'wptexturize' );
	add_filter( 'the_block_template_html', 'wp_filter_content_tags', 12 );

[felixarntz]

In the original code that is being changed here, convert_smilies() was run before wp_filter_content_tags(). While I'm not opposed to use the same priority as convert_smilies uses elsewhere, we should make sure using a different order here wasn't an intentional decision (and there is a good chance it wasn't).

But if we don't feel strongly, it would be safer to leave out the priority on convert_smilies() so that the original order is maintained.

Suggested change

	add_filter( 'the_block_template_html', 'wp_filter_content_tags' );
	add_filter( 'the_block_template_html', 'convert_smilies', 20 );
	add_filter( 'the_block_template_html', 'wp_filter_content_tags', 12 );
	add_filter( 'the_block_template_html', 'convert_smilies' );

[felixarntz]

While this maintains the original order of the function, I wonder whether we could align this with the filter order execution on the other similar scenarios, e.g. on the_content. On those, shortcode_unautop uses the default 10 and do_shortcode uses 11.

I know this comment is a bit opposite of my other comment, but I'm purely throwing it out there for consideration. I wonder why it was even decided in the first place to use a different order here.

On another note: If we used 10 and 11 here, it would also allow to use 8 for the filters in the WP_Embed class, which is another place to make things more consistent.

Suggested change

	add_filter( 'the_block_template_html', 'shortcode_unautop', 7 );
	add_filter( 'the_block_template_html', 'do_shortcode', 8 );
	add_filter( 'the_block_template_html', 'shortcode_unautop' );
	add_filter( 'the_block_template_html', 'do_shortcode', 11 );

[felixarntz]

If we go with my other suggestion on adjusting the shortcode filter priorities, we could align this:

Suggested change

	add_filter( 'the_block_template_html', array( $this, 'run_shortcode' ), 6 );
	add_filter( 'the_block_template_html', array( $this, 'run_shortcode' ), 8 );

[felixarntz]

If we go with my other suggestion on adjusting the shortcode filter priorities, we could align this:

Suggested change

	add_filter( 'the_block_template_html', array( $this, 'autoembed' ), 6 );
	add_filter( 'the_block_template_html', array( $this, 'autoembed' ), 8 );

[joemcgill]

Another use case for a filter like this is described in WordPress/gutenberg#61212.

[westonruter]

@joemcgill Is it? That filter allows attributes to be added to the body tag, but that's not possible with this the_block_template_html filter, right?

[felixarntz]

@joemcgill Is this something you're still planning to get back to? I think foundationally the PR looks good, mostly minor feedback to address, plus this now needs a refresh due to a merge conflict.