Make WordPress Core

Opened 13 years ago

Closed 8 months ago

Last modified 3 months ago

#18298 closed enhancement (fixed)

deprecate TEMPLATEPATH and STYLESHEETPATH

Reported by: aaroncampbell's profile aaroncampbell Owned by: flixos90's profile flixos90
Milestone: 6.4 Priority: normal
Severity: normal Version: 3.0
Component: Themes Keywords: has-patch has-unit-tests commit add-to-field-guide
Focuses: template, performance Cc:

Description (last modified by aaroncampbell)

As part of #15086 we wanted to add unit tests, but we came across a problem in #UT22. Basically locate_template() uses these constants instead of get_template_directory() and get_stylesheet_directory() which makes it impossible to use switch_theme(). It looks like the constants are only used in about 20 places through core, so I recommend we fix those places and deprecate the constants.

Attachments (4)

18298.diff (3.7 KB) - added by aaroncampbell 13 years ago.
18298.2.diff (4.3 KB) - added by aaroncampbell 13 years ago.
18298.3.diff (4.4 KB) - added by aaroncampbell 13 years ago.
18298.4.diff (4.7 KB) - added by obenland 10 years ago.

Download all attachments as: .zip

Change History (96)

#1 @scribu
13 years ago

20 places isn't exactly few, but I agree that functions are better than constants, in this case.

#2 @lloydbudd
13 years ago

deprecate the constants

This would be very well received by the WordPress.com VIP team. We are regularly cleaning up TEMPLATEPATH and STYLESHEETPATH, replacing with the get_ functions in VIP partners themes' function to ensure features like mobile theme plugins and post-by-email work which don't go through initialization of those constants.

#3 @toscho
13 years ago

  • Cc info@… added

@aaroncampbell
13 years ago

#4 @aaroncampbell
13 years ago

  • Keywords has-patch added; needs-patch removed

The patch removes all core uses of the constants. It's still used in twentyten and twentyeleven, so those should be fixed as well.

I'm not sure how we want to deprecate constants, so I just left them there. If we don't have any way in place to actually deprecate them, we should at LEAST add comments that say they're deprecated.

#5 @scribu
13 years ago

Yeah, it seems the practice is simply to add a @deprecated tag. Example: PLUGINDIR

#6 @aaroncampbell
13 years ago

18298.2.diff adds the @deprecated tag to the constants.

#7 @cais
13 years ago

Following along the lines of the @since documentation would there be a general benefit to having the version when the item is deprecated included as well?

For example, from this ticket's premise, @deprecated 3.3

#8 @chipbennett
13 years ago

  • Cc chip@… added

Shouldn't these constants have version/replacement function information in the @deprecated phpDoc tag? (I ask mainly so that the various development tools, such as the Log Deprecated Notices Plugin and the Theme Check Plugin can be configured to pick up this information, to pass along to developers.

#9 @aaroncampbell
13 years ago

18298.3.diff adds php doc like this:

 * @deprecated 3.3
 * @deprecated Use get_stylesheet_directory()

#10 follow-up: @scribu
13 years ago

@chip: Log Deprecated Notices plugin can't do anything about it, since there's no mechanism in PHP to find out when a particular constant is used. Same for global variables (which is why they're evil).

#11 in reply to: ↑ 10 @chipbennett
13 years ago

Replying to scribu:

@chip: Log Deprecated Notices plugin can't do anything about it, since there's no mechanism in PHP to find out when a particular constant is used. Same for global variables (which is why they're evil).

Why is that? Couldn't either Plugin read the phpDoc headers, and cross reference the @deprecated tag against the constant being used anywhere in the Theme files? (Isn't that what they do anyway, to provide the deprecation (since-version, alternative function)?

#12 follow-ups: @aaroncampbell
13 years ago

I think that what he's saying is that you'd have to build some sort of regex to look for the constant being used since there's no way to actually throw a deprecated notice since it's not a function. For example, how do you find:

$test = $prefix . TEMPLATEPATH . $suffix;

but not

$test = $prefix . 'TEMPLATEPATH' . $suffix;
$test = "$prefix TEMPLATEPATH $suffix";
$test = <<<TEST
    $prefix
    Bad: TEMPLATEPATH
    $suffix
TEST;

#13 in reply to: ↑ 12 @chipbennett
13 years ago

Replying to aaroncampbell:

I think that what he's saying is that you'd have to build some sort of regex to look for the constant being used since there's no way to actually throw a deprecated notice since it's not a function.

Pross is the WPTRT's resident RegEx ninja; if anybody can figure out a way to do it, he can. :) So, maybe it's something that can find its way into Theme Check. My interest in the question (which now exceeds the scope of the patch itself) is in providing as much information as possible to Theme Developers, and getting them pre-emptively to begin to adopt the use of get_template_directory_*() and get_stylesheet_directory_*().

#14 in reply to: ↑ 12 ; follow-up: @toscho
13 years ago

Replying to aaroncampbell:

I think that what he's saying is that you'd have to build some sort of regex to look for the constant being used

Impossible. PHP is more complex than PCRE, so you’d have to use a tokenizer. Just think about constants in comments … ;)

#15 in reply to: ↑ 14 @chipbennett
13 years ago

Replying to toscho:

Replying to aaroncampbell:

I think that what he's saying is that you'd have to build some sort of regex to look for the constant being used

Impossible. PHP is more complex than PCRE, so you’d have to use a tokenizer. Just think about constants in comments … ;)

