FSE: Incorrect template archive.html used for custom post type archive

bobbingwide commented 3 years ago

Describe the bug When I updated to Gutenberg 9.4.0 the template file being used for the archive display of my Blocks CPT changed to archive.html. Previously, in Gutenberg 9.3.0 the correct template file was being used: archive-block.html.

Looking at gutenberg_resolve_template() I can't see how it can determine the correct template to use without reference to the post type or taxonomy.

It's the same problem for single templates. I've not tried post ID or post name specific templates but I imagine it's the same.

To reproduce

Steps to reproduce the behavior:

Create a hierarchical CPT called block that supports archives.
Create archive-block.html and archive.html template files.
View the archive for the blocks e.g. example.com/block
See that it uses the wrong template

Expected behavior With 9.3.0 the template used was archive-block.

Screenshots Gutenberg 9.4.0 on the left, 9.3.0 on the right. Both using the same Experimental FSE theme - Fizzie with no custom wp_template or wp_template_part posts except the 'auto-draft's.

Editor version (please complete the following information):

WordPress version: [e.g: 5.3.2] 5.6-beta4 on the left 5.5.1 on the right
Does the website has Gutenberg plugin installed, or is it using the block editor that comes by default? [e.g: "gutenberg plugin", "default"] 9.4.0 on the left. 9.3.0 on the right
If the Gutenberg plugin is installed, which version is it? [e.g., 7.6]

Desktop (please complete the following information):

OS: [e.g. iOS] Windows
Browser [e.g. chrome, safari] Chrome
Version [e.g. 22] latest

Additional context

This trace output shows the values of variables

$slugs

C:\apache\htdocs\wp56\wp-content\plugins\gutenberg\lib\template-loader.php(178:0) 
gutenberg_resolve_template(5) 83 0 2020-11-21T22:17:50+00:00 1.072874 0.000296 
cf=archive_template 10433 21 974 20971520/20971520 256M F=651 slugs Array
(
    [0] => archive-block
    [1] => archive
)

$templates

C:\apache\htdocs\wp56\wp-content\plugins\gutenberg\lib\template-loader.php(179:0) 
gutenberg_resolve_template(6) 84 0 2020-11-21T22:17:50+00:00 1.073035 0.000161 
cf=archive_template 10433 21 974 20971520/20971520 256M F=651 templates Array
(
    [0] => WP_Post Object
        (
            [ID] => 1332
            [post_author] => 1
            [post_date] => 2020-11-21 20:55:22
            [post_date_gmt] => 0000-00-00 00:00:00
            [post_content] => archive-block.html

<!-- wp:template-part {"slug":"header-2-columns","theme":"fizzie"} /-->

<!-- wp:template-part {"slug":"block-a2z-pagination","theme":"fizzie"} /-->

<!-- wp:template-part {"slug":"breadcrumbs","theme":"fizzie","className":"breadcrumbs"} /-->

<!-- wp:template-part {"slug":"archive-query","theme":"fizzie","tagName":"section","className":"archive"} /-->

<!-- wp:template-part {"slug":"search","theme":"fizzie"} /-->

<!-- wp:template-part {"slug":"page-footer","theme":"fizzie"} /-->

<!-- wp:template-part {"slug":"footer","theme":"fizzie"} /-->

<!-- wp:template-part {"slug":"footer-menu","theme":"fizzie", "className":"footer-menu"} /-->

            [post_title] => archive-block
            [post_excerpt] => 
            [post_status] => auto-draft
            [comment_status] => closed
            [ping_status] => closed
            [post_password] => 
            [post_name] => archive-block
            [to_ping] => 
            [pinged] => 
            [post_modified] => 2020-11-21 20:55:22
            [post_modified_gmt] => 0000-00-00 00:00:00
            [post_content_filtered] => 
            [post_parent] => 0
            [guid] => https://s.b/wp56/?post_type=wp_template&p=1332
            [menu_order] => 0
            [post_type] => wp_template
            [post_mime_type] => 
            [comment_count] => 0
            [filter] => raw
        )

    [1] => WP_Post Object
        (
            [ID] => 1333
            [post_author] => 1
            [post_date] => 2020-11-21 20:55:22
            [post_date_gmt] => 0000-00-00 00:00:00
            [post_content] => archive.html - why this one?

<!-- wp:template-part {"slug":"header-2-columns","theme":"fizzie"} /-->

<!-- wp:template-part {"slug":"a2z-pagination","theme":"fizzie"} /-->

<!-- wp:template-part {"slug":"breadcrumbs","theme":"fizzie","className":"breadcrumbs"} /-->

<!-- wp:template-part {"slug":"archive-query","theme":"fizzie","tagName":"section","className":"archive"} /-->

<!-- wp:template-part {"slug":"search","theme":"fizzie"} /-->

<!-- wp:template-part {"slug":"page-footer","theme":"fizzie"} /-->

<!-- wp:template-part {"slug":"footer","theme":"fizzie"} /-->

<!-- wp:template-part {"slug":"footer-menu","theme":"fizzie", "className":"footer-menu"} /-->

            [post_title] => archive
            [post_excerpt] => 
            [post_status] => auto-draft
            [comment_status] => closed
            [ping_status] => closed
            [post_password] => 
            [post_name] => archive
            [to_ping] => 
            [pinged] => 
            [post_modified] => 2020-11-21 20:55:22
            [post_modified_gmt] => 0000-00-00 00:00:00
            [post_content_filtered] => 
            [post_parent] => 0
            [guid] => https://s.b/wp56/?post_type=wp_template&p=1333
            [menu_order] => 0
            [post_type] => wp_template
            [post_mime_type] => 
            [comment_count] => 0
            [filter] => raw
        )

)

