Add a filter for changing where parent templates are located

[apeatling]

I propose adding a filter to get_themes() that will allow plugins to redefine where to look for parent themes, based on the child themes' $template value.

Here is a scenario where this would be useful:

BuddyPress includes a default theme bundled inside of its plugin directory. Currently users have to move this theme to the themes directory. They have to do this on every single update.

If the default theme was a parent theme sitting inside of the BuddyPress plugin directory, it will be updated at the same time as the plugin is updated. This way any child themes would automatically see the updates without the need for moving folders or editing themes.

The change needed for this is very simple. Please see attached patch.

[apeatling]

Parent theme root patch.

[dd32]

The other option is for plugins to register extra themes somehow, And if the template is one that a plugin has registered, then load it from the plugin DIR instead.

..It could also be a benefit for plugins to register Page templates that way (or similar)

Just throwing another possible thought out there, As i'll run into the same thing in a few months.

[apeatling]

@dd32: That is another option, but it sounds as though it would probably be hacky. Doing it this way at least keeps within the standard way of activating and loading themes.

I've updated the patch as the $theme_loc needs to be filtered too so that the "Available Themes" screen accurately reflects the location of the parent template. The variables also need to be unfiltered at the beginning of each iteration, as to not filter the template location of other themes.

[dd32]

@dd32: That is another option, but it sounds as though it would probably be hacky. Doing it this way at least keeps within the standard way of activating and loading themes.

I guess your method would work alright, Just as long as its possible to filter both the source dir for the theme, and be able to inject it into the themes array early enough not to set off any alarm bells over incomplete themes.

The way i see it, My method is mearly a wrapper around the filters you're describing basically, much like the plugin activate hooks. But it would mean the Core handles most of the work and its done right everytime, not half-baked by a dodgy plugin or 2. (Of course, its not worth protecting against 1 or 2 dodgies, its a fact of life)

[apeatling]

But it would mean the Core handles most of the work and its done right everytime, not half-baked by a dodgy plugin or 2. (Of course, its not worth protecting against 1 or 2 dodgies, its a fact of life)

You do have to protect against this. In the second patch I've added a reset to the themes loop that will undo the filtered values. That way it's only possible to filter the values for a specific theme and not mess up any others.

[westi]

I think it would be better to make the functionality explicit.

So make WordPress understand the concept of a number of directories containing themes.

Give plugins the ability to register_theme_folder() where one or more themes lives

[josephscott]

+1 to westi's suggestion of just making WP aware of multiple paths that contain themes. With the appropriate filters and hooks so that plugins and themes can tweak them as needed.

[apeatling]

That sounds like a better idea for sure.

[ptahdunbar]

based on the conversation in the dev meetup:

westi: So make WordPress understand the concept of a number of directories containing themes.
Give plugins the ability to register_theme_folder() where one or more themes lives

Will this also be considered for plugins? A primary use case would be themes bundling canonical plugins with the theme.

[strider72]

+1 on register_theme_folder() and +! on register_plugin_folder() Great idea! :) :) :)

[westi]

I think we should do themes for now.. and consider plugins later - maybe open a separate ticket for that :-)

[markjaquith]

This is a really important patch, but it needs to get finished ASAP in order to make it in for 2.9 — Andy, can you finish this up?

[apeatling]

I'll concentrate on this tomorrow and post what I have.

[apeatling]

I've attached a new patch that allows registration of a new a theme directory. WordPress will then consider this as an additional place to look for themes.

register_theme_directory( 'plugins/my-plugin/bundled-themes' );

All directories must be relative to the content directory.

This was considerably more in-depth than I expected. The hardest part was getting the get_template_directory[_uri]() and get_stylesheet_directory[_uri]() functions to look in varying theme roots depending on the theme in use. Currently get_theme_root() simply returns a filterable "wp-content/themes" value with no theme parameters to allow us to determine where to look.

The solution was to make sure that the $wp_themes global was available and loop through that matching the template or stylesheet name, then use the new theme root/theme root uri value. I'm not completely happy with this method though, it seems quite inefficient. I'm sure there is a better way, but it's a start.

[markjaquith]

Updated patch to be relative to WP dir and to remove unnecessary $wp_themes calls. get_themes() returns $wp_themes if it exists, so there's no need to replicate that logic in all the other functions. We can just call get_themes() and it'll give us the cached global variable or it'll dynamically populate it.

Still looking over the patch.

[markjaquith]

