Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement core template loader overrides to rely on wp_template posts #17626

Merged
merged 11 commits into from
Oct 23, 2019
Merged

Implement core template loader overrides to rely on wp_template posts #17626

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
c1e61ec
Introduce wp_template post type.
felixarntz Sep 22, 2019
fc183d4
Improve (temporary) admin UI for wp_template post type by exposing slug.
felixarntz Sep 22, 2019
91df716
Implement template loader overrides to rely on 'wp_template' posts.
felixarntz Sep 22, 2019
afb6604
Render viewport meta tag.
felixarntz Sep 22, 2019
ee46ef5
Prevent deletion of fallback 'wp_template' post 'index'.
felixarntz Sep 22, 2019
b9864ea
Scope PR to just basic wp_template post type registration.
felixarntz Sep 27, 2019
27eb0f2
Implement core template loader overrides to rely on wp_template posts…
felixarntz Sep 27, 2019
7bdcd6d
Render title tag regardless of theme support
felixarntz Sep 28, 2019
8f1529a
Make getting correct wp_template post more error-proof
felixarntz Sep 28, 2019
960b555
Template Loader: Add more content filters.
epiqueras Oct 23, 2019
d3ddc40
Templates: Fix experiment flag logic.
epiqueras Oct 23, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/load.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@

require dirname( __FILE__ ) . '/blocks.php';
require dirname( __FILE__ ) . '/templates.php';
require dirname( __FILE__ ) . '/template-loader.php';
require dirname( __FILE__ ) . '/client-assets.php';
require dirname( __FILE__ ) . '/demo.php';
require dirname( __FILE__ ) . '/widgets.php';
Expand Down
23 changes: 23 additions & 0 deletions lib/template-canvas.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php
/**
* Template canvas file to render the current 'wp_template'.
*
* @package gutenberg
*/

?>
<!DOCTYPE html>
<html <?php language_attributes(); ?>>
<head>
<meta charset="<?php bloginfo( 'charset' ); ?>" />
<?php wp_head(); ?>
felixarntz marked this conversation as resolved.
Show resolved Hide resolved
</head>

<body <?php body_class(); ?>>
<?php wp_body_open(); ?>

<?php gutenberg_render_the_template(); ?>

<?php wp_footer(); ?>
</body>
</html>
177 changes: 177 additions & 0 deletions lib/template-loader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
<?php
/**
* Block template loader functions.
*
* @package gutenberg
*/

/**
* Adds necessary filters to use 'wp_template' posts instead of theme template files.
*/
function gutenberg_add_template_loader_filters() {
if ( ! post_type_exists( 'wp_template' ) ) {
return;
}

/**
* Array of all overrideable default template types.
*
* @see get_query_template
*
* @var array
*/
$template_types = array(
'index',
'404',
'archive',
'author',
'category',
'tag',
'taxonomy',
'date',
// Skip 'embed' for now because it is not a regular template type.
'home',
'frontpage',
'privacypolicy',
'page',
'search',
'single',
'singular',
'attachment',
);
foreach ( $template_types as $template_type ) {
add_filter( $template_type . '_template', 'gutenberg_override_query_template', 20, 3 );
}

add_filter( 'template_include', 'gutenberg_find_template', 20 );
}
add_action( 'wp_loaded', 'gutenberg_add_template_loader_filters' );

/**
* Filters into the "{$type}_template" hooks to record the current template hierarchy.
*
* The method returns an empty result for every template so that a 'wp_template' post
* is used instead.
*
* @see gutenberg_find_template
*
* @param string $template Path to the template. See locate_template().
* @param string $type Sanitized filename without extension.
* @param array $templates A list of template candidates, in descending order of priority.
* @return string Empty string to ensure template file is considered not found.
*/
function gutenberg_override_query_template( $template, $type, array $templates = array() ) {
global $_wp_current_template_hierarchy;

if ( ! is_array( $_wp_current_template_hierarchy ) ) {
$_wp_current_template_hierarchy = $templates;
} else {
$_wp_current_template_hierarchy = array_merge( $_wp_current_template_hierarchy, $templates );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When will this ever run more than once and require this merging?

And what guarantees subsequent runs pass $templates where the first item is of lesser priority than the last item of the current $_wp_current_template_hierarchy?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on how core's template loader works, this may be run multiple times (e.g. get_index_template() being called after another get_*_template() function). As well based on core, the functions are called in the correct hierarchy order (more specific come first), so it will always be correct regarding priority.

Note that this is only a layer to make it work in the plugin anyway. Most likely, if/when this gets merged into core, it would be more directly integrated with the existing template loader code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that makes sense.

}

return '';
}

