Implement 'template' filter for the block template HTML #5188
Conversation
felixarntz
requested review from
spacedmonkey,
desrosj,
joemcgill and
westonruter
src/wp-includes/block-template.php
Outdated
| $content = wptexturize( $content ); | ||
| $content = convert_smilies( $content ); | ||
| $content = wp_filter_content_tags( $content, 'template' ); | ||
| $content = apply_filters( 'template', $content ); |
There was a problem hiding this comment.
This filter name is already in use in get_template.
There was a problem hiding this comment.
This is really power filter, are we sure we want to let developers filter the all the markup of the page?
There was a problem hiding this comment.
I think that's okay. It's similar to the_content, and it allows customizing the callbacks. Of course it's all a matter of responsible usage, but I'd argue already today in core any filter can be misused if someone wants to.
If we feel this is too much, maybe we can implement some alternative. Maybe just a list of callback functions that can be filtered only to remove any. Or we could make it an action so that developers can add callbacks that "listen" to the template content but cannot modify it. WDYT?
There was a problem hiding this comment.
Such a power filter is what is requested in Core-43258, although for the entire page instead of just between wp_body_open() and wp_footer(), as I commented in #5188 (comment). If we add yet another filter for the entire document in addition to this block template HTML filter, would that be problematic? Doesn't seem so.
There was a problem hiding this comment.
I agree there is overlap here, but I would prefer to not mix the output buffering discussion into this change here since it doesn't require it.
I am curious though, for block themes specifically, wouldn't this filter here already be sufficient? If we had this, I'm not sure about the benefits block themes would get from using an output buffer. Even without filtering the entire document, I can't think of any real limitations, since the filter would run before wp_head, wp_body_open and wp_footer, so you could still affect pretty much all other parts of the HTML document as well at that point.
There was a problem hiding this comment.
This filter would be mostly sufficient for block themes. That is, it would be mostly sufficient for optimization purposes. It wouldn't help with standardizing output buffering for caching plugins, however. Also, the lack of output buffering in the head means that certain optimizations couldn't be done, like optimizing the order of head elements (e.g. what Rick's capo.js instructs devs to do.)
| * | ||
| * @param string $content The entire block template HTML content. | ||
| */ | ||
| $content = apply_filters( 'the_block_template_html', $content ); |
There was a problem hiding this comment.
Another option would be to move this filter out from inside get_the_block_template_html() to instead apply inside of template-canvas.php on the entire HTML page:
diff --git a/src/wp-includes/template-canvas.php b/src/wp-includes/template-canvas.php
index 55f03267c8..f809b63e36 100644
--- a/src/wp-includes/template-canvas.php
+++ b/src/wp-includes/template-canvas.php
@@ -9,6 +9,20 @@
* Get the template HTML.
* This needs to run before <head> so that blocks can add scripts and styles in wp_head().
*/
+
+ob_start(
+ static function( $buffer ) {
+ /**
+ * Filters the block template HTML content.
+ *
+ * @since 6.4.0
+ *
+ * @param string $content The entire block template HTML content.
+ */
+ return apply_filters( 'the_block_template_html', $buffer );
+ }
+);
+
$template_html = get_the_block_template_html();
?><!DOCTYPE html>
<html <?php language_attributes(); ?>>This would be a step toward implementing Core-43258/Core-58285. Granted, the filter would also need to apply for classic themes as well, and filters being added in this PR for block themes probably shouldn't then also apply to classic themes.
There was a problem hiding this comment.
See my reply in #5188 (comment), I'm questioning what the benefit of that would be.
There was a problem hiding this comment.
Based on the way @felixarntz implemented the filter, trying to move this higher in the execution order to run these after the whole template is generated could cause execution order problems. For example, blocks are being parsed as part of the filter in get_the_block_template_html(). It's important that some filters run prior to that parsing, and others run after. When blocks are parsed, they may enqueue scripts and styles to be printed in the head. I don't think we would be able to satisfy each of those requirements by a single filter that runs after the entire template-canvas.
I think adding an additional filter that would allow an output buffer to be implemented for the entire rendered content would be helpful, but doesn't necessarily negate the need to process template content prior to assembling the final HTML document.
There was a problem hiding this comment.
As for a separate filter for the entire HTML document, I've proposed this as part of the full-page client-side navigation experiment in Gutenberg: WordPress/gutenberg#61212
|
I've refreshed this PR in #6533. |
|
Closing in favor of #6533 |
See https://core.trac.wordpress.org/ticket/55996#comment:39 for context.
templatefilter for the block template HTML.wp_filter_content_tags(), to keep parity. It is also short and concise.convert_smilies()is now run afterwp_filter_content_tags(), which functionally shouldn't matter since these two functions do not interfere with each other. The reasonconvert_smilies()now runs later is to get parity with how the same callback is added tothe_content, at priority 20.do_blocks, only running it for thetemplatecontext.Trac ticket: https://core.trac.wordpress.org/ticket/55996
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.