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

Fix template resolution to give precedence to child theme PHP templates over parent theme block templates with equal specificity #1961

Closed
wants to merge 12 commits into from
Closed

Fix template resolution to give precedence to child theme PHP templates over parent theme block templates with equal specificity #1961

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
8800b0c
Enable unit test that covers child theme PHP template precedence
ockham Nov 27, 2021
aedf0bd
Filter for candidate slugs in _get_block_templates_files
ockham Nov 27, 2021
6bf5ca0
Remove now-redundant slug filter
ockham Nov 27, 2021
4a1f48b
Only have _get_block_templates_files look in given theme
ockham Nov 27, 2021
598a82b
phpcbf
ockham Nov 27, 2021
c44237c
Ooops
ockham Nov 27, 2021
1524e50
Mention Core Trac ticket for regression
ockham Nov 29, 2021
9cbc48f
Have get_block_templates() use the query's theme
ockham Nov 29, 2021
5116a47
Add PHPDoc for query theme arg
ockham Nov 29, 2021
eb2a0ae
Add query theme arg to filter signature PHPDoc
ockham Nov 29, 2021
92c7a74
Fix template file lookup logic
ockham Nov 29, 2021
ad14985
Pass fallback template to get_block_templates() to determine whether …
ockham Nov 29, 2021
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
114 changes: 77 additions & 37 deletions src/wp-includes/block-template-utils.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,51 +284,56 @@ function _get_block_template_file( $template_type, $slug ) {
}