/**
* Find the correct 'wp_template' post for the current hierarchy and return the path
* to the canvas file that will render it.
*
* @param string $template_file Original template file. Will be overridden.
* @return string Path to the canvas file to include.
*/
function gutenberg_find_template( $template_file ) {
global $_wp_current_template_post, $_wp_current_template_hierarchy;

// Bail if no relevant template hierarchy was determined, or if the template file
// was overridden another way.
if ( ! $_wp_current_template_hierarchy || $template_file ) {
return $template_file;
}

$slugs = array_map(
'gutenberg_strip_php_suffix',
$_wp_current_template_hierarchy
);

// Find most specific 'wp_template' post matching the hierarchy.
$template_query = new WP_Query(
array(
'post_type' => 'wp_template',
'post_status' => 'publish',
'post_name__in' => $slugs,
'orderby' => 'post_name__in',
'posts_per_page' => 1,
)
);

if ( $template_query->have_posts() ) {
$template_posts = $template_query->get_posts();
$_wp_current_template_post = array_shift( $template_posts );
}

// Add extra hooks for template canvas.
add_action( 'wp_head', 'gutenberg_viewport_meta_tag', 0 );
felixarntz marked this conversation as resolved.
Show resolved Hide resolved
remove_action( 'wp_head', '_wp_render_title_tag', 1 );
add_action( 'wp_head', 'gutenberg_render_title_tag', 1 );

// This file will be included instead of the theme's template file.
return gutenberg_dir_path() . 'lib/template-canvas.php';
}

felixarntz marked this conversation as resolved.
Show resolved Hide resolved
/**
* Displays title tag with content, regardless of whether theme has title-tag support.
*
* @see _wp_render_title_tag()
*/
function gutenberg_render_title_tag() {
echo '<title>' . wp_get_document_title() . '</title>' . "\n";
}

/**
* Renders the markup for the current template.
*/
function gutenberg_render_the_template() {
global $_wp_current_template_post;
global $wp_embed;

if ( ! $_wp_current_template_post || 'wp_template' !== $_wp_current_template_post->post_type ) {
echo '<h1>' . esc_html__( 'No matching template found', 'gutenberg' ) . '</h1>';
return;
}

$content = $_wp_current_template_post->post_content;

$content = $wp_embed->run_shortcode( $content );
$content = $wp_embed->autoembed( $content );
$content = do_blocks( $content );
$content = wptexturize( $content );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why only wptexturize()? Should there rather be a new filter applied like wp_template_content which then can have filters as core applies:

add_filter( 'wp_template_content', 'do_blocks', 9 );
add_filter( 'wp_template_content', 'wptexturize' );
add_filter( 'wp_template_content', 'convert_smilies', 20 );
add_filter( 'wp_template_content', 'wpautop' );
add_filter( 'wp_template_content', 'shortcode_unautop' );
add_filter( 'wp_template_content', 'prepend_attachment' );
add_filter( 'wp_template_content', 'wp_make_content_images_responsive' );
add_filter( 'wp_template_content', 'do_shortcode', 11 ); // AFTER wpautop()
add_filter( 'wp_template_content', 'capital_P_dangit', 11 );

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be very useful for theme devs.

cc @youknowriad

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is worth a discussion, actually about two things:

  1. Do we want to introduce a filter here?
  2. Which functions should the content be run through by default?

Regarding 1., I would prefer to start without one and only introduce it based on requests that are reasonable. The the_content filter is mostly misused to append/prepend additional content, which in a block-based world becomes unnecessary - having a wp_template_content being misused in the same way would most likely introduce broader issues (since it covers the entire page markup).

Regarding 2., I think some of the filter functions definitely don't make sense for wp_template content (e.g. prepend_attachment). Others we need to discuss further (like do we want to deal with shortcodes at all in this?).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with your points for 1.

I am not familiar enough with these filters to comment about 2. Maybe we should ping more people who worked on them?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the WP.com implementation of Full Site Editing templates we noticed that we had to run several filters on the template content.

We basically started adding them manually whenever we caught an issue, but then basically added all of them (see for example the useless prepend_attachment that I added—thanks for pointing it out @felixarntz! 😅) to save us headaches.
For example, I vividly recall how wp_make_content_images_responsive almost single-handedly fixed all image related issue we were having.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Copons, I just found that code as well and wanted to comment here, but you beat me to it. :)

