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.

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

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

Closed
bobbingwide opened this issue Nov 21, 2020 · 20 comments · Fixed by #27303
Closed

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

bobbingwide opened this issue Nov 21, 2020 · 20 comments · Fixed by #27303
Assignees
Labels
[Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Needs Testing Needs further testing to be confirmed. [Status] In Progress Tracking issues with work in progress

Comments

@bobbingwide
Copy link
Contributor

bobbingwide commented Nov 21, 2020

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:

  1. Create a hierarchical CPT called block that supports archives.
  2. Create archive-block.html and archive.html template files.
  3. View the archive for the blocks e.g. example.com/block
  4. 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.

image

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 ).

@gziolo gziolo added [Feature] Full Site Editing Needs Testing Needs further testing to be confirmed. labels Nov 21, 2020
@aristath aristath added the [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. label Nov 25, 2020
@aristath
Copy link
Member

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:

usort(
$templates,
function ( $template_a, $template_b ) use ( $slug_priorities ) {
$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 );
return $priority_b - $priority_a;
}
);

@youknowriad what is the purpose of that code? 🤔
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
Copy link
Contributor Author

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

@youknowriad
Copy link
Contributor

youknowriad commented Nov 26, 2020

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
Copy link
Member

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
Copy link
Contributor Author

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

  1. corrected the debug message at the top of the template
  2. 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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor

youknowriad commented Nov 26, 2020

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
Copy link
Member

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
Copy link
Contributor

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

@aristath
Copy link
Member

Done. PR on #27303 👍

@noahtallen
Copy link
Member

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.

Screen Shot 2020-11-27 at 2 42 11 PM

@youknowriad
Copy link
Contributor

@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
Copy link
Member

I explained more about it here: #27322 (comment)

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
Copy link
Contributor

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

@noahtallen
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

@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
Copy link
Contributor Author

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

See #27402

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Needs Testing Needs further testing to be confirmed. [Status] In Progress Tracking issues with work in progress
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants