Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#54515 closed defect (bug) (fixed)

Block template themes: Parent theme block template takes precedence over child theme PHP template

Reported by: bernhard-reiter's profile Bernhard Reiter Owned by:
Milestone: 5.9 Priority: normal
Severity: normal Version: 5.9
Component: Editor Keywords: has-patch has-unit-tests commit
Focuses: Cc:

Description

When a theme has a PHP template of a certain specificity (e.g. page-home.php), and it happens to be a child theme of another theme which has a block template for the same specificity (e.g. page-home.html), WordPress will currently pick the parent theme’s block template over the child theme's PHP template to render the page.

This is regression. If the PHP and block template have equal specificity, the child theme's template should be used. This issue was fixed before in https://github.com/WordPress/gutenberg/pull/31123. The relevant logic has since been modified and eventually backported to Core , so the fix now needs to happen in Core (plus GB's pre-WP-5.9 compat layer, but that's somewhat secondary).

We have a unit test for this (but obviously had to disable it). The unit test existed in Gutenberg before but didn’t fail there due to a faulty test setup. In fact, the issue was only found while backporting the unit test.

Change History (16)

#1 @Bernhard Reiter
3 years ago

I’ve looked a bit into the problem, and I’ll try to describe my findings here.

The unit test is for locate_block_template( $template, $type, array $templates ). It is called after „classic“ (i.e. pre-blocks, PHP theme) WP template resolution is run. If the latter has found a potential candidate template (PHP) file, its path is passed as the $template argument to locate_block_template. The other two arguments contain the template $type (e.g. page, category, single, etc), and the template candidate hierarchy (e.g. page-home.php, page-123.php, page.php — see template hierarchy documentation).

locate_block_template() then invokes resolve_block_template( $type, $templates ), whose job it is to return the one block template file that's the best match for the given template hierarchy (if any). To do so, it tries to be clever: If we did get a (PHP) template candidate in $template, then it only passes the part of the $templates hierarchy array up to that specificity. E.g. if our theme has a page-123.php template, we already know that we're not looking for any block templates with lesser specificity, so we shorten $templates to only look for page-home.html and page-123.html (but not page.html).

resolve_block_template() then creates a $query based on these criteria, and calls get_block_templates( $query ), whose job it is to find matching block templates, both in the DB (as user-created wp_template CPTs), and as HTML files in the current theme.

In order to find the latter, it uses _get_block_templates_files( $template_type ) (the argument being either wp_template or wp_template_part). This function will fetch all block template files of a given theme; and, if the theme is a child theme, also of its parent!

We then go back up the stack track (and through a few extra steps of processing), ending up with candidate block templates from both the theme and its parent (if any), of equal or higher specificity of the PHP fallback template.

---

Due to the current architecture (function signatures, separation of tasks), it's not obvious where to check if a candidate block theme is coming from the current theme's parent, and if the fallback PHP theme has the same specificity, but should be preferred since it's coming from the child theme.

This ticket was mentioned in PR #1961 on WordPress/wordpress-develop by ockham.


3 years ago
#2

  • Keywords has-patch has-unit-tests added

When a theme has a PHP template of a certain specificity (e.g. page-home.php), and it happens to be a child theme of another theme which has a block template for the same specificity (e.g. page-home.html), WordPress will currently pick the parent theme’s block template over the child theme's PHP template to render the page.

This is a regression. If the PHP and block template have equal specificity, the child theme's template should be used. This issue was fixed before in https://github.com/WordPress/gutenberg/pull/31123. The relevant logic has since been modified and eventually backported to Core , so the fix now needs to happen in Core (plus GB's pre-WP-5.9 compat layer, but that's somewhat secondary).

We have a unit test for this (but obviously had to disable it). The unit test existed in Gutenberg before but didn’t fail there due to a faulty test setup. In fact, the issue was only found while backporting the unit test.

Trac ticket: [](https://core.trac.wordpress.org/ticket/54515)

ockham commented on PR #1961:


3 years ago
#3

My current approach would change the semantics of _get_block_templates_files() which is already used by GB, so we'd need to figure something out there. (Add under a new name or whatnot.) I'll try to get this fix into shape first and then think about the details of backward compat.

ockham commented on PR #1961:


3 years ago
#4

Potential TODO: Merge the logic from locate_block_template into the corresponding logic that we now have in get_block_templates().

This ticket was mentioned in PR #1985 on WordPress/wordpress-develop by ockham.


3 years ago
#5

Alternative approach to #1961. For a general idea of the challenges that a solution to this issue faces, and some design choices, see the code comments in the diff.

Description of the issue (as found in the corresponding Trac ticket) below:

---

When a theme has a PHP template of a certain specificity (e.g. page-home.php), and it happens to be a child theme of another theme which has a block template for the same specificity (e.g. page-home.html), WordPress will currently pick the parent theme’s block template over the child theme's PHP template to render the page.

This is a regression. If the PHP and block template have equal specificity, the child theme's template should be used. This issue was fixed before in https://github.com/WordPress/gutenberg/pull/31123. The relevant logic has since been modified and eventually backported to Core , so the fix now needs to happen in Core (plus GB's pre-WP-5.9 compat layer, but that's somewhat secondary).

We have a unit test for this (but obviously had to disable it). The unit test existed in Gutenberg before but didn’t fail there due to a faulty test setup. In fact, the issue was only found while backporting the unit test.

Trac ticket: https://core.trac.wordpress.org/ticket/54515

ockham commented on PR #1961:


3 years ago
#6

Closing in favor of #1985.

ockham commented on PR #1961:


3 years ago
#7

Closing in favor of #1985.

#8 @johnbillion
3 years ago

  • Milestone changed from Awaiting Review to 5.9

ockham commented on PR #1985:


3 years ago
#9

cc/ @WordPress/gutenberg-core for review 🙂

#10 @youknowriad
3 years ago

  • Keywords commit added

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


3 years ago

#13 @audrasjb
3 years ago

I tested this PR using TT2 Theme and WP 5.9 beta 1.

Before patch:

  • Create a child theme with a single.php file with random content inside.
  • Activate the child theme.
  • Go to a single post on front-end.
  • Issue: the child theme’s single.php template doesn't apply.

After applying the patch, the child theme’s single.php template apply to single posts.

Given Riad also validated the approach in the proposed PR, I'm self assigning the ticket for commit consideration.

#15 @audrasjb
3 years ago

  • Resolution set to fixed
  • Status changed from new to closed

This was fixed in [52308]. Trac bot just didn't catch the commit.

This ticket was mentioned in Slack in #themereview by utz119. View the logs.


3 years ago

Note: See TracTickets for help on using tickets.