Based on that and your comment, I think what we should add to this PR for now are the following:

  • $wp_embed->run_shortcode( $content )
  • $wp_embed->autoembed( $content )
  • wp_make_content_images_responsive( $content )

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

$content = wp_make_content_images_responsive( $content );
$content = str_replace( ']]>', ']]&gt;', $content );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is in the_content() but why is it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's because wptexturize doesn't handle that specific case?


// Wrap block template in .wp-site-blocks to allow for specific descendant styles
// (e.g. `.wp-site-blocks > *`).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary? Couldn't the body be used instead of .wp-site-blocks? Alternatively, the wp-site-blocks class could be added to the body_classes when a block-based template is being served as opposed to a PHP-based template.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

body will also style wp_footer content.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly, in a way I agree it'd be nicer to not have this extra div, but the way many themes handle design rules for block content is to use a selector like .{blocks-wrap-class} > *, which would indeed style wp_footer content that it shouldn't apply to.

I'd say let's be conservative for now and start with this div. We should definitely continue to look for an alternative solution though that satisfies the styling needs.

echo '<div class="wp-site-blocks">';
echo $content; // phpcs:ignore WordPress.Security.EscapeOutput
echo '</div>';
}

/**
* Renders a 'viewport' meta tag.
*
* This is hooked into {@see 'wp_head'} to decouple its output from the default template canvas.
*/
function gutenberg_viewport_meta_tag() {
echo '<meta name="viewport" content="width=device-width, initial-scale=1" />' . "\n";
}

/**
* Strips .php suffix from template file names.
*
* @access private
*
* @param string $template_file Template file name.
* @return string Template file name without extension.
*/
function gutenberg_strip_php_suffix( $template_file ) {
return preg_replace( '/\.php$/', '', $template_file );
}
31 changes: 30 additions & 1 deletion lib/templates.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
*/
function gutenberg_register_template_post_type() {
if (
get_option( 'gutenberg-experiments' ) &&
! get_option( 'gutenberg-experiments' ) ||
! array_key_exists( 'gutenberg-full-site-editing', get_option( 'gutenberg-experiments' ) )
) {
return;
Expand Down Expand Up @@ -65,3 +65,32 @@ function gutenberg_grant_template_caps( array $allcaps ) {
return $allcaps;
}
add_filter( 'user_has_cap', 'gutenberg_grant_template_caps' );

/**
* Filters capabilities to prevent deletion of the 'wp_template' post with slug 'index'.
*
* Similar to today's themes, this template should always exist.
*
* @param array $caps Array of the user's capabilities.
* @param string $cap Capability name.
* @param int $user_id The user ID.
* @param array $args Adds the context to the cap. Typically the object ID.
* @return array Filtered $caps.
*/
function gutenberg_prevent_index_template_deletion( $caps, $cap, $user_id, $args ) {
if ( 'delete_post' !== $cap || ! isset( $args[0] ) ) {
return $caps;
}

$post = get_post( $args[0] );
if ( ! $post || 'wp_template' !== $post->post_type ) {
return $caps;
}

if ( 'index' === $post->post_name ) {
$caps[] = 'do_not_allow';
}

return $caps;
}
add_filter( 'map_meta_cap', 'gutenberg_prevent_index_template_deletion', 10, 4 );