Works so far. Created wp-content/my-themes dir. Copied default theme there, renamed dir to awesome-theme and changed name of theme. Created child theme in regular themes dir, using Template: awesome-theme. Everything appeared to work normally.

[westi]

From a quick review the current issue we need to resolve is calling get_themes() when displaying on the front end.

That is going to scan the theme folder for every page load which is not a scalable solution.

I guess we need to cache more info in the database so the front end stuff can be all options based.

We should only be calling get_themes() when we want to display the list of themes.

[ryan]

Remember the last time we accidentally called get_themes() for a front page load? Not good indeed.

[apeatling]

I'll take another look at this in the morning. There has to be a better way of resolving the theme root without having to fetch all the themes first.

[apeatling]

I've attached another patch that will store an option with all the theme roots stored. The option will update when get_themes() is run. Instead of fetching themes on the front end and looping to find the theme root, the option is fetched. If a match for the theme is found, the theme_root or theme_root_uri is filtered with the theme root value.

[westi]

Looking over this now.

Comments:

I am trying to remove all create_function calls in WordPress which are written such that the function body is based on variables as it has been shown that this can be used to perform code evaluation so I would rather not add some more of these.

I guess we may need more code in get_theme_root and get_theme_root_uri instead.

[apeatling]

Good to know, can change.

[apeatling]

Here's a fourth patch that modifies get_theme_root() and get_theme_root_uri() to allow us to pass an optional template/stylesheet parameter. This avoids the create_function() based filters.

[markjaquith]

Should we be using the transient system to store this? We could have a front-end wrapper that grabs it from transient if available, or grabs it live and populates the transient if not.

[ryan]

Probably a transient or at least a site option. I don't really want to store this for every blog on wp.com since it will always be the same.

[markjaquith]

On for MU we can use a pre_option_ hook to grab it from sitemeta instead, and short circuit updates.

[markjaquith]

-005.diff switches to using the transient system with a cache of two hours (there is some chance that someone will shuffle themes around, not hit the admin, and change the output of get_themes()). Fetches go through get_theme_roots() which will check the transient, and if === false, run get_themes() to set it.

[ryan]

Looks good to me.

[markjaquith]

(In [12025]) Introducing register_theme_directory() which takes a wp-content-relative path and will additionally scan it for themes. Plugins can use this to add themes without requiring copying by the user. props apeatling. fixes #10467

[dd32]

Slight issue with this: #10953

Some other thoughts: This would be better accepting an absolute directory..

My reasoning is that plugins are not always relative to the WP_CONTENT_DIR, which really limits its usage for that.. It means that for 100% compatibility, Plugins are going to have to work out a relative link dynamically.. You cant just go hard-coding 'plugins/my-plugin-name/my-theme'.. It changes.. Passing however, dirname(FILE).'/my-theme' would work if it was a absolute path required.

I'm reopening this for feedback on the latter issue..

[scribu]

Possibly related: #10959

[westi]

(In [12043]) Restore the functionality of the theme_root filter. See #10467.

[scribu]

Possibly related: #10974

[ryan]

This breaks subdir support. Before, you could put themes in directories one level below the themes directory. Example: wp-content/themes/pub/, wp-content/themes/priv/

[ryan]

(In [12117]) Partial fix for theme subdir support. see #10467

[ryan]

[12118]

[automattor]

(In [12119]) Theme subdirs that contain themes are not broken if missing a stylesheet. see #10467

[ryan]

(In [12129]) get_themes() and theme root fixes. see #10467

[ryan]

Theme roots should always be within WP_CONTENT_DIR, yes? If so, we can trim WP_CONTENT_DIR from the template file paths because repeating it over and over is making the themes array much bigger.

[westi]

Default theme root has to be inside WP_CONTENT_DIR but plugins don't have to be and it's plugins that are most likely to want to register an extra theme root - e.g. BuddyPress

[ryan]

Don't the BuddyPress roots live in the plugins dir inside the content dir?

[westi]

Replying to ryan:

Don't the BuddyPress roots live in the plugins dir inside the content dir?

AFAIK yes.

[ryan]

I guess WP_PLUGIN_DIR can be defined to be outside of WP_CONTENT_DIR. I don't know if anyone actually does that.

[westi]

I think with [12226] we have reduced the data size of get_themes() enough for now even though we have the full paths as it works out smaller than it was with 2.8.6

We can probably improve on this but it is not going to be a small change.

Closing this as Fixed for 2.9 and opening a new ticket (#11214) for 3.0 to see if we can improve it even more and remove the full paths.