get_theme_data can cause invalid WP_Theme object data to be persistantly cached

[westi]

The new version of get_theme_data in trunk is not aware of the concept of themes in subdirectories and makes the naive assumption that the style.css file it is passed to parse is for a theme in the root of a registered theme root rather than in a subdirectory.

The function itself works fine and returns correct data (Tests in ​[UT591]) but when a persistent caching back end is enabled the WP_Theme object caches data with incorrect stylesheet and template values for the theme.

For example if we have a theme in wp-content/themes/awesome/amazing-theme/ then both stylesheet and template should point to awesome/amazing-theme but if you call get_theme_data with the full path to the style.css file in wp-content/themes/awesome/amazing-theme/ the WP_Theme object gets created for the theme slug amazing-theme with the theme root wp-content/themes/awesome//.

Because the cache keys are based on md5sum of the path to the theme folder this data is then cached for the awesome/amazing-theme theme in the wp-content/themes/ theme root.

Attached patch fixes this for me in my testing (using error_logs inside get_theme_data to examine the WP_Theme object.)

I tried writing unit tests to show the badly cached data but it wasn't easy to do :(

[westi]

Fix to the logic in get_theme_data to correctly determine the theme slug and root to create a correct WP_Theme object.

[nacin]

Yeah, I definitely see what went wrong here. Nice catch.

new WP_Theme( 'twentyten', WP_CONTENT_DIR . '/themes/pub' ) should not work... It should recognize that the theme root is not a valid one and that the stylesheet should instead be twentyten/pub. Either it should error out by saying the theme does not exist, or it should correct the theme root and stylesheet on the fly to reflect the proper placement of /pub.

I will take a closer look at the patch -- not sure it will always work due to the finickiness of get_theme_root().

Yeah, testing the cached data can be tough. cache_delete() is a public method, so that could be used to test before/after situations.

[nacin]

In [20316]:

Correct the values for theme_root and stylesheet when the values passed to the WP_Theme constructor have a directory appended to the theme root when it should actually be prepended to the stylesheet (when the theme is in a directory of themes inside a theme root). see #20313.

[nacin]

get_theme_data-subdirs-fix.diff​ uses get_theme_root(). For reference, it was a design decision for the WP_Theme API to use raw, unfiltered values as much as possible. (The only situation where it does not is theme root URIs.) Hence, get_raw_theme_root() would be a better choice here.

I looked at the problem and think [20316] is the best solution. I do remember testing this exact situation but I neglected to account for it in the end. It is something that WP_Theme should handle, given that after immersing myself in this for the entire month of March, I still made that mistake when deprecating get_theme_data().

So, if WP_Theme is going to handle it, I think it is easiest if get_theme_data() just relies on the behavior.

[westi]

I think your solution should work fine.

I added some tests in ​[UT598] for the bug I saw and it fixes it perfectly.