$slug_priorities

C:\apache\htdocs\wp56\wp-content\plugins\gutenberg\lib\template-loader.php(185:0) 
gutenberg_resolve_template(7) 85 0 2020-11-21T22:17:50+00:00 1.073173 0.000138 
cf=archive_template 10433 21 974 20971520/20971520 256M F=651 slug_priorities Array
(
    [archive-block] => 0
    [archive] => 1
)

After the sort $templates[0] contains the archive post ( ID 1333 ).

aristath commented 3 years ago

I came across this issue today as well, while working on #27128 From what I can tell this behavior was introduced in #26650 and is caused by this part: https://github.com/WordPress/gutenberg/blob/cfb3696b601352884d4acf7a24cccb7410863a92/lib/template-loader.php#L151-L158

@youknowriad what is the purpose of that code? :thinking: From what I can tell it will always just reverse the order of templates? Assuming a recipe post-type, the templates order is [ 'archive-recipe.php', 'archive.php' ]. We strip the slugs so it becomes [ 'archive-recipe', 'archive' ]. We then flip the array, so that boceomes [ 'archive-recipe' => 0, 'archive' => 1 ]. So in these lines here:

$priority_a = $slug_priorities[ $template_a->post_name ] * 2 + ( 'publish' === $template_a->post_status ? 1 : 0 );
$priority_b = $slug_priorities[ $template_b->post_name ] * 2 + ( 'publish' === $template_b->post_status ? 1 : 0 );

priority_a will always be smaller than priority_b and as a result it will simply reverse the order of the array. Am I missing something?

bobbingwide commented 3 years ago

I believe that what's missing is any matching to what was asked for. It just picks the first array entry.

youknowriad commented 3 years ago

The idea there was to give priority to the "published" templates over the "auto-draft" ones but without really affecting the order of the "slug_priorities". Maybe I got the condition inverted inadvertantly.

aristath commented 3 years ago

The idea there was to give priority to the "published" templates over the "auto-draft" ones but without really affecting the order of the "slug_priorities". Maybe I got the condition inverted inadvertantly.

Hmmm so if I have a published archive but the theme includes an archive-recipe template, we want to force the use of the archive template - even if a more specific template exists? Is that the expected behavior? (just trying to understand so I can submit a PR)

bobbingwide commented 3 years ago

I changed the return from the usort to

return $priority_a - $priority_b;

Results:

Archive	Template used	Correct ?
block	archive-block	Yes
oik-plugins	archive-block	No

I then noticed that for archive-oik-plugins there were two auto-drafts, with different authors. The correct one to use is post 1369. - this is the most recently updated template where I'd

corrected the debug message at the top of the template
added a template part that should be missing.

C:\apache\htdocs\wp56\wp-content\plugins\gutenberg\lib\template-loader.php(178:0) gutenberg_resolve_template(5) 90 0 2020-11-26T10:42:17+00:00 0.325696 0.000305 cf=archive_template 10409 21 1257 6291456/6291456 256M F=650 slugs Array
(
    [0] => archive-oik-plugins
    [1] => archive
)