Which would be just fine for Theme Check. It ignores anything in PHP comments. Also, even if it finds false-positives, that's okay. Most such output is listed as INFO anyway: meant as a hint to the developer/reviewer to verify that its usage is proper.

In any case, this rabbit trail has strayed far from the intent of the ticket. I'll take it offline with Pross, and if he comes up with something viable for Theme Check, then I will take a look at the rest of the defined constants with @deprecated, to see if any could benefit from additional deprecation information, and submit a separate patch.

#16 @nacin
13 years ago

  • Type changed from defect (bug) to enhancement

#17 @aaroncampbell
13 years ago

  • Description modified (diff)
  • Version set to 3.0

#18 @rmccue
12 years ago

  • Cc me@… added

#19 @aaroncampbell
12 years ago

  • Milestone changed from Awaiting Review to Future Release

#20 @mordauk
12 years ago

  • Cc pippin@… added

#21 @omarabid
12 years ago

  • Cc omarabid added

Any updates on this. This ticket has over a year, and yet the Constants are still used instead of the function.

The file is question is: wp-includes/template.php

The Constants are used in the locate_template() function. This makes it impossible to change the location of my template files. For a special App. I'm building, I need to change these locations.

This is possible, but only if the functions were used instead of the Constant.

Is there a reason to keep the Constants instead? I have changed my WordPress Core code (which is bad), shall I submit my modification.

#22 @wpsmith
12 years ago

  • Cc travis@… added

#23 @juliobox
11 years ago

  • Cc juliobosk@… added

#24 @digitalquery
11 years ago

  • Cc anu@… added

#25 @wycks
11 years ago

  • Cc bob.ellison@… added

#26 @CrazyJaco
10 years ago

Also checking in for an update on this.

I have a similar need as omarabid.

It looks like its been 2 years since there was any sort of activity on this. Is this still on anyone's radar?

Thanks

#27 @DrewAPicture
10 years ago

  • Focuses template added
  • Keywords needs-refresh added
  • Milestone changed from Future Release to 4.0

I think we could take another look at this. Moving to 4.0 for consideration.

@obenland
10 years ago

#28 @obenland
10 years ago

  • Keywords needs-refresh removed

#29 @wonderboymusic
10 years ago

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

In 28563:

Replaces all uses of TEMPLATEPATH and STYLESHEETPATH in core with get_template_directory() and get_stylesheet_directory().

Add @deprecated annotations to TEMPLATEPATH and STYLESHEETPATH definitions.

Props obenland, aaroncampbell.
Fixes #18298.

#30 @DrewAPicture
10 years ago

In 28569:

Use three-digit x.x.x style version for @deprecated phpDoc tags.

See #18298.

#31 @nacin
10 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

I've always thought this was a bit silly, but didn't care enough to complain (and, of course, I like making things more testable). However, I've noticed we've now greatly increased what gets executed with the inclusion of functions.php and every theme template file. In the case of including functions.php, we're running each function twice (via is_child_theme(), too). These functions are definitely not as cheap as a constant — they inclusively run three filters and a get_option() call (five if multiple theme roots are in play).

I have not yet run any kind of profiling on this.

Eliminating TEMPLATEPATH and STYLESHEETPATH was designed to make things more testable, I get that. But that doesn't apply to wp-settings.php, for example. This also exposes the ability to tinker with locate_template() by filtering the theme directory itself, which seems pretty scary. And, the changes in locate_template() mean these functions are called in a loop, rather than outside of it, especially problematic because it could be called up to four times per iteration even though the values might be the same (in the case of no child theme).

