Theme live preview is broken

[karl94]

The newly added memoization in 6.4 for get_stylesheet_directory() and get_template_directory() breaks themes live preview.

​https://github.com/WordPress/wordpress-develop/commit/ec21b604e0527cff54b774adcb9631036d6b57ec#diff-b9f1810ad43acfa11ba58a9f21eb0ee3a8063c80aa155de9b210323252534716R190-R223

How to reproduce:

• Load a WP 6.4 beta or RC (playground link: ​https://playground.wordpress.net/?theme=twentynineteen&lazy&wp=beta)
• Install some classic themes e.g. twentytwenty and twentynineteen
• Activate twentynineteen
• Live preview twentytwenty
• The preview is broken: the HTML comes from twentynineteen but the stylesheet is from twentytwenty

[karl94]

@flixos90 can you give a look at this?

It appears that WP is invoking get_stylesheet_directory() before that WP_Customize_Manager attaches its filters for previewing the theme.

WP_Customize_Manager ​adds its preview filters in WP_Customize_Manager::start_previewing_theme(), which is called from WP_Customize_Manager::setup_theme(), which ​is run on action setup_theme at default priority (10).

The first invocation of get_stylesheet_directory() appears to be from wp_theme_has_theme_json() (​https://github.com/WordPress/wordpress-develop/blob/9352b2a/src/wp-includes/global-styles-and-settings.php#L399) which is called by wp_enable_block_templates() (​https://github.com/WordPress/wordpress-develop/blob/9352b2a/src/wp-includes/theme-templates.php#L227) yet again during the hook setup_theme at priority 10 (​https://github.com/WordPress/wordpress-develop/blob/9352b2a/src/wp-includes/default-filters.php#L721).

So, both wp_enable_block_templates() and WP_Customize_Manager::setup_theme() are attached to the same hook at the same priority, therefore they get invoked in the same order they were attached to the hook.

default-filters.php is executed before, then the callback order is: first wp_enable_block_templates(), then WP_Customize_Manager::setup_theme().

Because get_stylesheet_directory() is used inside wp_enable_block_templates(), the global variable $wp_stylesheet_path is set, caching $stylesheet_dir. Any following get_stylesheet_directory() call will always return the same value.

The ​cache-bypass logic doesn't help in this case since when get_stylesheet_directory() is first invoked (via wp_enable_block_templates()) the customizer hasn't added its filter yet.

get_template_directory() has the same issue.

Maybe a doing_it_wrong() should be added to both get_stylesheet_directory() and get_template_directory() to warn of every usage before or during the hook setup_theme? And bypass their cache until that hook completes?

[flixos90]

Thank you for the report @karl94! I have investigated the problem and identified the root cause: There are two default filters (_add_default_theme_supports(), and wp_enable_block_templates() as you've mentioned), which are hooked into the setup_theme action to add_theme_support() for certain features.

This is incorrect, as theme support should only be added on the after_setup_theme hook. Using the setup_theme hook does not only introduce the above bug, but leads to other problems as well as the theme can only be expected to be completely set up after the setup_theme action. For example, at the moment these callbacks will add theme support based on the current theme which then would still be maintained even when previewing another theme in the Customizer.

It looks like these problems were introduced in [51199] and [52369] respectively, therefore pinging @jorgefilipecosta and @audrasjb for visibility.

This should be relatively straightforward to fix by relying on the appropriate action after_setup_theme instead. There is no relevant usage of these functions in the plugin repository which would be affected by this change, see ​https://wpdirectory.net/search/01HDKY6F7BJ58ZD7H6JQ8K4P3S and ​https://wpdirectory.net/search/01HDKY998M6WV6TRE2WR3ZCYG6.

This fixes the bug of using the setup_theme action for adding theme support, which is incorrect for such purposes, as at that point the current theme is not set up yet. For example, Customizer previewing, which may exchange the current theme at runtime only fires on setup_theme.

Therefore any code depending on the current theme must only run after that, i.e. on after_setup_theme. This is also the hook which theme authors have been encouraged for more than a decade to use to add theme supports.

See https://core.trac.wordpress.org/ticket/59732#comment:2 for additional context.

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

This fixes the bug of using the setup_theme action for adding theme support, which is incorrect for such purposes, as at that point the current theme is not set up yet. For example, Customizer previewing, which may exchange the current theme at runtime only fires on setup_theme.

Therefore any code depending on the current theme must only run after that, i.e. on after_setup_theme. This is also the hook which theme authors have been encouraged for more than a decade to use to add theme supports.

See https://core.trac.wordpress.org/ticket/59732#comment:2 for additional context.

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

[flixos90]

I have implemented ​https://github.com/WordPress/wordpress-develop/pull/5574 as a fix for this bug. It addresses the root problem and through that the concrete broken user experience reported: With the PR applied, the Customizer-previewed theme's data is correctly loaded, without any data from the original theme bleeding over.

I share the concerns @joemcgill expressed:

could there be some unintentional side effects where some block data or settings get instantiated between setup_theme and after_setup_theme and assume that theme support has already been established?

While it fixes Customizer theme live preview, what might it do to block powered sites? Is something dependent upon on it? Not sure, but should be validated with careful testing within the editors and front-end.

@hellofromtonya

While it fixes Customizer theme live preview, what might it do to block powered sites? Is something dependent upon on it? Not sure, but should be validated with careful testing within the editors and front-end.

I have checked the WordPress core codebase for further usage of the setup_theme action, and I couldn't find any relevant instances where delaying the execution of _add_default_theme_supports() and wp_enable_block_templates() could lead to a problem. In fact the hook is only used to register the available theme features and for the Customizer preview.

@hellofromtonya I've tested this PR using the Site Editor for a bit, with both Twenty Twenty-Three and Twenty Twenty-Four, and couldn't find any problems. It also fixes the original Customizer bug. I personally think this change is safe to make, though of course any additional testing would be appreciated.

[hellofromTonya]

Patch: ​https://github.com/WordPress/wordpress-develop/pull/5574

The patch changes the "when" (to after the theme is set up) for:

• adding block theme's theme supports (including 'block-templates'),
• turning on 'should_load_separate_core_block_assets',
• and removing Customizer's Menu panel.

Testing instructions need to test for:

• the bug reported (i.e. Customizer themes live preview)
• and the above changes for block themes, i.e. to ensure the patch does not change the behavior.

Test Instructions: Customizer themes live preview bug

Steps to Reproduce

Though listed in the ticket's description, repeating here to keep all testing instructions in one place:

1. Load WordPress 6.4 RC2
2. Install some classic themes e.g. twentytwenty and twentynineteen
3. Activate twentynineteen
4. Live preview twentytwenty 🐞 Bug occurs.

Expected Results

When reproducing a bug:

• ❌ Before applying the patch, the live preview should not work.

When testing a patch to validate it works as expected:

• ✅ After applying the patch, the live preview should work.

Test Instructions: Block themes not affected by the patch

Though listed in the ticket's description, repeating here to keep all testing instructions in one place:

1. Load WordPress 6.4 RC2.
2. With a classic theme, such as Twenty Twenty-One (TT1) activated, open Customizer in a different browser tab or ease of access in the next steps.
   • Notice: The "Menus" item in the navigation sidebar.
3. Activate Twenty Twenty-Four (TT4).
4. Refresh Customizer. Expected: The "Menus" item should not be in the navigation sidebar. ✅
5. Apply the patch.
6. Repeat step 4. Expected: No "Menus" item ✅
7. Navigate to Posts > and open a post.
   • Expected: "Featured image" subpanel should render in the right sidebar and be functional. ✅

Expected Results

After applying the patch, the following should still work for block themes:

• ✅ In Customizer, the "Menus" item should not in the navigation sidebar.
• ✅ When adding or editing a post, the "Featured image" subpanel should render and be functional.

Test Report Icons:
🐞 <= Indicates where issue ("bug") occurs.
✅ <= Behavior is expected.
❌ <= Behavior is NOT expected.

[hellofromTonya]

Test Report: After applying PR 5574 - resolves the bug ✅

[hellofromTonya]

Test Report: After applying PR 5574, with block theme TT4 activated, Menus does not show and live preview (for classic theme) still works ✅

[hellofromTonya]

Environment

• OS: macOS
• Localhost: wp-env Docker
• PHP: 7.4
• WordPress: trunk
• Browser: Firefox
• Theme: TT1 and TT0 (classic themes) and TT4 (block theme)
• Active Plugins: none

Actual Results: For Live Theme Preview bug

✅ After applying the patch, the live preview works.

See test-report-theme-live-preview.png​

Actual Results: For Block Theme

✅ In Customizer, the "Menus" item does not show in the navigation sidebar.
✅ In Customizer, selecting TT0, live preview works.

See test-report-block-theme-in-customizer.png​

✅ When adding or editing a post, the "Featured image" subpanel still renders and is functional.

[hellofromTonya]

Patch: ​https://github.com/WordPress/wordpress-develop/pull/5574

With multiple testing reports and the patch being approved by 2 committers, the patch is ready for commit.

To expedite, the patch is also okay to backport to the 6.4 branch.

[hellofromTonya]

Test Report: Bug before the patch: Live Preview in WP 6.3.2 (left) ✅ vs 6.4 RC (right) ❌

[hellofromTonya]

Additional information why this regression should be included in 6.4 RC4:

Without the patch applied (i.e. before the patch), test-report-6.3.2v6.4-rc2.png​ shows theme live preview:

• Works with WordPress 6.3.2 ✅
• Broken with WordPress 6.4. RC2 ❌

With the live theme preview experience broken, a fix is indeed needed. As previously noted, ​the patch resolves the issue without impacting block themes.

[flixos90]

In 57009:

Themes: Fix block theme supports being added too early, leading to Customizer live preview bugs in 6.4.

The Customizer live preview broke because of [56635], however the root cause for the bug was a lower-level problem that had been present since WordPress 5.8: The block theme specific functions _add_default_theme_supports() and wp_enable_block_templates() were being hooked into the setup_theme action, which fires too early to initialize theme features. Because of that, theme functionality would be initialized before the current theme setup being completed. In the case of the Customizer, that includes overriding which theme is the current theme entirely, thus leading to an inconsistent experience.

This changeset fixes the bug by moving those two callbacks to the after_setup_theme action, which is the appropriate action to initialize theme features.

Props karl94, hellofromTonya, joemcgill, flixos90.
Fixes #59732.
See #18298, #53397, #54597.

Committed in https://core.trac.wordpress.org/changeset/57009

[flixos90]

Reopening for backport.

[flixos90]

In 57010:

Themes: Fix block theme supports being added too early, leading to Customizer live preview bugs in 6.4.

The Customizer live preview broke because of [56635], however the root cause for the bug was a lower-level problem that had been present since WordPress 5.8: The block theme specific functions _add_default_theme_supports() and wp_enable_block_templates() were being hooked into the setup_theme action, which fires too early to initialize theme features. Because of that, theme functionality would be initialized before the current theme setup being completed. In the case of the Customizer, that includes overriding which theme is the current theme entirely, thus leading to an inconsistent experience.

This changeset fixes the bug by moving those two callbacks to the after_setup_theme action, which is the appropriate action to initialize theme features.

Props karl94, hellofromTonya, joemcgill, flixos90.
Merges [57009] to the 6.4 branch.
Fixes #59732.
See #18298, #53397, #54597.