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 (alternative approach) #1985

Closed
wants to merge 5 commits into from
Closed

Fix template resolution to give precedence to child theme PHP templates over parent theme block templates with equal specificity (alternative approach) #1985

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a73ee64
Enable unit test that covers child theme PHP template precedence
ockham Nov 27, 2021
cab0f39
Mention Core Trac ticket for regression
ockham Nov 29, 2021
6899d47
Fix the issue
ockham Nov 30, 2021
5585807
typo
ockham Nov 30, 2021
ba4b591
Make sure we do have a template_file
ockham Dec 1, 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
43 changes: 41 additions & 2 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 @@ -92,12 +92,14 @@ function locate_block_template( $template, $type, array $templates ) {
*
* @access private
* @since 5.8.0
* @since 5.9.0 Added the `$fallback_template` parameter.
*
* @param string $template_type The current template type.
* @param string[] $template_hierarchy The current template hierarchy, ordered by priority.
* @param string $fallback_template A PHP fallback template to use if no matching block template is found.
* @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 Down Expand Up @@ -129,6 +131,43 @@ static function ( $template_a, $template_b ) use ( $slug_priorities ) {
}
);

$theme_base_path = get_stylesheet_directory() . DIRECTORY_SEPARATOR;
$parent_theme_base_path = get_template_directory() . DIRECTORY_SEPARATOR;

// Is the current theme a child theme, and is the PHP fallback template part of it?
if (
strpos( $fallback_template, $theme_base_path ) === 0 &&
strpos( $fallback_template, $parent_theme_base_path ) === false
) {
$fallback_template_slug = substr(
$fallback_template,
// Starting position of slug.
strpos( $fallback_template, $theme_base_path ) + strlen( $theme_base_path ),
// Remove '.php' suffix.
-4
);

// Is our candidate block template's slug identical to our PHP fallback template's?
if (
count( $templates ) &&
$fallback_template_slug === $templates[0]->slug &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible that the fallback is actually different than the block template slug but has more priority. Ex: single.php, index.html. Which one would win here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it possible that the fallback is actually different than the block template slug but has more priority.

No, we prevent that by truncating the list of candidate templates that we pass from locate_block_template() to resolve_block_template() to only include equal or higher specificity than the PHP fallback template:

wordpress-develop/src/wp-includes/block-template.php

Lines 25 to 48 in 8bd1509

if ( $template ) {
/*
* locate_template() has found a PHP template at the path specified by $template.
* That means that we have a fallback candidate if we cannot find a block template
* with higher specificity.
*
* Thus, before looking for matching block themes, we shorten our list of candidate
* templates accordingly.
*/
// Locate the index of $template (without the theme directory path) in $templates.
$relative_template_path = str_replace(
array( get_stylesheet_directory() . '/', get_template_directory() . '/' ),
'',
$template
);
$index = array_search( $relative_template_path, $templates, true );
// If the template hiearchy algorithm has successfully located a PHP template file,
// we will only consider block templates with higher or equal specificity.
$templates = array_slice( $templates, 0, $index + 1 );
}
$block_template = resolve_block_template( $type, $templates );

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok I guess this looks good to me then.

'theme' === $templates[0]->source
) {
// Unfortunately, we cannot trust $templates[0]->theme, since it will always
// be set to the current theme's slug by _build_block_template_result_from_file(),
// even if the block template is really coming from the current theme's parent.
// (The reason for this is that we want it to be associated with the current theme
// -- not its parent -- once we edit it and store it to the DB as a wp_template CPT.)
// Instead, we use _get_block_template_file() to locate the block template file.
$template_file = _get_block_template_file( 'wp_template', $fallback_template_slug );
if ( $template_file && get_template() === $template_file['theme'] ) {
// The block template is part of the parent theme, so we
// have to give precedence to the child theme's PHP template.
array_shift( $templates );
}
}
}

return count( $templates ) ? $templates[0] : null;
}

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