C:\apache\htdocs\wp56\wp-content\plugins\gutenberg\lib\template-loader.php(179:0) gutenberg_resolve_template(6) 91 0 2020-11-26T10:42:17+00:00 0.325874 0.000178 cf=archive_template 10409 21 1257 6291456/6291456 256M F=650 templates Array
(
    [0] => WP_Post Object
        (
            [ID] => 1368
            [post_author] => 0
            [post_date] => 2020-11-25 12:20:54
            [post_date_gmt] => 0000-00-00 00:00:00
            [post_content] => <div class="WP_DEBUG">archive-block.html</div>

<!-- wp:template-part {"slug":"header","theme":"fizzie"} /-->

<!-- wp:template-part {"slug":"a2z-pagination-oik-plugins","theme":"fizzie"} /-->

<!-- wp:template-part {"slug":"breadcrumbs","theme":"fizzie","className":"breadcrumbs"} /-->

<!-- wp:template-part {"slug":"archive-query","theme":"fizzie","tagName":"section","className":"archive"} /-->

<!-- wp:template-part {"slug":"search","theme":"fizzie"} /-->

<!-- wp:template-part {"slug":"page-footer","theme":"fizzie"} /-->

<!-- wp:template-part {"slug":"footer","theme":"fizzie"} /-->

<!-- wp:template-part {"slug":"footer-menu","theme":"fizzie","className":"footer-menu"} /-->
            [post_title] => archive-oik-plugins
            [post_excerpt] => 
            [post_status] => auto-draft
            [comment_status] => closed
            [ping_status] => closed
            [post_password] => 
            [post_name] => archive-oik-plugins
            [to_ping] => 
            [pinged] => 
            [post_modified] => 2020-11-25 12:20:54
            [post_modified_gmt] => 0000-00-00 00:00:00
            [post_content_filtered] => 
            [post_parent] => 0
            [guid] => https://s.b/wp56/?post_type=wp_template&p=1368
            [menu_order] => 0
            [post_type] => wp_template
            [post_mime_type] => 
            [comment_count] => 0
            [filter] => raw
        )

    [1] => WP_Post Object
        (
            [ID] => 1369
            [post_author] => 1
            [post_date] => 2020-11-25 12:20:54
            [post_date_gmt] => 0000-00-00 00:00:00
            [post_content] => <div class="WP_DEBUG">archive-oik-plugins.html</div>
<!-- wp:template-part {"slug":"bodge", "theme":"fizzie" } /-->
<!-- wp:template-part {"slug":"header","theme":"fizzie"} /-->

<!-- wp:template-part {"slug":"a2z-pagination-oik-plugins","theme":"fizzie"} /-->

<!-- wp:template-part {"slug":"breadcrumbs","theme":"fizzie","className":"breadcrumbs"} /-->

<!-- wp:template-part {"slug":"archive-query","theme":"fizzie","tagName":"section","className":"archive"} /-->

<!-- wp:template-part {"slug":"search","theme":"fizzie"} /-->

<!-- wp:template-part {"slug":"page-footer","theme":"fizzie"} /-->

<!-- wp:template-part {"slug":"footer","theme":"fizzie"} /-->

<!-- wp:template-part {"slug":"footer-menu","theme":"fizzie", "className":"footer-menu"} /-->
            [post_title] => archive-oik-plugins
            [post_excerpt] => 
            [post_status] => auto-draft
            [comment_status] => closed
            [ping_status] => closed
            [post_password] => 
            [post_name] => archive-oik-plugins
            [to_ping] => 
            [pinged] => 
            [post_modified] => 2020-11-26 10:40:38
            [post_modified_gmt] => 2020-11-26 10:40:38
            [post_content_filtered] => 
            [post_parent] => 0
            [guid] => https://s.b/wp56/?post_type=wp_template&#038;p=1369
            [menu_order] => 0
            [post_type] => wp_template
            [post_mime_type] => 
            [comment_count] => 0
            [filter] => raw
        )

    [2] => WP_Post Object
        (
            [ID] => 1333
            [post_author] => 1
            [post_date] => 2020-11-21 20:55:22
            [post_date_gmt] => 0000-00-00 00:00:00
            [post_content] => <div class="WP_DEBUG">archive.html - why this one?</div>

<!-- wp:template-part {"slug":"header","theme":"fizzie"} /-->

<!-- wp:template-part {"slug":"a2z-pagination","theme":"fizzie"} /-->

<!-- wp:template-part {"slug":"breadcrumbs","theme":"fizzie","className":"breadcrumbs"} /-->

<!-- wp:template-part {"slug":"archive-query","theme":"fizzie","tagName":"section","className":"archive"} /-->

<!-- wp:template-part {"slug":"search","theme":"fizzie"} /-->

<!-- wp:template-part {"slug":"page-footer","theme":"fizzie"} /-->

<!-- wp:template-part {"slug":"footer","theme":"fizzie"} /-->

<!-- wp:template-part {"slug":"footer-menu","theme":"fizzie", "className":"footer-menu"} /-->

            [post_title] => archive
            [post_excerpt] => 
            [post_status] => auto-draft
            [comment_status] => closed
            [ping_status] => closed
            [post_password] => 
            [post_name] => archive
            [to_ping] => 
            [pinged] => 
            [post_modified] => 2020-11-26 10:42:05
            [post_modified_gmt] => 2020-11-26 10:42:05
            [post_content_filtered] => 
            [post_parent] => 0
            [guid] => https://s.b/wp56/?post_type=wp_template&#038;p=1333
            [menu_order] => 0
            [post_type] => wp_template
            [post_mime_type] => 
            [comment_count] => 0
            [filter] => raw
        )

)

