Make WordPress Core

Opened 14 years ago

Closed 13 years ago

#17597 closed defect (bug) (fixed)

register theme directory() not working for themes outside WP_CONTENT_DIR

Reported by: tliebig's profile tliebig Owned by: nacin's profile nacin
Milestone: 3.4 Priority: normal
Severity: normal Version: 3.1.3
Component: Themes Keywords:
Focuses: Cc:

Description

I'm trying to add an additional theme directory that lives outside WP_CONTENT_DIR.

To do so, I've added
global $wp_theme_directories; $wp_theme_directories[] = $_SERVER['DOCUMENT_ROOT'].'/wp-content/themes';
to my wp-config.php.

The problem: Although WordPress correctly finds the themes within my custom theme folder, it reports them to be broken as soon as I activate them. From my investigations, this seems to be an issue with get_theme_root() putting the WP_CONTENT_DIR before the path although it should not do so when I give it the full file system path for my own theme directory (http://codex.wordpress.org/Function_Reference/register_theme_directory).

Completely moving /wp-content/ via WP_CONTENT_DIR constant is not an option since that would require me to store the default theme and the language files outside the /wordpress structure, which I'm trying to avoid. My goal ist to basically trying to remodel the strict separation between application/user data that the mac file system does: store application under /apps, and user data (including custom settings) under /home. This approach would allow to update WordPress as easily as just replacing the /wordpress-folder completely with the unzipped files from a freshly downloaded wordpress package without having to worry about /wp-content.

My final desired structure would be as such:

/
/wordpress              (the virgin WordPress system directory as downloaded)
/wp-config.php          (my custom wp-config.php holding my settings)
/wp-content/plugins     (folder holding the plugins that I've installed)[1]
/wp-content/themes      (folder holding the themes that I've installed)[2]
/uploads                (folder holding the user's uploads)

[1] would be great if it could be merged with the system plugins from /wordpress/wp-content/plugins, but for now it's okay if I just define the WP_PLUGIN_URL constant

[2] Expected to merge with /wordpress/wp-content/themes, but not working: themes within this folder are shown in the backend, but reported to be broken when I try to activate them. This is where register_theme_directory() does not work as advertised.

Attachments (2)

get_raw_theme_root.diff (3.1 KB) - added by nacin 13 years ago.
17597.diff (3.2 KB) - added by nacin 13 years ago.

Download all attachments as: .zip

Change History (15)

#1 @duck_
14 years ago

https://irclogs.wordpress.org/chanlog.php?channel=wordpress-dev&day=2011-01-27&sort=asc#m242222

The documentation needs to be clearer. register_theme_directory should only take a "wp-content-relative path", see [12025] and the IRC chat log linked above.

#2 @ocean90
13 years ago

Related: [12043]

It's not register_theme_directory more get_theme_root which provides an issue here.

#3 @ocean90
13 years ago

  • Cc nacin added
  • Milestone changed from Awaiting Review to 3.4

With the improvements in #20103 we should try to address this issue too.

#4 @nacin
13 years ago

register_theme_directory() is only supposed to work within WP_CONTENT_DIR. Supporting it outside of WP_CONTENT_DIR could happen I suppose, but definitely not before #20103 settles. Unfortunately the theme root API is a bit kludgy and only lightly used. (#20103 includes a fix for a rather annoying bug, for example, to get_raw_theme_root().)

#5 @ocean90
13 years ago

register_theme_directory() is only supposed to work within WP_CONTENT_DIR.

I'm fine with that, but then we should make it clear and remove the bits from [12043].

#6 @nacin
13 years ago

Sorry, I wasn't aware of the history there. Yes, WP_PLUGIN_DIR can be outside of WP_CONTENT_DIR. It probably shouldn't be, but it can be. I will run some tests after #20103 drops.

#7 @nacin
13 years ago

get_raw_theme_root.diff should fix this.

This changes the return value of get_raw_theme_root() in some cases., for it to always be absolute. I am not concerned. One, there are no plugins using get_raw_theme_root() in the directory. Two, it is such an edge function, that if you were using it, you were surely playing with multiple theme roots, and therefore would have noticed the discrepancy in the return value such that you worked around it, perhaps by looking for '/themes' and prepending WP_CONTENT_DIR.

This also fixes the return value of get_theme_roots() by making it absolute when only one directory is registered. I am not concerned for the same reason as above. It also now returns in this case get_theme_root(), rather than WP_CONTENT_DIR . /themes, because get_theme_root() is a filtered version of that, and that is what we pass to register_theme_directory() to register the initial theme root. However, WP_CONTENT_DIR . '/themes' is still used in get_raw_theme_root() as that is designed to be raw and unfiltered.

The code for grabbing a theme root URI is still lame (it strips out WP_CONTENT_DIR then passes it to content_url()), but this is no less lame than it is in get_themes() or even WP_Theme.

#8 @nacin
13 years ago

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

In [20001]:

Always return an absolute path in get_raw_theme_root() and get_theme_roots().

These functions were changed in [15641] to avoid any calculations when only one theme directory is registered. However, the short-circuit ended up being relative to WP_CONTENT_DIR, rather than absolute. This broke situations where theme roots are outside the content directory (technically allowed), and made return values inconsistent, as when multiple roots were registered, absolute paths were always returned.

fixes #17597. see #20103. see #14911.

#9 @nacin
13 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

I made this worse with [20001].

Due to work in #20103 and due to my familiarity with the return value of search_theme_directories(), I missed that the theme_roots transient was actually relative to WP_CONTENT_DIR. Therefore it is not only possible for one to have a relative path in their template_root and stylesheet_root (which I did not think was possible), but it is the only situation.

Paths relative to WP_CONTENT_DIR are going to be much nicer to work with, and they are portable. But we still need absolute paths somewhere.

I am trying to come up with a solution here, but I am not sure what it will be yet. One option is that we try to store a relative path (strip out WP_CONTENT_DIR if we find it), and then when we pull that data, if the path is not in $wp_theme_directories, but WP_CONTENT_DIR + path is, it's pretty easy to assume it is relative.

This keeps the database relatively clear, the theme_roots transient free of repetitive absolute paths (much less data being stored), and doesn't hurt #20103 too much.

#10 @nacin
13 years ago

In [20015]:

Revert [20001]. Theme roots that are passed around, stored, and cached can be valid as absolute paths or paths relative to wp-content. We'll have to patch this elsewhere. see #17597.

@nacin
13 years ago

#11 @nacin
13 years ago

17597.diff

  • get_theme_root() considers a theme root returned from get_raw_theme_root() to be an absolute path if it is a registered theme directory. This provides a big of wiggle room for a situation where the current theme is ABSPATH but not WP_CONTENT_DIR, or in WP_PLUGIN_DIR in a situation where that is not in WP_CONTENT_DIR.
  • get_theme_root_uri() makes the same considerations as get_theme_root(), taking it a step further and attempts to find the right URL when we're not in WP_CONTENT_DIR. It is occasionally even successful.
  • Cleans up the branching in register_theme_directory().

#12 @nacin
13 years ago

In [20016]:

Fix the return value of get_theme_root() when the theme root is outside of WP_CONTENT_DIR, thus making it absolute rather than the typical relative theme root.

Make get_theme_root_uri() tolerate an absolute path for a theme root. It will now make an attempt to find a corresponding URL for absolute paths as well.

see #17597.

#13 @nacin
13 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed
Note: See TracTickets for help on using tickets.