Make WordPress Core

Opened 22 months ago

Closed 12 months ago

Last modified 12 months ago

#62755 closed enhancement (fixed)

Allow template duplication + concept of active templates

Reported by: ellatrix Owned by: ellatrix
Priority: normal Milestone: 6.9
Component: Editor Version: 6.9
Severity: normal Keywords: gutenberg-merge has-patch
Cc: Focuses:

Description (last modified by ellatrix)

Creating this Trac ticket to signal that ​https://github.com/WordPress/gutenberg/pull/67125 has to be backported in the 6.9 release cycle.

Change History (24)

This ticket was mentioned in ​PR #8063 on ​WordPress/wordpress-develop by ​@ellatrix.


22 months ago
#1

  • Keywords has-patch added

This is a stub PR to signal that ​https://github.com/WordPress/gutenberg/pull/67125 has to backported for the 6.8 WP release.

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

This ticket was mentioned in ​Slack in #core-editor by poena. ​View the logs.


21 months ago

This ticket was mentioned in ​Slack in #core-test by oglekler. ​View the logs.


21 months ago

#4 @Mamaduka
21 months ago

  • Milestone 6.8 → Future Release

#5 @ellatrix
13 months ago

  • Milestone Future Release → 6.9

#6 @wildworks
12 months ago

  • Keywords gutenberg-merge added

#7 @ellatrix
12 months ago

  • Description modified (diff)
  • Keywords has-patch removed

#8 @wildworks
12 months ago

  • Type defect (bug) → enhancement

My understanding is that this ticket is an enhancement, not a bug, and should be committed before 6.9 Beta1.

#9 @ellatrix
12 months ago

  • Owner set to ellatrix
  • Resolution → fixed
  • Status new → closed

In 61029:

Templates: add PHP changes required for the template activation feature.

  • Adds the active_templates setting, which is an object holding the template slug as a key and template post ID as the value.
  • To maintain backwards compatibility, any wp_template (post type) not created through the new API will be activated.
  • get_block_template and get_block_templates have been adjusted to check active_templates. These functions should never return inactive templates, just like before, to maintain backwards compatibility.
  • The pre-existing /templates endpoint and sub-endpoints remain and work exactly as before.
  • A new endpoint /wp_template has been added, but this is just a regular posts controller (WP_REST_Posts_Controller). We do register an additional theme field and expose the is_wp_suggestion meta.
  • Another new endpoint /wp_registered_template has been added, which is read-only and lists the registered templates from themes and plugin (un-edited, without activations applied).

These changes are to be iterated on.

See ​https://github.com/WordPress/wordpress-develop/pull/8063.

Props ellatrix, shailu25, ntsekouras.
Fixes #62755.

#10 @ellatrix
12 months ago

In 61033:

Templates: add missing file after [61029].

See #62755.

#11 @dd32
12 months ago

  • Resolution fixed
  • Status closed → reopened

Hey @ellatrix,

This has started to produce some PHP Warnings on WordPress.org with the ​https://github.com/wordpress/wporg-main-2022/ theme.

I'm unsure if the theme does something wrong, or if it's related to this change.

It appears to be related to a 404 condition not setting the templates correctly.

Eg: ​https://wordpress.org/download/?author=1

E_WARNING: Undefined array key "page-download" in wp-includes/block-template.php:246

Debug on line 242 of [61029] with print_r( compact ( 'slug_priorities', 'templates' ) ); reveals:

Array
(
    [slug_priorities] => Array
        (
            [404] => 0
        )
    [templates] => Array
        (
            [0] => WP_Block_Template Object
                (
                    [type] => wp_template
                    [theme] => wporg-main-2022
                    [slug] => 404
                    [id] => wporg-main-2022//404
                    ....
                )

            [1] => WP_Block_Template Object
                (
                    [type] => wp_template
                    [theme] => wporg-main-2022
                    [slug] => page-download
                    [id] => wporg-main-2022//page-download
                    ......
                )
        )
)

Prior to [61029] it's this:

Array
(
    [slug_priorities] => Array
        (
            [404] => 0
        )
    [templates] => Array
        (
            [0] => WP_Block_Template Object
                (
                    [type] => wp_template
                    [theme] => wporg-main-2022
                    [slug] => 404
                    [id] => wporg-main-2022//404
                    ....
                )
        )
)

Also appears to affect this theme, which I'm assuming is because the URL is a 404 unless logged in.

https://github.com/WordPress/wporg-showcase-2022

URL (logged out) https://wordpress.org/showcase/submit-a-wordpress-site/

E_WARNING: Undefined array key "page-submit" in wp-includes/block-template.php:246

