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

Return empty array when not using themes #4409

Conversation

danielbachhuber
Copy link
Member

Returns an empty array from wp_get_active_and_valid_themes() when ! wp_using_themes(). The empty array prevents wp-content/themes/functions.php from being loaded erroneously.

See https://core.trac.wordpress.org/ticket/57928

@danielbachhuber
Copy link
Member Author

Seems like a hacky test failure...

@@ -948,6 +948,10 @@ function wp_get_active_and_valid_themes() {
return $themes;
}

if ( ! wp_using_themes() ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danielbachhuber Would STYLESHEETPATH be empty in this context?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@spacedmonkey Unfortunately not. It's set to an erroneous value:

diff --git a/src/wp-includes/load.php b/src/wp-includes/load.php
index 9780519856..bc0a35ab0e 100644
--- a/src/wp-includes/load.php
+++ b/src/wp-includes/load.php
@@ -948,6 +948,9 @@ function wp_get_active_and_valid_themes() {
 		return $themes;
 	}
 
+	var_dump( 'TEMPLATEPATH: ' . TEMPLATEPATH );
+	var_dump( 'STYLESHEETPATH: ' . STYLESHEETPATH );
+
 	if ( ! wp_using_themes() ) {
 		return $themes;
 	}
$ wp --skip-themes option get home
string(82) "TEMPLATEPATH: /Users/danielbachhuber/projects/wordpress-develop/wp-content/themes/"
string(84) "STYLESHEETPATH: /Users/danielbachhuber/projects/wordpress-develop/wp-content/themes/"
https://wordpress-develop.test

Notice the value is /wordpress-develop/wp-content/themes/ without a theme slug.

@spacedmonkey
Copy link
Member

Some unit tests for this would be useful to understand what this change means.

@danielbachhuber danielbachhuber force-pushed the 57928-return-empty-not-using-themes branch from 1553607 to 11384bf Compare June 7, 2023 18:50
@danielbachhuber
Copy link
Member Author

Some unit tests for this would be useful to understand what this change means.

Added a test case in 35091f8

wp_get_active_and_valid_themes() is only used here:

// Load the functions for the active theme, for both parent and child theme if applicable.
foreach ( wp_get_active_and_valid_themes() as $theme ) {
if ( file_exists( $theme . '/functions.php' ) ) {
include $theme . '/functions.php';
}
}
unset( $theme );

Interestingly, the random test failures in my first commit were caused by WP_USE_THEMES being undefined. Apparently that hasn't ever bit anyone before.

WP_USE_THEMES is defined in src/index.php and originates from https://core.trac.wordpress.org/changeset/2303. The test suite doesn't load src/index.php, so the constant wasn't ever defined.

@danielbachhuber
Copy link
Member Author

@danielbachhuber danielbachhuber deleted the 57928-return-empty-not-using-themes branch June 7, 2023 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants