-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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.
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
10.5 breaks templating logic and use index.php instead of dedicated template. #31399
Comments
Hi What is the name of the template and what is the file slug? |
Hi It used the parent index.php because I did not have any in my child theme (normal behaviour). I did it in DEBUG_MODE on , in order to check any error, but I saw nothing. PS : I'm not alone ... |
What is the name of the template and what is the file slug? |
I am able to reproduce it with Argent, a theme that was mentioned in the support topic. https://wordpress.org/themes/argent/ |
We are also seeing pages use |
Does this still happen for you after this was merged? #31336 |
I think this was a deliberate addition; there's some discussion about it here, plus some more background provided here. |
Edit 2: I've been able to repro with Argent now. Some notes (in case others are also struggling to repro):
Without GB: With GB 10.5.3 active: |
Thanks! Yeah, I seem to be able to confirm. Furthermore, it seems like the bug is still present on |
I'll try to replicate and see if I can get it fixed, or at least point in the right direction! |
I'm looking into a fix as we speak. I'm pretty sure the issue is in
We need to get the fallback priority of templates that are returned right (which is a bit finicky -- we really need some unit test coverage 😅 ). |
This hack seems to fix it: $current_template_slug = str_replace( [ trailingslashit( get_template_directory() ), '.php' ], [ '', '' ], $template ); Here:
Edit: not sure whether to use |
Thanks @vdwijngaert and @WraithKenny -- I think we're on the right track here. I agree that the template resolution algorithm has unfortunately become a bit hard to reason about. The crucial point is probably indeed the way that some functions attempt to strip e.g. If we get those right, we can probably strip the child-theme specific code: gutenberg/lib/full-site-editing/template-loader.php Lines 79 to 93 in 9c242ee
which I believe is basically duplicating this logic: gutenberg/lib/full-site-editing/template-loader.php Lines 105 to 111 in 9c242ee
However, let's make sure we don't add any more obscure string parsing and the like to the algorithm 😬 -- ideally, we should have a good understanding of what we're doing (as there's always the chance that our fix for custom templates might affect other scenarios, e.g. pure block themes, or child themes, etc). I'll follow up with a few more concrete notes in a moment... |
Let's try to state what we know: If a custom template is assigned to a page, it's always the first entry in the
So if we can determine that As stated above, we need to make sure if that has any implications for the other scenarios; and specifically, we need to find out the role of this check: gutenberg/lib/full-site-editing/template-loader.php Lines 98 to 101 in 9c242ee
I've seen a number of scenarios described in the descriptions of PRs that modified the algorithm, e.g. #29026, #30599, #31123, or #31336. We'd need to make sure that we're not breaking any of these by reading them, and recreating them in our local testing. This is where unit test coverage would come in immensely helpful -- it could do that for us automatically, and would protect us from regressions, as we further refine the algorithm. |
Exactly my sentiment 👍
Sounds pretty good to me! @WraithKenny It'd be great if you could file a PR to that effect -- it's getting pretty late here, unfortunately 😅
I did a major refactor to this stuff about a year ago but haven't worked with it recently, so I'm not that familiar with the latest changes. Even back then, it was kinda hard to get it right -- the template hierarchy resolution algorithm is complex as it is, and we're extending it here for a whole new breed of themes. Edge cases tend to fall through the cracks easily. My biggest regret is not having added unit test coverage back then. This issue is renewed motivation to finally do that. |
Awesome, I'll give it a spin! |
I was going for a fix for the issue that crept in 10.5. If the code responsible for the issue can be removed completely, as suggested, that's even better! It's getting late here, too, so I'm not able to look into the full history to make sure nothing breaks, but it might be a good idea to ask @aristath to take a look as well. |
Here's a very basic unit test: <?php
/**
* Template_Loader_Test class
*
* @package WordPress
*/
/**
* Tests for the block template loading algorithm.
*/
class Template_Loader_Test extends WP_UnitTestCase {
private static $post;
private static $template_part_post;
public static function wpSetUpBeforeClass() {
switch_theme( 'tt1-blocks' );
gutenberg_register_template_post_type();
gutenberg_register_template_part_post_type();
gutenberg_register_wp_theme_taxonomy();
gutenberg_register_wp_template_part_area_taxonomy();
// Set up template post.
$args = array(
'post_type' => 'wp_template',
'post_name' => 'page',
'post_title' => 'Page',
'post_content' => 'Content',
'post_excerpt' => 'Description of my template',
'tax_input' => array(
'wp_theme' => array(
get_stylesheet(),
),
),
);
self::$post = self::factory()->post->create_and_get( $args );
wp_set_post_terms( self::$post->ID, get_stylesheet(), 'wp_theme' );
}
public static function wpTearDownAfterClass() {
wp_delete_post( self::$post->ID );
}
function test_gutenberg_page_template() {
$type = 'page';
$templates = array(
'page-slug.php',
'page-1.php',
'page.php',
);
$resolved_template_path = gutenberg_override_query_template( $custom_page_template_path, $type, $templates );
$this->assertEquals( plugin_dir_path( __DIR__ ) . 'lib/template-canvas.php', $resolved_template_path );
}
function test_gutenberg_custom_page_template() {
$custom_page_template = 'templates/full-width.php';
$custom_page_template_path = get_stylesheet_directory() . '/' . $custom_page_template;
$type = 'page';
$templates = array(
$custom_page_template,
'page-slug.php',
'page-1.php',
'page.php',
);
$resolved_template_path = gutenberg_override_query_template( $custom_page_template_path, $type, $templates );
$this->assertEquals( $custom_page_template_path, $resolved_template_path );
}
} This fails on current |
Let's do the following:
Once that short-term fix is out:
Since it's very late here now, I won't do the release tonight. I'll continue work tomorrow, unless someone from @WordPress/gutenberg-core beats me to it. |
Thanks @ockham! Could you create these follow-up issue(s)? |
Thanks to all of you !!! |
The block template resolution algorithm is quite complex, as it has to cover quite a lot of different possible cases resulting from various combinations of - WP's "old" template hierarchy (including custom page templates, and child themes) - block templates. Things become especially complex when there are e.g. "oldschool" PHP templates with higher specificity than the available block templates; or when a child theme has a PHP template with equal specificity as the parent theme's corresponding block template. For more discussion on this, see #31399. Examples of previous iterations that sought to refine the algorithms' behavior include #29026, #30599, #31123, and #31336. This PR's goal is to eventually cover all of those cases.
Description
Using 10.5.3 on some site breaks page rendering.
My home page is binded to a specific page template but with Gutenberg 10.5 WordPres uses index.php template instead.
I'm using understrap child theme. The index.php was really simple, just to display title/excerpt, but of course I never use it.
Installing Gutenberg breaks the look by using index.php.
Step-by-step reproduction instructions
add a customised template to a page : WordPress render it using index.php template but not the required template.
change it to default template : WordPress uses page.php template
Expected behaviour
DIsplay as defined and rendered without gutenberg plugin.
Actual behaviour
Wrong look due to wrong template
WordPress information
Device information
The text was updated successfully, but these errors were encountered: