Make WordPress Core

Opened 18 months ago

Closed 17 months ago

Last modified 16 months ago

#57630 closed defect (bug) (fixed)

Site Editor: Template part customization is not saved for certain child themes

Reported by: mreishus's profile mreishus Owned by: hellofromtonya's profile hellofromTonya
Milestone: 6.2 Priority: normal
Severity: normal Version: 6.2
Component: Themes Keywords: has-testing-info fixed-major
Focuses: Cc:

Description (last modified by SergeyBiryukov)

Issue Summary

For some FSE child themes, when using the site editor to make the first customization of a template part, the edit appears to save but does not display on the front-end of the site.

The bug applies to editing template parts that do not exist in the child theme.

Steps to reproduce

Testing Notes - Certain types of child themes trigger the bug

The bug is related to child themes, but only certain types. Both Twenty Twenty-Two (Red) - WordPress.com and Geologist are child themes, but only the former is affected by the bug.

Child themes that have a template part present in both the child theme and the parent theme (example parts/header.html) has shown to allow template editing. However, child themes that don’t have a specific template in, but the parent theme does, will not allow template editing.

Testing Notes - Bug only occurs when making the first customization to a non-customized template part

If the header has already been customized, the bug cannot happen: You will always be able to edit an already customized header.

For example: When following the above directions with a fresh site, I will not be able to edit the header. After applying the patch below, I will be able to edit the header.
After one edit is saved, even if I rollback the patch and remove the fix, I’m still able to apply more edits. The bug will only reappear if I use the “Clear customizations” for that template part in the site editor. (Site editor -> Middle Top Bar -> “Browse all templates” -> Template Parts -> Header -> Three dots menu -> Clear customization).

Related change

Reverting the change introduced in #55437 fixes the issue:

diff --git a/src/wp-includes/block-template-utils.php b/src/wp-includes/block-template-utils.php
index acbb7724c5..55ad6e15d0 100644
--- a/src/wp-includes/block-template-utils.php
+++ b/src/wp-includes/block-template-utils.php
@@ -503,7 +503,8 @@ function _build_block_template_result_from_file( $template_file, $template_type
 
       $template                 = new WP_Block_Template();
       $template->id             = $theme . '//' . $template_file['slug'];
-       $template->theme          = ! empty( $template_file['theme'] ) ? $template_file['theme'] : $theme;
+       //$template->theme          = ! empty( $template_file['theme'] ) ? $template_file['theme'] : $theme;
+       $template->theme          = $theme;
       $template->content        = _inject_theme_attribute_in_block_template_content( $template_content );
       $template->slug           = $template_file['slug'];
       $template->source         = 'theme';

However, I cannot determine the exact mechanism why; or how to resolve the issue in #55437 while also resolving this issue.

Attachments (5)

2023-02-03_13-43.png (47.1 KB) - added by mreishus 18 months ago.
Screenshot: Adding the new block to the header and saving
2023-02-03_13-43_1.png (26.5 KB) - added by mreishus 18 months ago.
Screenshot: Changes are not reflected in front end after saving
57630-reproduce1.gif (8.1 MB) - added by hellofromTonya 17 months ago.
Test Report: Can reproduce ❌ - change to the parent's Header template part does not save
57630-parent-header-reproduce.gif (3.0 MB) - added by hellofromTonya 17 months ago.
Test Report: Can reproduce editing the parent's Header template part does not save (even on multiple attempts) ❌
Test-after-revert-r54860.gif (5.0 MB) - added by hellofromTonya 17 months ago.
Test Report: Can save changes to parent's template part after reverting r54860 ✅ Works as expected ✅

Change History (14)

@mreishus
18 months ago

Screenshot: Adding the new block to the header and saving

@mreishus
18 months ago

Screenshot: Changes are not reflected in front end after saving

#1 @hellofromTonya
17 months ago

  • Milestone changed from Awaiting Review to 6.2

Env:

  • Child theme: Twenty Twenty-Two (Red) - WordPress.com
  • Parent theme: Twenty Twenty-Two - WordPress.com
  • Plugins: none
  • WordPress: trunk
  • OS: macOS
  • Browser: Firefox

In a brief investigation, found the following:

  • get_stylesheet() returns the child theme, thus $theme has the child theme's stylesheet name.

This is expected ✅

  • The theme in the $template_file array is pointing to the parent theme.

This is not expected ❌ It should be pointing to the child theme and its template part.

Why is the parent theme info $template_file being passed into the function?

My current thinking:

  • the problem is happening before this function is called, i.e. where the child vs. parent decision making happens for which template part to pass to the function.
  • [54860] is not the root cause, but rather before that changeset, it was hiding this mismatch problem.

This issue will need a backtrace to following the decision making before _build_block_template_result_from_file() is invoked.

Regression or Bug?

On the surface this looks like a regression. Underneath, the bug was hidden but then revealed by [54860].

I'm pulling this into 6.2 for visibility. In the current state, child theme template parts like this example are broken.

Need to make some decisions. Some options might be:

  • Revert [54860] for now until the root cause is found.
  • Deep dive to figure out where the root cause is and fix it. But this will likely trigger another Beta and may not happen for RC1 which is tomorrow.

Pinging @audrasjb @SergeyBiryukov @costdev

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


17 months ago

@hellofromTonya
17 months ago

Test Report: Can reproduce ❌ - change to the parent's Header template part does not save

#3 @SergeyBiryukov
17 months ago

  • Description modified (diff)

@hellofromTonya
17 months ago

Test Report: Can reproduce editing the parent's Header template part does not save (even on multiple attempts) ❌

#4 @hellofromTonya
17 months ago

  • Keywords has-testing-info added

Reproduction Report

This report validates that the issue can be reproduced.

Environment

  • OS: macOS
  • Web Server: wp-env (Docker)
  • WordPress: trunk
  • Browser: Firefox 110.0.1
  • Theme: Twenty Twenty-Two (Red) - WordPress.com version 1.0
  • Active Plugins: none

Actual Results

  • ✅ Yes, I can reproduce the reported issue.

See it in action: 57630-reproduce1.gif

  • Also, changes to the parent's Header template part do not save on repeated attempts.

See it in action: 57630-parent-header-reproduce.gif

Additional Notes

  • The TT2 (Red) theme is a child theme that has 2 template parts:
    • header-large-dark.html
    • header-small-dark.html
  • The parent theme has the following template parts:
    • header.html
    • header-large-dark.html
    • header-small-dark.html

Changes to the child theme's template parts do save on the 1st and multiple saves. But changes to the parent's header.html template part does not save on any save attempt.

@hellofromTonya
17 months ago

Test Report: Can save changes to parent's template part after reverting r54860 ✅ Works as expected ✅

#5 @hellofromTonya
17 months ago

Test Report

After reverting [54860], works as expected ✅

  • I was able to save changes to the parent theme's Header template part.
  • Also retried editing and saving the child theme's Header (Dark, small) template part. Worked as expected.

See it in action Test-after-revert-r54860.gif

Notes

But I don't yet understand why or how [54860] is affecting the issue.

#6 @hellofromTonya
17 months ago

For clarity, adding testing instructions.

Testing Instructions

These steps define how to test the feature or enhancement, and indicates the expected behavior or results.

Set up

  • Download the parent and child themes:
  • Add both themes:
    • Go to Appearance > Themes
    • Click on "Add New"
    • Click "Upload Theme"
    • Drag and drop the zip file
    • Click "Install" Now button
    • Repeat for the other theme
  • Activate the TT2 Red child theme

Steps to Test

  1. Open the Site Editor.
  2. Click on the "WordPress Develop" header area to edit.
  3. Select the List View icon, which opens the List View.
  4. Expand each node in Header (Dark, small)
  5. In the Header (Dark, small) > Group > Header > Group > Row > Row, click the "Options" (i.e. 3 vertical dots icon) and select "Insert after". A paragraph block is added between the logo/site name and navigation.
  6. In that paragraph block, type: Parent's Header template part .
  7. In the Header (Dark, small) > Group > Header, click on the Header's "Options" (ie 3 vertical dots icon) and select "Insert after". A paragraph block is added after the logo, site name, and navigation.
  8. In that paragraph block, type: Child's Header (Dark, small) template part .
  9. Click on "Save" button twice.
  10. Refresh your browser.

Expected Results

Lists each expected result or behavior, i.e. what should happen when running the test(s):

  • ✅ The paragraph in the parent's Header template part should still be there are refreshing, meaning it was saved.
  • ✅ The paragraph in the child's Header (Dark, small) template part should still be there are refreshing, meaning it was saved.

Test Report Icons:
✅ <= Behavior is expected.
❌ <= Behavior is NOT expected.

#7 @hellofromTonya
17 months ago

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

In 55493:

Site Editor: Revert r54860.

[54860] caused a regression. Changes to a parent theme's template part (i.e.e when a child theme does not override that template part) no longer saved in the Site Editor. Reverting the changeset resolves the regression.

Props mreishus, hellofromTonya, azaozz, ironprogrammer, antonvlasenko.

Fixes #57630.
See #55437.

#8 @hellofromTonya
17 months ago

Thank you @mreishus for reporting this regression. I'm sorry that your ticket was missed earlier in the 6.2 cycle.

The changeset that caused this regression has been reverted. The reasonings are shared in #55437#comment:10.

#9 @hellofromTonya
16 months ago

  • Keywords fixed-major added
Note: See TracTickets for help on using tickets.