I just want to make sure we know what we're getting into, and also optimize for the fact that we're now dealing with dynamic code so we need to be more mindful of how we're writing our control structures.

#32 @helen
10 years ago

  • Keywords needs-patch added; has-patch removed

#33 follow-up: @nacin
10 years ago

  • Keywords revert added

Suggest we revert [28563] for now.

#34 in reply to: ↑ 33 @DrewAPicture
10 years ago

Replying to nacin:

Suggest we revert [28563] for now.

+1. Good points all in comment:31. "I just want to make sure we know what we're getting into ...".

#36 @nacin
10 years ago

  • Keywords revert removed
  • Milestone changed from 4.0 to Future Release

Happy to see this happen at some point, but it's not a priority and isn't a straight replacement due to the cost differences in these functions.

#37 @chriscct7
8 years ago

Re: comment:31 It'd be interesting to see the difference in real cost could be benchmarked for comparison

#38 @dd32
8 years ago

#37443 was marked as a duplicate.

#39 @hkchakladar
6 years ago

Are those constants deprecated? I can't see deprecated tag in WP-4.9.1

#40 follow-up: @sebastienserre
2 years ago

Hello,

I think this ticket should be consider again as logs are full of

PHP Warning:  Use of undefined constant TEMPLATEPATH - assumed 'TEMPLATEPATH' (this will throw an Error in a future version of PHP) in /var/www/XXX/prod/wp-includes/theme.php on line 158
Warning: Use of undefined constant TEMPLATEPATH - assumed 'TEMPLATEPATH' (this will throw an Error in a future version of PHP) in /var/www/XXX/prod/wp-includes/theme.php on line 158
PHP Warning:  Use of undefined constant STYLESHEETPATH - assumed 'STYLESHEETPATH' (this will throw an Error in a future version of PHP) in /var/www/XXX/prod/wp-includes/theme.php on line 158
Warning: Use of undefined constant STYLESHEETPATH - assumed 'STYLESHEETPATH' (this will throw an Error in a future version of PHP) in /var/www/XXX/prod/wp-includes/theme.php on line 158

We should maybe just check if constant are defined ?

#41 in reply to: ↑ 40 @SergeyBiryukov
2 years ago

Replying to sebastienserre:

We should maybe just check if constant are defined ?

They are defined via wp_templating_constants() early in wp-settings.php, so it seems like the only way to get these warnings is to call is_child_theme() too early: directly on file inclusion or on one of the following actions:

  • plugins_loaded
  • sanitize_comment_cookies
  • setup_theme

I suppose is_child_theme() could check if TEMPLATEPATH and STYLESHEETPATH are defined, and return false otherwise, but that might not be the expected result if a child theme is indeed in use, it's just that the check is performed too early. Perhaps a _doing_it_wrong() message would be a better option, similar to the one we have for conditional tags:

if ( ! isset( $wp_query ) ) {
	_doing_it_wrong( __FUNCTION__, __( 'Conditional query tags do not work before the query is run. Before then, they always return false.' ), '3.1.0' );
	return false;
}

#42 @swissspidy
10 months ago

  • Focuses performance added

#43 @joemcgill
10 months ago

#58866 was marked as a duplicate.

#44 @joemcgill
10 months ago

  • Milestone changed from Future Release to 6.4

This issue was discovered again recently as part of #58576. I'm going to move this into the 6.4 milestone for consideration.

#45 @flixos90
10 months ago

  • Keywords has-patch added; needs-patch removed
  • Owner changed from wonderboymusic to thekt12
  • Status changed from reopened to assigned

Given that @thekt12 has been working on a fresh PR for this in https://github.com/WordPress/wordpress-develop/pull/4880, I'll assign him for now.

It's great to see the additional discussion history here, and it seems the main question here is whether there are any notable adverse performance effects from this change (since surely accessing constants is faster than calling these functions).

Therefore, in order to make a decision, we need to carefully benchmark and assess the performance implications and whether they are negligible or not

This ticket was mentioned in PR #4880 on WordPress/wordpress-develop by @thekt12.


10 months ago
#46

  • Keywords has-unit-tests added

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

This bug was found while working on performance ticket https://core.trac.wordpress.org/ticket/58576
This PR replaces usage of STYLESHEETPATH and TEMPLATEPATH with get_template_directory() and get_stylesheet_directory()

@flixos90 commented on PR #4880:


10 months ago
#47