/**
* Retrieves the template files from the theme.
* Retrieves the template files from the theme.
*
* @access private
* @since 5.9.0
*
* @param string $template_type wp_template or wp_template_part.
* @param string $template_type wp_template or wp_template_part.
* @param string $theme The theme in which to look for template files.
* @param array $candidate_slugs If specified, only consider files that match slugs in this array.
*
* @return array Template.
*/
function _get_block_templates_files( $template_type ) {
function _get_block_templates_files( $template_type, $theme, $candidate_slugs ) {
if ( 'wp_template' !== $template_type && 'wp_template_part' !== $template_type ) {
return null;
}

$themes = array(
get_stylesheet() => get_stylesheet_directory(),
get_template() => get_template_directory(),
);
$theme_root = get_theme_root( $theme );
$theme_dir = "$theme_root/$theme";

$template_base_paths = get_block_theme_folders( $theme );
$theme_template_files = _get_block_templates_paths( $theme_dir . '/' . $template_base_paths[ $template_type ] );

$template_files = array();
foreach ( $themes as $theme_slug => $theme_dir ) {
$template_base_paths = get_block_theme_folders( $theme_slug );
$theme_template_files = _get_block_templates_paths( $theme_dir . '/' . $template_base_paths[ $template_type ] );
foreach ( $theme_template_files as $template_file ) {
$template_base_path = $template_base_paths[ $template_type ];
$template_slug = substr(
$template_file,
// Starting position of slug.
strpos( $template_file, $template_base_path . DIRECTORY_SEPARATOR ) + 1 + strlen( $template_base_path ),
// Subtract ending '.html'.
-5
);
$new_template_item = array(
'slug' => $template_slug,
'path' => $template_file,
'theme' => $theme_slug,
'type' => $template_type,
);
foreach ( $theme_template_files as $template_file ) {
$template_base_path = $template_base_paths[ $template_type ];
$template_slug = substr(
$template_file,
// Starting position of slug.
strpos( $template_file, $template_base_path . DIRECTORY_SEPARATOR ) + 1 + strlen( $template_base_path ),
// Subtract ending '.html'.
-5
);

if ( 'wp_template_part' === $template_type ) {
$template_files[] = _add_block_template_part_area_info( $new_template_item );
}
if ( isset( $candidate_slugs ) && ! in_array( $template_slug, $candidate_slugs, true ) ) {
continue;
}

if ( 'wp_template' === $template_type ) {
$template_files[] = _add_block_template_info( $new_template_item );
}
$new_template_item = array(
'slug' => $template_slug,
'path' => $template_file,
'theme' => $theme,
'type' => $template_type,
);

if ( 'wp_template_part' === $template_type ) {
$template_files[] = _add_block_template_part_area_info( $new_template_item );
}

if ( 'wp_template' === $template_type ) {
$template_files[] = _add_block_template_info( $new_template_item );
}
}

Expand Down Expand Up @@ -565,12 +570,13 @@ function _build_block_template_result_from_post( $post ) {
* @type int $wp_id Post ID of customized template.
* @type string $area A 'wp_template_part_area' taxonomy value to filter by (for wp_template_part template type only).
* @type string $post_type Post type to get the templates for.
* @type string $theme Theme in which to look for template files.
* }
* @param array $template_type wp_template or wp_template_part.
*
* @return array Templates.
*/
function get_block_templates( $query = array(), $template_type = 'wp_template' ) {
function get_block_templates( $query = array(), $template_type = 'wp_template', $child_theme_fallback_php_template = null ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't like how I had to change the function signature of get_block_templates() -- which is, after all, supposed to be a general-purpose function to get all templates for a given theme (DB or file-based).

I'll try a different approach (where the filtering will hopefully happen in a later stage, i.e. in resolve_block_template -- even though that'll require some redundant checks) in a separate PR.

/**
* Filters the block templates array before the query takes place.
*
Expand All @@ -586,6 +592,7 @@ function get_block_templates( $query = array(), $template_type = 'wp_template' )
* @type array $slug__in List of slugs to include.
* @type int $wp_id Post ID of customized template.
* @type string $post_type Post type to get the templates for.
* @type string $theme Theme in which to look for template files.
* }
* @param array $template_type wp_template or wp_template_part.
*/
Expand All @@ -595,6 +602,7 @@ function get_block_templates( $query = array(), $template_type = 'wp_template' )
}

$post_type = isset( $query['post_type'] ) ? $query['post_type'] : '';
$theme = isset( $query['theme'] ) ? $query['theme'] : wp_get_theme()->get_stylesheet();
$wp_query_args = array(
'post_status' => array( 'auto-draft', 'draft', 'publish' ),
'post_type' => $template_type,
Expand All @@ -604,7 +612,7 @@ function get_block_templates( $query = array(), $template_type = 'wp_template' )
array(
'taxonomy' => 'wp_theme',
'field' => 'name',
'terms' => wp_get_theme()->get_stylesheet(),
'terms' => $theme,
),
),
);
Expand Down Expand Up @@ -646,7 +654,41 @@ function get_block_templates( $query = array(), $template_type = 'wp_template' )
}

if ( ! isset( $query['wp_id'] ) ) {
$template_files = _get_block_templates_files( $template_type );
$parent_theme = wp_get_theme( $theme )->get_template();
$parent_theme = isset( $parent_theme ) ? $parent_theme : $theme;

$themes = array_unique(
array( $theme, $parent_theme )
);

$slug__in = isset( $query['slug__in'] ) ? $query['slug__in'] : null;

$template_files = array();
foreach ( $themes as $theme_slug ) {
$template_files = array_merge(
$template_files,
_get_block_templates_files( $template_type, $theme_slug, $slug__in )
);
if ( $child_theme_fallback_php_template && isset( $slug__in ) ) {
$theme_base_path = wp_get_theme( $theme )->get_stylesheet_directory();
$fallback_php_template_slug = substr(
$child_theme_fallback_php_template,
// Starting position of slug.
strpos( $child_theme_fallback_php_template, $theme_base_path . DIRECTORY_SEPARATOR ) + 1 + strlen( $theme_base_path ),
// Remove '.php' suffix.
-4
);

// Locate the index of the fallback template's slug (without the theme directory path) in $slug__in.
$index = array_search( $fallback_php_template_slug, $slug__in, true );

// If the template hierarchy algorithm has successfully located a PHP template file
// in the child theme, we will only consider block templates with higher specificity
// from the parent theme.
$slug__in = array_slice( $slug__in, 0, $index );
}
}

foreach ( $template_files as $template_file ) {
$template = _build_block_template_result_from_file( $template_file, $template_type );

Expand All @@ -666,11 +708,9 @@ function get_block_templates( $query = array(), $template_type = 'wp_template' )
array_column( $query_result, 'id' ),
true
);
$fits_slug_query =
! isset( $query['slug__in'] ) || in_array( $template_file['slug'], $query['slug__in'], true );
$fits_area_query =
! isset( $query['area'] ) || $template_file['area'] === $query['area'];
$should_include = $is_not_custom && $fits_slug_query && $fits_area_query;
$should_include = $is_not_custom && $fits_area_query;
if ( $should_include ) {
$query_result[] = $template;
}
Expand Down
6 changes: 3 additions & 3 deletions src/wp-includes/block-template.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ function locate_block_template( $template, $type, array $templates ) {
$templates = array_slice( $templates, 0, $index + 1 );
}

$block_template = resolve_block_template( $type, $templates );
$block_template = resolve_block_template( $type, $templates, $template );

if ( $block_template ) {
if ( empty( $block_template->content ) && is_user_logged_in() ) {
Expand Down Expand Up @@ -97,7 +97,7 @@ function locate_block_template( $template, $type, array $templates ) {
* @param string[] $template_hierarchy The current template hierarchy, ordered by priority.
* @return WP_Block_Template|null template A template object, or null if none could be found.
*/
function resolve_block_template( $template_type, $template_hierarchy ) {
function resolve_block_template( $template_type, $template_hierarchy, $fallback_template ) {
if ( ! $template_type ) {
return null;
}
Expand All @@ -116,7 +116,7 @@ function resolve_block_template( $template_type, $template_hierarchy ) {
'theme' => wp_get_theme()->get_stylesheet(),
'slug__in' => $slugs,
);
$templates = get_block_templates( $query );
$templates = get_block_templates( $query, 'wp_template', $fallback_template );

// Order these templates per slug priority.
// Build map of template slugs to their priority in the current hierarchy.
Expand Down
8 changes: 1 addition & 7 deletions tests/phpunit/tests/block-template.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,16 +89,10 @@ function test_more_specific_php_template_takes_precedence_over_less_specific_blo
* otherwise equal specificity.
*
* Covers https://github.com/WordPress/gutenberg/pull/31123.
* Covers https://core.trac.wordpress.org/ticket/54515.
*
*/
function test_child_theme_php_template_takes_precedence_over_equally_specific_parent_theme_block_template() {
/**
* @todo This test is currently marked as skipped, since it wouldn't pass. Turns out that in Gutenberg,
* it only passed due to a erroneous test setup.
* For details, see https://github.com/WordPress/wordpress-develop/pull/1920#issuecomment-975929818.
*/
$this->markTestSkipped( 'The block template resolution algorithm needs fixing in order for this test to pass.' );

switch_theme( 'block-theme-child' );

$page_slug_template = 'page-home.php';
Expand Down