Hopefully that's enough information to reproduce and fix?

#12 @ellatrix
12 months ago

Thanks @dd32, I will have a look.

#13 @ellatrix
12 months ago

@dd32 Interesting. What does $object and $specific_template return? Could it be that this returns a slug for this URL even though it's a 404? 🤔

$object = get_queried_object();
$specific_template = $object ? get_page_template_slug( $object ) : null;

#14 @dd32
12 months ago

@ellatrix Yup, that's exactly it.

get_queried_object() is IMHO very.. annoying :) When you start to mix query types (Author/post/archive/taxonomy/taxonomy2/etc) it can return something unexpected.

Eg: ​https://wordpress.org/download/?author=1

In this case, get_queried_object() is the Downloads page WP_Post, and specific template is page-download

Changing it to $specific_template = $object && ! is_404() ? get_page_template_slug( $object ) : null; does work, but I feel that is_404() might also be truthful for intentionally empty archives.

It might be more unexpected that /page/?author=x even causes a 404, and that ?author should be ignored on a singular query like this.

This is a much larger issue with Template selection and WP_Query generation than this ticket :) I think this should just try to avoid ending up generating a PHP warning..

#15 @dd32
12 months ago

Oh here's another one, Query Monitor can trigger a similar warning, when calling other template functions. This is probably less expected though.

WARNING: wp-includes/block-template.php:247 - Undefined array key "page-downloads"
require('wp-blog-header.php'), require_once('wp-includes/template-loader.php'), do_action('template_redirect'), WP_Hook->do_action, WP_Hook->apply_filters, QM_Collector_Theme->action_template_redirect, get_index_template, get_query_template, locate_block_template, resolve_block_template, usort, {closure}

In that case, $object and all that will be as expected, but the templates will be more limited:

slug_priorities' => 
    array (size=1)
      'index' => int 0
 'templates' => 
    array (size=2)
      0 => 
        object(WP_Block_Template)[3977]
          public 'type' => string 'wp_template' (length=11)
          public 'theme' => string 'wporg-main-2022' (length=15)
          public 'slug' => string 'index' (length=5)
          .....
      1 => 
        object(WP_Block_Template)[3981]
          public 'type' => string 'wp_template' (length=11)
          public 'theme' => string 'wporg-main-2022' (length=15)
          public 'slug' => string 'page-download' (length=22)
          .....
  'object' => 
    object(WP_Post)[3829]
      .....
      public 'post_name' => string 'downloads'

#16 @ellatrix
12 months ago

@dd32 Yeah, it looks like we need to make sure that the queried object is a post in addition to also checking that all templates added to that array have a slug that is actually requested. How urgent is this to fix? I'll probably make a quick patch in a bit but I'm wondering how I can refactor the entire thing as well to. Perhaps there's a way to avoid getting the post template altogether.

#17 @dd32
12 months ago

@ellatrix For WordPress.org: Not at all. I just needed to be sure it was tracked and aware of the breakage before release.

#18 @ellatrix
12 months ago

In 61054:

Template activation: fix unique slug filtering.

The logic introduced for the unique slug filter in [61033] is incorrect. We should return the desired slug for the wp_template post type. This causes any second created template targeting a certain slug to not work correctly.

Developed in ​https://github.com/WordPress/wordpress-develop/pull/10397.

See #62755.
Fixes #64141.
Props ntsekouras, priethor.

#19 @wildworks
12 months ago

Can we close this ticket? Are there any other issues that need to be addressed before closing it?

This ticket was mentioned in ​PR #10425 on ​WordPress/wordpress-develop by ​@ellatrix.


12 months ago
#20

  • Keywords has-patch added

#21 @westonruter
12 months ago

I just discovered a problem with the wp_registered_template post type. It is too long. The maximum length allowed in the database schema is 20 characters. Now, maybe this post type isn't actually inserted into the database, but its presence in $wp_post_types can cause very confusing problems if it is attempted. For example, a plugin which loops over all registered post types to create posts after . See my ​PR comment.

I thin a shorter name should be used, like wp_registered_tmpl, wp_register_template, or registered_template.

#22 @westonruter
12 months ago

Ah, this is eliminated as of <​https://github.com/WordPress/wordpress-develop/pull/10425>. So, nevermind I guess!

#23 @ellatrix
12 months ago

  • Resolution → fixed
  • Status reopened → closed

In 61078:

Template activation: merge changes for Beta 2.

Developed in ​https://github.com/WordPress/wordpress-develop/pull/10425.
See https://core.trac.wordpress.org/ticket/62755.

Fixes #62755.
Props ellatrix, priethor.

Note: See TracTickets for help on using tickets.