@kt-12 I've updated the PR description to reference the ticket https://core.trac.wordpress.org/ticket/18298, given we discovered that the new one is a duplicate.

#48 @oglekler
9 months ago

  • Keywords needs-testing added

This ticket was mentioned in Slack in #core-performance by joemcgill. View the logs.


9 months ago

This ticket was mentioned in Slack in #core-performance by thekt12. View the logs.


9 months ago

@thekt12 commented on PR #4880:


9 months ago
#51

@mukeshpanchal27 @felixarntz
Conducted benchmarking of CONST(trunk), FUNCTION(this pr) and GLOBAL(PR #5156).

cmd used to beanchmark npm run research -- benchmark-server-timing -u http://localhost:8889 -n 500

| CONST (trunk) | GLOBAL | FUNCTION |   | BEST | WORST

-- | -- | -- | -- | -- | -- | --
Twenty Twenty-Three |   |   |   |   |   |  
Response Time (median) | 147.69 | 151.99 | 149.52 |   | CONST | GLOBAL
wp-before-template (median) | 55.85 | 57.47 | 60.34 |   | CONST | FUNCTION
wp-template (median) | 88.9 | 91.72 | 86.15 |   | FUNCTION | GLOBAL
wp-total (median) | 145.05 | 149.38 | 146.99 |   | CONST | GLOBAL

|   |   |   |   |   |  

Twenty Twenty-One |   |   |   |   |   |  
Response Time (median) | 143.74 | 142.72 | 143.19 |   | GLOBAL | CONST
wp-before-template (median) | 39.3 | 39.51 | 39.52 |   | CONST | FUNCTION
wp-template (median) | 101.46 | 100.5 | 100.82 |   | GLOBAL | CONST
wp-total (median) | 140.95 | 140.02 | 140.39 |   | GLOBAL | CONST

For block theme constants had a better performance over other methods. wp-template (median) performance was better for function. global was the worst performing candidate. If we need performance then we can keep the constants intact, however if we need flexibility then the next best option asper data is function. Possible reason for global preforming low, might be due to the lookup time in finding variable in hash table ( I couldn't find anything relevant to support his claim though)

For classic theme, the values were near about similar for all 3 scenario with performance of global slightly better than others.

This ticket was mentioned in Slack in #core-performance by thekt12. View the logs.


9 months ago

This ticket was mentioned in PR #5160 on WordPress/wordpress-develop by @flixos90.


9 months ago
#53

This PR is another attempt at replacing use of the STYLESHEETPATH and TEMPLATEPATH constants to aid testability with a hopefully minimal (as in negligible) performance impact.

It is a combination of the ideas explored in #4880 and #5156.

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

@flixos90 commented on PR #5160:


9 months ago
#54

Running benchmarks of this doesn't show a notable difference either way:

  • I used npm run research -- benchmark-server-timing -u http://localhost:8889 -n 100.

### Twenty Twenty-Three results

  1. 93.31ms with PR vs 96.21ms without PR
  2. 98.97ms with PR vs 97.16ms without PR

### Twenty Twenty-One results

  1. 57.15ms with PR vs 58.25ms without PR
  2. 57.21ms with PR vs 56.81ms without PR

Based on this data, it seems that the difference in performance is tiny enough to not care.

#55 @flixos90
9 months ago

After reviewing the two PRs https://github.com/WordPress/wordpress-develop/pull/4880 and https://github.com/WordPress/wordpress-develop/pull/5156 and their benchmarks (thanks @thekt12!), I implemented another alternative which is a combination of the two approaches in https://github.com/WordPress/wordpress-develop/pull/5160:

  • It uses the get_*_directory() functions instead of the constants.
  • It uses globals to "cache" the results of the filter in memory so that the respective filter is only run once per page load.
  • The globals are reset whenever the active theme is switched, leading to a recalculation of the result, which addresses the main limitation of the constant approach.

I conducted a benchmark for this (see results above), and the differences are minimal, with no clear indication in either direction, which makes me think the difference in performance is tiny enough to be irrelevant. FWIW that may be the case for the other approaches too if we conduct more benchmarks for the PRs, at least that's what I'd suspect based on my benchmarks.

I'd like to propose https://github.com/WordPress/wordpress-develop/pull/5160 for consideration. If we go with this approach, we could probably port over the tests from https://github.com/WordPress/wordpress-develop/pull/4880 for additional coverage.

@flixos90 commented on PR #5160:


9 months ago
#56

@kt-12 Would be great to get your thoughts on this as well.

@flixos90 commented on PR #5160:


9 months ago
#57

@kt-12 I have migrated some of your tests from #4880 over to this PR and added a few more tests. I think the issues in your tests with the theme root were a specific issue with the way the test setup was handled; most importantly, using filters to override the theme root should not be necessary here, or rather would not be related to the change that is being tested here.

I've also updated the existing (very old) test_switch_theme() test specifically to "un-comment" the commented out code for the get_query_template() logic which used to be not testable but now is - so that's a neat way to ensure the change here works. Obviously that test is very messy and huge, but rather than bothering to clean it up, I think it can serve as a good demonstration that what was intended years ago now works that way finally.

Of course I've also added a few more targeted tests, to make sure we also have coverage that reflects our current testing standards :)

@mukesh27 commented on PR #4880:


8 months ago
#58

Thanks @kt-12 for sharing the benchmarking report.

@mukesh27 commented on PR #5160:


8 months ago
#59

The https://3v4l.org/D2WrI, demonstrates that the performance impact is negligible and not a significant concern.

@flixos90 commented on PR #5160:


8 months ago
#61

@mukeshpanchal27 @spacedmonkey I have addressed all your feedback. Please take another look.

@spacedmonkey Thank you for the benchmark! I think the 3% improvement confirms that there is no negligible performance impact. I don't think the 3% are an _actual_ performance improvement, I assume they just come from variance, similarly to how in my own benchmarks sometimes the PR was faster, sometimes slower, but always just a tiny bit.

Since we're replacing globals with function calls, I don't think in reality there is any chance that the code here is faster than the one before. The point is mostly to ensure that it doesn't become slower to a notable degree - which I think we have proven with the different benchmarks we've been running against this. 👍

#62 @flixos90
8 months ago

  • Keywords needs-dev-note added; needs-testing removed

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


8 months ago

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


8 months ago

#66 @spacedmonkey
8 months ago

  • Keywords commit added
  • Owner changed from thekt12 to flixos90

Marked as ready to commit and assigned to @flixos90 to commit!

@flixos90 commented on PR #4880:


8 months ago
#67

Closing in favor of #5160.

#68 @flixos90
8 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 56635:

Themes: Deprecate usage of TEMPLATEPATH and STYLESHEETPATH constants.

While generally the functions get_template_directory() and get_stylesheet_directory() were long recommended to use to get the parent or child theme directory, the TEMPLATEPATH and STYLESHEETPATH constants were still used in a few places in core, most importantly in template related logic.

The remaining usage was problematic as it prevented testability of certain key components of WordPress core.

This changeset replaces all remaining usage with the corresponding functions and effectively marks these constants as deprecated. It also adds test coverage accordingly and even unlocks some existing, previously commented out test coverage to work as expected.

Performance of the new approach has been benchmarked and shows no notable differences. Yet, given that the current theme directories are not expected to change within a regular WordPress page load, the get_template_directory() and get_stylesheet_directory() functions were amended with in-memory caching of the result, unless one of the defining values is being filtered.

Props thekt12, spacedmonkey, mukesh27, aaroncampbell, scribu, lloydbudd, cais, chipbennett, toscho, omarabid, CrazyJaco, DrewAPicture, obenland, wonderboymusic, nacin, helen, dd32, chriscct7, SergeyBiryukov, swissspidy, joemcgill, flixos90.
Fixes #18298.

#70 @iandunn
8 months ago

I think this may have caused an issue with load-styles.php. I put reproduction steps in #59417.

#71 @flixos90
8 months ago

In 56641:

Themes: Fix fatal error in load-styles.php.

Following [56635], a fatal error occurred in load-styles.php leading to admin styles not working, because of a has_filter() call being added to get_stylesheet_director() and get_template_directory().

This changeset adds has_filter() to wp-admin/includes/noop.php to prevent such errors. The lack of loading the function does not cause any unintended side effects itself.

Props iandunn, adamsilverstein.
Fixes #59417.
See #18298.

#72 @desrosj
8 months ago

In 56679:

Docs: Correct typo in new @since tag.

Follow up to [56635], [56641].

See #18298.

#73 @desrosj
8 months ago

In 56686:

Docs: Revert [56679].

Memoizes is actually correct in this context.

Unprops desrosj.
See #18298.

#74 follow-up: @danielbachhuber
8 months ago

I think this broke WP-CLI's --skip-themes global argument.

Here's a failing test: https://github.com/wp-cli/wp-cli/actions/runs/6333462262/job/17201622545#step:13:241

I can't reproduce it in my wordpress-develop checkout for some reason, but this comment leads me to believe we were leaning on TEMPLATEPATH and STYLESHEETPATH: https://github.com/wp-cli/wp-cli/blob/8114cb4ef3a901c19f23a65cca7dca59c7335f1c/php/WP_CLI/Runner.php#L1808

Here's the original implementation: https://github.com/wp-cli/wp-cli/pull/2944

#75 in reply to: ↑ 74 @danielbachhuber
8 months ago

Replying to danielbachhuber:

I think this broke WP-CLI's --skip-themes global argument.

This turned out to be an easy fix, fortunately: https://github.com/wp-cli/wp-cli/pull/5840

This ticket was mentioned in Slack in #core-performance by spacedmonkey. View the logs.


7 months ago

#77 @codente
7 months ago

I'm reviewing tickets that may require dev notes with @webcommsat and we do not see a draft dev note for this. We checked the 3 Performance drafts and did not see it included.

Documentation tracker issue for this trac ticket:
https://github.com/WordPress/Documentation-Issue-Tracker/issues/1184

We have noted that performance have clustered a number of tickets together for the dev notes.
We will cross-reference the issues to reflect this.

This ticket was mentioned in Slack in #core-performance by clarkeemily. View the logs.


7 months ago

#79 follow-up: @flixos90
7 months ago

  • Keywords add-to-field-guide added; needs-dev-note removed

I think this should be added to the field guide as it is not significant enough to warrant a dev note and can be quickly explained. Maybe something like:

The TEMPLATEPATH and STYLESHEETPATH constants have been deprecated. get_template_directory() and get_stylesheet_directory() should be used instead.

#80 in reply to: ↑ 79 @webcommsat
7 months ago

For completeness, this has been added to the Field Guide. Thank you @flixos90 and everyone who has worked on this ticket.

I presume has-patch and has-unit-tests tags can now be removed from this ticket too for housekeeping.

Replying to flixos90:

I think this should be added to the field guide as it is not significant enough to warrant a dev note and can be quickly explained. Maybe something like:

The TEMPLATEPATH and STYLESHEETPATH constants have been deprecated. get_template_directory() and get_stylesheet_directory() should be used instead.

#81 @flixos90
7 months ago

In 56974:

Multisite: Ensure that switching sites resets the current theme directory globals.

The globals introduced in [56635] to cache the current theme directories in memory were not considering switching sites in a multisite network. This changeset addresses the bug including test coverage.

Props codex-m, jeremyfelt.
Fixes #59677.
See #18298.

#82 @flixos90
7 months ago

In 56986:

Multisite: Ensure that switching sites resets the current theme directory globals.

The globals introduced in [56635] to cache the current theme directories in memory were not considering switching sites in a multisite network. This changeset addresses the bug including test coverage.

Props codex-m, jeremyfelt, costdev, joemcgill.
Merges [56974] to the 6.4 branch.
Fixes #59677.
See #18298.

#83 @flixos90
7 months ago

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.

#84 @flixos90
7 months ago

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.

This ticket was mentioned in PR #5634 on WordPress/wordpress-develop by @mukesh27.


6 months ago
#85

Trac ticket:

Followup for #18298 / [56635]

While i working on other ticket i found this issue.

When i run unit test for test_switch_theme using npm run test:php -- --filter test_switch_theme i found that it show me below error. They error comes from the TT4 theme, the tests did not run checks for block theme per this line of code but when we run single unit test current_theme_supports( 'block-templates' ) return false due to some global updates in other unit tests.

This PR add the missing theme-compat check similar to locate_template function so it will not failed.

@mukesh27 commented on PR #5634:


6 months ago
#86

Thanks @felixarntz, I have open separate PR 5646 in which we can check the switch_theme behaviour.

@flixos90 commented on PR #5634:


6 months ago
#87

@mukeshpanchal27 I've looked into the problem as well and implemented #5650 as an attempt. Can you try applying that to your site (without this PR here though) and see if that would address the failure with TT4 as well?

This ticket was mentioned in Slack in #core-performance by joemcgill. View the logs.


5 months ago

This ticket was mentioned in Slack in #core-performance by joemcgill. View the logs.


4 months ago

This ticket was mentioned in Slack in #core-performance by joemcgill. View the logs.


4 months ago

This ticket was mentioned in Slack in #core-performance by joemcgill. View the logs.


3 months ago

Note: See TracTickets for help on using tickets.