bobbingwide commented 3 years ago

To resolve this additional issue I tried logging out, so that the auto-draft for author 0 was rebuilt. But synchronization didn't kick in. So deleted the auto-drafts, and the two options in settings to force the auto-drafts to be rebuilt. The 35 new autodrafts were all created with post_author = 0.

If I now log in again I expect there'll be new autodrafts created.

bobbingwide commented 3 years ago

I've not tried post ID or post name specific templates but I imagine it's the same.

I have now tested page-$id and page-$slug with the fix applied. The correct templates were used.

youknowriad commented 3 years ago

Hmmm so if I have a published archive but the theme includes an archive-recipe template, we want to force the use of the archive template - even if a more specific template exists? Is that the expected behavior?

No no, if by mistake you end p with both archive-recipe as an auto-draft and one published, we'd prefer published. That's it but archive-recipe should always be more priotary than archive. I believe even the auto-draft/published check might not be required anymore after the refactoring of auto-drafts.

aristath commented 3 years ago

archive-recipe should always be more priotary than archive.

Understood. 👍

I believe even the auto-draft/published check might not be required anymore after the refactoring of auto-drafts.

Yeah, I believe we can just go ahead and replace the logic there with something as simple as this:

usort(
    $templates,
    function ( $template_a, $template_b ) use ( $slug_priorities ) {
        return $slug_priorities[ $template_a->post_name ] - $slug_priorities[ $template_b->post_name ];
    }
);

On my test site that seems to return the right result

youknowriad commented 3 years ago

@aristath Would you be able to create a PR for that?

aristath commented 3 years ago

Done. PR on https://github.com/WordPress/gutenberg/pull/27303 :+1:

noahtallen commented 3 years ago

I think there is another behavior that got broken. Consider the case where you have a theme template named "front-page". You also create your own "front-page" template manually in wp-admin. Both are published. Currently, we are resolving the theme template, but the expected behavior is to resolve the user-created template.

youknowriad commented 3 years ago

@noahtallen I believe that use-case should never happen, you shouldn't be able to create two CPT entries for the same theme and the same template name.

noahtallen commented 3 years ago

I explained more about it here: https://github.com/WordPress/gutenberg/pull/27322#issuecomment-735458427

The thing is that the "theme" gets set automatically to the current theme when you create a template, but in reality it's just something you made yourself

youknowriad commented 3 years ago

It doesn't matter for me, what purpose there is in making two "front-page" templates. It shouldn't be possible.

noahtallen commented 3 years ago

What if I have a customized front-page template on my site, and then I activate another theme which also has a front-page template. In that case, there are going to be two front-page templates, so which one should be shown? I think it should be customized one. The two front-page templates exist -- so do we just not show the one from the freshly activated theme at all?

bobbingwide commented 3 years ago

What if I have a customized front-page template on my site

If your front page was just a bunch of template parts would you expect it to load these from the old theme? My front-page.html file is currently

<div class="WP_DEBUG">front-page.html</div>

<!-- wp:template-part{ "slug":"issue-39", "theme":"fizzie" } /-->

<!-- wp:template-part {"slug":"header","theme":"fizzie"} /-->

... you get the idea.

noahtallen commented 3 years ago

Yeah, that's a good point. I wonder if (at this time) we are marking template parts as customized + published if their parent template is modified.

And then, what if each template part is also customized? Do we discard everything when switching themes, or do we allow users to keep one or two templates which they might want to stay the same?

bobbingwide commented 3 years ago

@noahtallen I'm sure you're aware that this whole business of template and template part loading and reconciliation is currently being reviewed by a number of parties, working on different Issues and PRs.

I think that what's missing is a documented set of expectations for the Site Editor's and front end's behaviour when certain events occur. Requirements and proposed solutions are required for:

switching themes
theme updates
multiple language versions
child themes
hybrid themes

I've been looking at supporting multiple language versions. I intend to document my proposals today. They are based on prototype work I've been doing in bobbingwide/oik-i18n/issues/7 and bobbingwide/fizzie/issues/46.

bobbingwide commented 3 years ago

I've been looking at supporting multiple language versions. I intend to document my proposals today.

See #27402

WordPress / gutenberg

FSE: Incorrect template archive.html used for custom post type archive #27177