Make WordPress Core

Opened 13 years ago

Closed 13 years ago

Last modified 11 years ago

#20103 closed defect (bug) (fixed)

Rewrite get_themes() and other enemies of sanity

Reported by: nacin's profile nacin Owned by:
Milestone: 3.4 Priority: normal
Severity: normal Version: 1.5
Component: Themes Keywords: has-patch
Focuses: Cc:

Description

get_themes() is a memory hog (#11214). The data it returns, and how it calculates it, is terribly designed (#15858, #12275, others) and inflexible (#19816, others). It is not possible to translate this data. Its aspects are poorly named (#11215). get_themes() keys things by theme name, rather than slug, which the theme editor and other areas rely on. Indeed, get_current_theme() and the corresponding option in the DB stores the name.

The functions that scan (get_themes(), search_theme_directories()) are stuck in a PHP4 world. They use globals and arrays, and one took your lunch money. They implement unnecessary loops and directory scans, making for very lame performance.

With get_themes(), everything is loaded at once, as there is no opportunity for lazy-loading, even though lists of files are only needed for the theme editor and often all we want is the current theme's information.

Almost none of this is even remotely cacheable, certainly not sanely.

Attempts to work with this API usually lead to disaster. The theme editor is a textbook example. It breaks on basically anything other than A-Z (again relying on names rather than slugs), and does some really evil things. I cleaned up what I could two years ago during my GSoC project, but because get_themes() makes most of MU look good, little of it could flow back into core.

Over the next few days, I will be attaching some patches that rewrite large swaths of this aging area of core.

Attachments (7)

20103.diff (4.1 KB) - added by nacin 13 years ago.
search_theme_directories.diff (4.3 KB) - added by nacin 13 years ago.
20103.2.diff (4.2 KB) - added by nacin 13 years ago.
Fixes a bad conditional.
wp_theme.diff (98.7 KB) - added by nacin 13 years ago.
20103.3.diff (99.5 KB) - added by nacin 13 years ago.
20103.4.diff (2.9 KB) - added by nacin 13 years ago.
wp_get_themes-cleanup.diff (1.1 KB) - added by duck_ 13 years ago.
See [20152] and [20029]

Download all attachments as: .zip

Change History (115)

@nacin
13 years ago

#1 @scribu
13 years ago

  • Cc scribu added

#2 @nacin
13 years ago

20103.diff rewrites search_theme_directories(). It uses scandir() rather than opendir/readdir, eliminates unnecessary loops in favor of file_exists() checks, takes maximum indentation from 9 tabs down to about 5, and makes some attempt to improve variable names.

The average time it took for search_thmee_directories() to run over wp-content/themes/ (where /themes/ was a checkout of http://wpcom-themes.svn.automattic.com) went from to 0.02042368 to 0.00315408 seconds, an 85% speed-up.

#3 @nacin
13 years ago

Bad branching in 20103.diff. Adjusted (and the whole process made clearer) in search_theme_directories.diff.

#4 @ocean90
13 years ago

  • Cc ocean90 added

#5 @kawauso
13 years ago

  • Cc kawauso added

#6 @toscho
13 years ago

  • Cc info@… added

@nacin
13 years ago

Fixes a bad conditional.

#7 @kirasong
13 years ago

  • Cc mike.schroder@… added

#8 @nacin
13 years ago

WP_Themes_List_Table::search_theme() does a sanitize_title_with_dashes() on every tag of every theme.

This is not necessary. Tags are already hyphen-delimited, or we should be doing that in get_themes() or get_theme_data() to begin with.

In terms of performance, it's terrible. With the WP.com themes directory loaded up, search_theme() takes up more than 50% of processing time. 40% of processing time is spent in sanitize_title_with_dashes().

Simply removing the sanitization (and slightly better branching) results in a tremendous speed-up. In my working patch, it works out to be 17% of the processing time with a page that is twice as fast.

@nacin
13 years ago

#9 @nacin
13 years ago

  • Milestone changed from Awaiting Review to 3.4

wp_theme.diff

Introducing the WP_Theme class

As the ticket promises, WP_Theme replaces get_themes() and other enemies of sanity. Each theme is its own object, decorated on construction and packed with 21 main public methods, four public static methods, and a few internal helpers. Full support for multiple theme roots and directories of themes within a theme root. The handling for broken-ness and odd situations is smoother than get_themes().

array wp_get_themes( $args = array() );

To fetch a array of WP_Theme objects, call wp_get_themes(). wp_get_themes( $args ) takes three arguments that filter the results: errors, allowed, and blog_id.

  • errors (Default: FALSE) - Filters themes based on whether they have errors. True returns only themes with errors, False only themes without errors, Null if all themes should be returned.
  • allowed (Default: NULL) - Filters themes based on whether they are allowed on the site or network. (Multisite only.) Valid options are 'network' (allowed on the network), 'site' (allowed on the site specified by the blog_id argument), true (allowed at either level), false (not allowed).
  • blog_id (Default: current) - Controls which site should be checked for the allowed filter. Only works, of course, in multisite.
WP_Theme wp_get_theme( $stylesheet = null, $theme_root = null );

To fetch a single theme, call wp_get_theme(). This will return a single WP_Theme object for the requested theme.

It takes two argumements: $stylesheet and $theme_root. If $stylesheet is omitted, the current stylesheet (via get_stylesheet()) will be used. If the $theme_root is omitted, get_raw_theme_root($stylesheet) will be used. Thus, to get the current theme, you may call wp_get_theme() with no arguments.

wp_get_theme() replaces get_theme(). get_theme() previously used get_current_theme() to get the name-keyed theme array from get_themes(). Not only are we no longer juggling arrays keyed by the theme name, we're not even processing every theme into memory to get a single WP_Theme object. Nice.

Handling Errors

Currently, search_theme_directories() and get_themes() will build a separate array of broken themes, $wp_broken_themes. This array is then returned by get_broken_themes().

With WP_Theme, errors like no template, invalid parent themes, or unreadable stylesheets can be nicely caught. Errors are populated into an errors property, accessible via WP_Theme->errors(). It is either null or no errors, or a WP_Error object. wp_get_themes( array( 'errors' => true ) ) enables one to loop through and pull out error messages and codes from a WP_Error object. In some cases, the current theme could 'error' but it still works, and we have access to what went wrong.

Network Support

WP_Theme encapsulates the logic for allowed themes on a network, with static methods to fetch all allowed themes on a network, themes allowed at the site level, and a merge of both lists (all themes allowed for a single site).

Each theme object then has a simple is_allowed( $check = 'both', $blog_id = 0 ) method to allow for checking whether it is allowed.

The okay-named function get_site_allowed_themes() and the terribly named function wpmu_get_blog_allowedthemes() are both deprecated in favor of static class methods.

Outputting Headers

What started me on WP_Theme was that get_themes() could not be internationalized in any sane way (#15858). The data usually came out of get_theme_data() already cleaned and even marked up (such as with wptexturize), making translation difficult or impossible. Since the data was keyed by name, changing that value would break quite a bit. As get_themes() already stored a lot of data in memory (and blows up memcached on WP.com, as they try to store it persistently), adding more keys didn't make sense.

mixed WP_Theme->get( $header )
mixed WP_Theme->display( $header, $markup = true, $translate = true )

WP_Theme has two main method for gathering header data: get() and display(). Once you call get(), the data has already been sanitized, either by strip_tags(), kses, or esc_url, depending on the header. (Tags are returned as an array.) By calling display(), the data is translated, and then marked up. (Even Tags — the array is translated for known tags.) The textdomain is automatically loaded if necessary.

display() returns data. It does not echo it. It is formatting the data for display, and would roughly be analogous to get_bloginfo( 'name', 'display' ).

get('Author') returns Andrew Nacin. display('Author') marks that up with <a> tags and a link to http://nacin.com (being AuthorURI).

display() also allows you to decline markup and translation with additional arguments, but these are mainly used for backwards compatibility or to allow fields to be searched through without Author HTML and the like.

Private methods sanitize_header(), translate_header(), and markup_header() support get() and display() in their endeavors.

Helpers

WP_Theme has a number of public methods that corresponds to the global function acting on the active theme. These include:

bool   WP_Theme->is_child_theme()
string WP_Theme->get_stylesheet()
string WP_Theme->get_template()
string WP_Theme->get_stylesheet_directory()
string WP_Theme->get_template_directory()
string WP_Theme->get_stylesheet_directory_uri()
string WP_Theme->get_template_directory_uri()
string WP_Theme->get_theme_root()
string WP_Theme->get_theme_root_uri()
bool   WP_Theme->load_textdomain()
array  WP_Theme->get_page_templates()

In some cases (notably, get_page_templates()), the global function now wraps the method.

Screenshots

string function get_screenshot( $uri = 'relative' )
int    function get_screenshot_count()
array  function get_screenshots()

3.4 means multiple screenshots. WP_Theme has you covered. Three public methods provide the screenshot URL (absolute URL, or relative to the stylesheet directory), the total count of screenshots, and an array of each screenshot filename.

Template and Stylesheet Files

array WP_Theme->get_files( $type = null )

get_themes() stored the template and stylesheet files in the array, which resulted in longer processing times and, especially since absolute paths were added in 2.9, an explosion in memory usage.

WP_Theme handles traversing the PHP and CSS files theme directories on demand. As these are only used in the theme editor (and only for the current theme), this is a pretty huge performance and memory boost.

Full Backwards Compatibility

WP_Theme is 100% backwards compatible. Because it implements ArrayAccess, you can take a theme object and look for keys like 'Template Files', 'Stylesheet', and 'Version' — just as they were returned by get_themes(). WP_Theme also includes a magic getter, handling all properties that were returned by current_theme_info() (which returned stdClass in 3.3). This enables us to pass a fully decorated WP_Theme object as an existing action argument, without plugins choking because they expected an array or an older object.

Indeed, get_themes() now returns an array of WP_Theme objects, because it can. get_theme() returns a WP_Theme object, as does current_theme_info(). (All are deprecated, as is get_broken_themes().)

If you don't believe me on backwards compatibility, head over to the Theme Editor, which passes around theme names like they're crack. This file is the only area that hasn't been updated to rely on WP_Theme; it still uses get_themes() and the array it returns for each theme. Everything works.

The Parent Theme

A reference to a WP_Theme object for a parent theme is stored in the parent property, and accessible with the parent() method. Want a theme's parent name? $theme->parent() && $theme->parent()->display('Name').

Caching and Performance

Okay, so, this is sweet. After profiling get_themes() and WP_Theme with Xdebug and KCacheGrind, a few functions have been sped up, in some cases pretty significantly. This includes search_theme_directories() and theme searching, which was a drag for no real reason. Searching is now improved significantly, and is implemented as a class method to allow for better speeds. (Now that things are "ready," I hope to go back and look for further improvements.)

Once sorting was fixed, two things stood out as weighing down processing the most: Filesystem reads, and sanitization of headers. With WP_Theme, both are non-persistently cached, internally, to ensure serious speed. No style.css is ever read twice, and no header is ever sanitized twice. Same goes for identifying screenshots and PHP/CSS files — everything gets cached.

Non-persistence is a must for themes because someone might make an FTP change then refresh the page. BUT, that doesn't mean a particular installation shouldn't be able to opt into persistence.

When the themes cache bucket is persistent (there's a filter), WP_Theme gets smart about it. To prevent thousands of cache sets to update the headers cache, it sanitizes every header once the first one is requested and adds it to cache. Additionally, search_theme_directories() is cached.

With persistent caching, it is possible to hit themes.php, and even do a search, and avoid a single filesystem or kses operation. The 150-something themes in the WP.com themes repository sings in this situation.

What's Next?

  1. Commit this bad boy.
  1. If object cache drop-ins had a wp_cache_get_non_persistent_groups() function, we could scrap that filter and make it much cleaner. (It's a rough implementation and could use some tweaks anyway.)
  1. Some of the filesystem functions — specifically, the underlying scandir() static method and get_files() — don't quite have their arguments or loops ironed out yet. Once the theme editor is converted over, those things should come into place nicely. Code review appreciated.

#10 follow-up: @scribu
13 years ago

Could we take this opportunity to find better names for get_template() / get_stylesheet() and co.?

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

Replying to scribu:

Could we take this opportunity to find better names for get_template() / get_stylesheet() and co.?

We could indeed! It was just not a priority during the construction. And, for my sanity, it was easiest to code it in the existing paradigm.

#12 follow-up: @ryan
13 years ago

So. Freaking. Awesome.

#13 @jeremyfelt
13 years ago

  • Cc jeremy.felt@… added

#14 @scribu
13 years ago

I actually looked at the patch now. It implements ArrayAccess and everything!

We can figure out the naming after the initial commit.

#15 @scribu
13 years ago

  • Keywords has-patch commit added

#16 @sabreuse
13 years ago

  • Cc sabreuse@… added

#17 @aaroncampbell
13 years ago

  • Cc aaroncampbell added

#18 @JustinSainton
13 years ago

  • Cc JustinSainton added

I don't often profess my love for other men, but when I do, it's for nacin.

#19 @divinethemes
13 years ago

  • Cc divinethemes added

#20 @goto10
13 years ago

  • Cc dromsey@… added

#21 @nacin
13 years ago

Related tickets I could find:

  • #17597, register_theme_directory() does not work for files outside WP_CONTENT_DIR (WP_PLUGIN_DIR does not need to be within WP_CONTENT_DIR, for example). get_theme_root() always prepends WP_CONTENT_DIR, but get_raw_theme_root() sometimes returns an absolute path instead of the relative-to-content path as documented. The patch here tries to fix this but not correctly. Coming up with a better solution.
  • #16744, theme editor does not work when the parent theme is in a different theme root. Should be easy to fix with WP_Theme.
  • #10067 (closed as duplicate), theme_basename() function. This is superseded by the power available here.

If you can find any other tickets related to the theme or theme root API, please shout.

#22 @nacin
13 years ago

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.

#23 follow-up: @SergeyBiryukov
13 years ago

Possibly related: #18501, #BB1596

#24 @nacin
13 years ago

In [20002]:

Move the template loading functions from wp-includes/theme.php to wp-includes/template.php. This includes get_query_template(), locate_template(), and friends. see #20103.

#25 in reply to: ↑ 23 @nacin
13 years ago

Replying to SergeyBiryukov:

Possibly related: #18501, #BB1596

Commenting in #18501. Turns out that line of code is dead code — the variable it sets up is never used. I missed it in my audit, but removing it now.

#26 @F J Kaiser
13 years ago

Pretty nice class and +1 for translatable theme headers.

@Nacin: Some Questions...
I only briefly looked over the code

  1. Couldn't - inside function offsetGet( $offset ) - the default be set to return $this->display( $offset ); (instead of null), so we'd have the ability to add (for e.g.) DataBaseVersion to the header (we still have no update/upgrade hook for themes)?
  2. Extending the switch for Author & Description inside function sanitize_header( $header, $value ) - wp_kses() with the allowed tags of cite for addresses and strike for update notes wouldn't hurt. I'd also love to see sub/sup and maybe even u be available.
  3. For cases similar to this one (filtering page templates) it would be nice to have a filter for the returned list of themes. Currently it's only in use to build the templates drop-down for the post edit/new screen.
Last edited 13 years ago by F J Kaiser (previous) (diff)

#27 @ocean90
13 years ago

I have two issues with the class:

  • Theme editor can't find the theme – "The requested theme does not exist."
  • themes.php: The current theme display is broken: http://cl.ly/EaoG

Also the patch doesn't apply cleanly anymore.

#28 @scribu
13 years ago

  • Keywords commit removed

@nacin
13 years ago

#29 @GaryJ
13 years ago

  • Cc gary@… added

On the naming issue, could we fix the non-existent word "stylesheet"? It has, and always was two words, as per the CSS abbreviation, and the names on the corresponding W3C documents.

After a very quick look through the patch, I see $stylesheet only as a private property, or as the argument names / variables inside functions, so they could be switched to $style_sheet without fuss.

Any standalone function like get_stylesheet() that remains after scribu has had his suggestion of renaming them, could stay as it was, but the methods introduced in the class could be renamed as get_style_sheet(). Obviously Stylesheet array keys would stay as they are too, unless you wanted to allow both versions.

I suspect the suggestion will be disregarded on grounds of introducing inconsistency, but since you're re-writing a hefty chunk of the internals, now is the most opportune moment to suggest improvements to the internals to match the naming conventions in the coding standards.

#30 @nacin
13 years ago

ocean90's difficulties were a result of r20001. See #17597. Once we have an idea there for how to proceed, 20103.3.diff will land.

Given the universal (in WordPress) usage of stylesheet to refer to a particular theme directory (and not necessarily CSS files), we're not going to change it to style_sheet.

#31 @nacin
13 years ago

In [20018]:

Clarify second argument for get_raw_theme_root() as the multiple negatives can get confusing. see #20103.

#32 @nacin
13 years ago

In [20020]:

Rewrite search_theme_directories() -- better performance and allow for caching.

  • Update it to use PHP5 scandir().
  • Don't scan individual theme dirs -- we only need to check for style.css. (Solid performance gains.)
  • Improve and simplify branching.
  • Introduce wp_cache_themes_persistently filter to enable persistent caching of the return value, based on the theme_roots transient.

see #20103.

#33 @nacin
13 years ago

In [20021]:

Theme root transient caching is now in search_theme_directories(), not get_themes(). see #20103.

#34 @nacin
13 years ago

In [20022]:

Do not save the last visited tab on the multisite Network Themes page and Site Themes tab. These are poor UX, see #18810 for plugins. Entering these screens will always default to 'all' themes view. see #20103.

#4 @nacin
13 years ago

In [20025]:

Add argument to get_theme_feature_list() to allow the tags to be returned without hitting the WP.org API. We'll be using this to translate tags. Remove trailing space in the 'Light' string. see #20103.

#5 @nacin
13 years ago

In [20026]:

Don't sanitize theme tags while trying to search through them, as it is unnecessary. It is also very expensive -- 50% of the pageload for a search was spent sanitizing tags. see #20103.

#6 @nacin
13 years ago

In [20027]:

Faster theme searching. Only calculate what is necessary -- if the theme doesn't have all of the features, bail. If a word matches a tag or header, jump to the next word, we don't care how many times it matches. see #20103.

#7 @nacin
13 years ago

In [20029]:

Introduce WP_Theme, wp_get_themes(), and wp_get_theme() to replace get_themes(), get_theme(), get_theme_data(), current_theme_info(), and others.

  • Getters and Helpers: Introduces a series of methods to allow for easy generation of headers for display, and other theme metadata, including page templates.
  • Screenshots: Handles support for multiple screenshots. (see # Additional screenshots must be PNG and start with screenshot-2.png, and be sequential to be counted. see #19816.
  • Error Handling: Broken themes have a WP_Error object attached to them.
  • Caching: Introduces a wp_cache_themes_persistently filter (also in [20020]) to enable persistent caching of all filesystem and sanitization operations normally handled by WP_Theme (and formerly get_file_data() and get_themes()). Themes are cached individually and across five different cache keys for different data pieces.
  • Compatibility: A WP_Theme object is backwards compatible with a theme's array formerly returned by get_themes() and get_theme(), and an stdClass object formerly returned by current_theme_info().
  • i18n/L10n: Theme headers are now localizable with proper Text Domain and Domain Path headers, like plugins. (Language packs may remove the requirement for headers.) For page templates, see #6007 (not fixed yet, but will be easy now). For headers, fixes #15858.
  • PHP and CSS files: New methods that fetch a list of theme files (for the theme editor) only on demand, rather than only loading them into memory. fixes #11214.

Functions deprecated:

  • get_themes(), get_allowed_themes() and get_broken_themes() -- use wp_get_themes()
  • get_theme() and current_theme_info() -- use wp_get_theme()
  • get_site_allowed_themes() -- use WP_Theme::get_allowed_on_network()
  • wpmu_get_blog_allowedthemes() -- use WP_theme::get_allowed_on_site()

see also [20016], [20018], [20019], [20020], [20021], [20022], [20025], [20026], [20027]. also fixes #19244.

see #20103.

#8 @nacin
13 years ago

I opened #20138 to discuss what happens to get_current_theme().

#9 @nacin
13 years ago

In [20033]:

Update switch_theme() to use wp_get_theme(). see #20103, #20138.

#3 @nacin
13 years ago

In [20037]:

Use wp_get_theme() rather than get_theme_data() in the verify theme deletion (multisite network) screen. see #20103.

#4 @nacin
13 years ago

In [20038]:

Use proper object. see #20103.

#5 @nacin
13 years ago

In [20039]:

(string) WP_Theme is now the theme name, translated. Good replacement for get_current_theme(); better than wp_get_theme()->display('Name'). see #20103, see #20138.

#5 @nacin
13 years ago

In [20040]:

Deprecate get_current_theme(). Use (string) wp_get_theme() to get the translated name of the theme. Keep the current_theme option for now. see #20103, see #20138.

#6 @nacin
13 years ago

New ticket, #20146 — on being back compat with old MU code for allowed themes.

#7 @nacin
13 years ago

In [20043]:

Have WP_Theme::get_screenshot() default to an absolute URI. Allow 'relative' to be requested. see #20103, see #19816.

#18 @nacin
13 years ago

In [20044]:

Remove debug cruft. see #20103.

#19 @nacin
13 years ago

In [20045]:

Use correct variable; return correct value for get_screenshot() when value is uncached. see #20103.

#20 @mbijon
13 years ago

  • Cc mike@… added

#21 @nacin
13 years ago

In [20052]:

Remove extra $. see #20103.

#22 @nacin
13 years ago

In [20053]:

Strict comparisons for the $allowed arg for wp_get_themes(). Otherwise 'network' does == true. see #20103.

#23 follow-up: @pauldewouters
13 years ago

  • Cc pauldewouters added

#24 @nacin
13 years ago

In [20105]:

Allow get_screenshots() to return absolute URLs. Make this the default; relative can be achieved with 'relative' as the argument. see #20103.

#25 @nacin
13 years ago

In [20112]:

Provide back compat for $theme->$var where $theme was an array from get_themes() but cast to an object, as had occurred in get_theme_updaes(). see #20173. see #20103.

#3 @nacin
13 years ago

In [20113]:

Use new WP_Theme API in list_theme_updates(). fixes #20173. see #20103.

#4 @nacin
13 years ago

get_theme_updates() could use some work. I don't like how it assigns a property directly to WP_Theme — WP_Theme has no public properties, only read-only back compat ones.

It should instead wrap wp_get_themes( array( 'updates' => true ) ). WP_Theme should set up an update property to automatically pull from the site transient. We can cache the site transient value as a static variable in WP_Theme->update(), to prevent calling get_site_transient() over and over.

The structure ends up being backwards compatible so we could easily continue to use get_theme_updates() rather than wp_get_themes(), but since method access will be preferred to property access, we should probably switch to wp_get_themes().

#5 @nacin
13 years ago

In general, update-related code needs an audit. wp_theme_update_row() will also need work for sure.

#6 @nacin
13 years ago

In [20116]:

Cast $wp_theme_directories to array. This had been done in get_themes(). A must-loaded plugin could call these functions before the first theme root is ever added. see #20103.

#7 @nacin
13 years ago

In [20118]:

Store relative-to-wp-content theme roots, as that is what get_theme_roots() should be receiving. see #20103.

Since search_theme_directories() now generates this and leverages this as a cache, convert absolute to relative on cache storage, and relative to absolute on cache retrieval.

#8 @nacin
13 years ago

In [20119]:

Default themes always trump their pretenders. Even though we are no longer keying by name, it is not a good experience to see multiple 'WordPress Default' themes when one was copied and had the directory renamed. This also keeps with the behavior of get_themes() (and the deprecated return value). see #20103.

Allow a default theme within a directory of wp-content/themes (e.g. 'somedir/twentyeleven') to be identified as a default theme.

#9 @nacin
13 years ago

In [20126]:

Fix variable collision in WP_Theme->get() that rears its head when the themes bucket is cached persistently. Also correct the inline doc. see #20103.

#10 follow-up: @nacin
13 years ago

In [20132]:

Fetch the raw 'Template' header in the WP_Theme constructor. By passing sanitization routines (which are unnecessary for this header), we prevent a persistent themes cache from sanitizing the headers of every theme until/unless they actually need a real header like Name. Note that if WP_Theme was instantiated through get_themes(), this will have no effect, as get_themes() does ask for Name for the keys to return. see #20103.

#11 @nacin
13 years ago

[20126] had haunted me previously, but I never found the bug and chalked it up to a caching quirk. [20132] is a nice performance improvement for a site like WP.com, which might have 500 themes but only a minority of them are for public consumption. Assuming there are no get_themes() calls, [20132] limits the amount of header sanitization and cache_get calls taking place to the themes that will actually be displayed.

#12 follow-up: @nacin
13 years ago

In [20144]:

Provide back compat for allowed_themes option keys, which were from the early days of WordPress MU. see #20103. Curses.

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

Replying to nacin:

In [20144]:

Provide back compat for allowed_themes option keys, which were from the early days of WordPress MU. see #20103. Curses.

Fixes #20146.

#14 @nacin
13 years ago

In [20145]:

Only update the old allowed_themes network option from the main site admin or in the network admin. see #20103, #20146.

#15 @nacin
13 years ago

In [20146]:

On display, replace an empty Theme Name with the theme's directory. see #20103.

#16 @nacin
13 years ago

In [20147]:

Always print the Author in the MS themes list table. display('Author') will be 'Anonymous' if no author is specified. see #20103.

#17 @nacin
13 years ago

In [20151]:

In multisite, list broken themes on network/themes.php instead of on individual Manage Themes pages. see #20103.

#18 @nacin
13 years ago

In [20152]:

In wp_get_themes() under multisite, filter out allowed themes before creating WP_Theme objects, rather than after. search_theme_directories() provides us the stylesheet of each theme which is all we need to do the proper intersection. This results in major performance gains on sites that have large numbers of themes unavailable to them. see #20103.

#19 @ocean90
13 years ago

Related to "list broken themes on network/themes.php...": #20099


The theme editor still use get_themes():

$ ack ' get_themes()'
wp-admin/theme-editor.php
47:$themes = get_themes();

#20 @nacin
13 years ago

In [20160]:

Don't show the Network Enable link for broken themes. Include broken themes in searching on network/themes.php. fixes #20099, see #20103.

#21 @nacin
13 years ago

[20151] and [20152] together mean big performance improvements in multisite.

Because errors were displayed on themes.php to super admins, when they visited themes.php for a site, every installed theme needed to be instantiated as a WP_Theme (to check for errors). [20151] moved this to the network admin, where every theme needs to be instantiated anyway. [20152] means that only allowed themes get instantiated on themes.php for a site. End result is very nice.

The theme editor still use get_themes()

Yeah, I haven't had the chance to go through it and update it to use the new API. You will find some Edit links to it broken (as they were updated to use the slug), but the editor itself works. If someone wishes to take a crack at it, I would be interested to hear your feedback about the API.

#22 @nacin
13 years ago

In [20161]:

Use correct variable. see #20103.

@nacin
13 years ago

#23 follow-up: @nacin
13 years ago

20103.4.diff improves get_theme_root_uri():

  • Short-circuits if only one directory is added (as we do elsewhere)
  • Allows a theme_root to be passed, which would then avoid the call to get_raw_theme_root(), which in turn will look up the theme root from the transient. If you already have this information, passing it can save some execution, especially if this is called for many themes (such as a list of screenshots on themes.php).

Additionally, WP_Theme->get_theme_root_uri() will now call get_theme_root_uri(), therefore triggering the theme_root_uri filter. This is the only helper method in WP_Theme that wraps its function counterpart. This is because it forms the basis for all other URIs returned by the theme object. Of all the data points, it's the one we do want to be filterable.

#24 @nacin
13 years ago

In [20162]:

Pass WP_Theme->get_theme_root_uri() to get_theme_root_uri(), thereby always triggering the theme_root_uri filter. see #20103.

#25 @nacin
13 years ago

In [20163]:

Store stylesheet rather than theme name in the theme_switched option. see #20103.

#26 @nacin
13 years ago

In [20173]:

The default value of a theme's Status header is 'publish', not 'public'. see #20103.

#27 @nacin
13 years ago

In [20176]:

Hash WP_Theme cache keys to ensure long file paths don't overflow in object backends. see #20103.

#28 @nacin
13 years ago

Unit tests [570], [578], [579]. A few things in the next commit here are either bug fixes, or tweaks to allow testing.

#29 @nacin
13 years ago

In [20193]:

Updates to WP_Theme, wp_get_themes(), and related deprecated functions, after [UT570] [UT578] [UT579]. see #20103.

  • Template Files? and Stylesheet Files? need to return files from the parent theme as well.
  • Don't strip links from the Author header. Some themes rely on the previous behavior, such as to link multiple authors (Sandbox, for example.) Don't restore links to the Name, that's just a bad idea.
  • Ensure we are always passing around arrays in get_files/scandir.
  • Better inline doc for wp_get_themes() arguments.
  • Introduce a 'force' flag for search_theme_directories() to re-scan, rather than return the cache. We will use this to re-build the theme_roots transient in get_theme_roots(), but it is more helpful for unit tests. Since search_theme_directories() is cached, don't cache again in wp_get_themes(). (Again benefits testing.)
  • Handle duplicate theme names in the old get_themes() when two themes match (and neither are a default theme, which is already handled). wp_get_themes() will consider both names to be the same; this is just for back compat since get_themes() is keyed by name.
  • Include an old array key in wp_broken_themes().

#30 @nacin
13 years ago

In [20233]:

Sanitize Theme URI and Author URI in WP_Theme with esc_url_raw. Escape with esc_url on display. see #20103.

#31 @nacin
13 years ago

In [20238]:

WP_Theme::get_allowed_on_network() doesn't take a blog_id argument. props duck_. Restore static declaration accidentally removed in [20193]. see #20103.

#32 @nacin
13 years ago

In [20268]:

Leverage WP_Theme in the upgrader. Remove debug line from [20267]. Stop using get_theme_data() as it will be deprecated. see #13774. see #20103.

#33 @nacin
13 years ago

In [20269]:

Deprecate get_theme_data(). Use wp_get_theme() instead. see #20103.

#34 @nacin
13 years ago

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

New tickets for any bug reports or enhancements.

#35 @nacin
13 years ago

In [20312]:

Introduce WP_Theme->exists() to check if the queried theme actually exists. WP_Theme->exists() is a subset of errors() -- a theme with errors may still exist, but a theme that does not exist has an error of theme_not_found. wp_get_theme() now returns false if the theme does not exist. Improve scandir() and get_files() logic. see #20103.

#36 @nacin
13 years ago

In [20313]:

Rewrite theme-editor.php to use the new WP_Theme API. see #20103.

#37 @nacin
13 years ago

In [20314]:

urldecode() the incoming $file in the theme editor. see [20313] when the encode was added. see #2994 for the original bug report. see #20103.

#19 @nacin
13 years ago

In [20315]:

Always set WP_Theme->template even when there is an error and we have no idea what the template is. (Assume it is the stylesheet.) Prevents a number of issues including WP_Theme->is_child_theme() lying. Tidy the theme editor for broken themes and themes with no templates (PHP files), or no template (parent), or are broken. Allow broken themes to be edited. see #20103.

#21 @nacin
13 years ago

In [20317]:

Use new scandir() return value (key is path relative to theme, value is absolute path) in WP_Theme->get_page_templates(). Use parent()->get_page_templates() and merge in a parent's page templates, rather than extra logic. see [20312], see #20103.

#22 @nacin
13 years ago

In [20324]:

Correct docs for WP_Theme scandir(), _name_sort(), and _name_sort_i18n() methods. All of them are private. see #11216, #20103.

#23 follow-up: @nacin
13 years ago

In [20327]:

Set WP_Theme::scandir()'s default depth to 0 (flat), as intended. Cache untranslated page template names. Properly merge in a parent theme's templates when dealing with cached values. Always look one level deep for WP_Theme->get_files() regardless of file type. see #20103. see #11216.

#24 @nacin
13 years ago

checked_theme_switched() - #20334, [20330]

Expiring caches - #20331, [20328], [20329]

#25 @nacin
13 years ago

In [20331]:

Ensure we get a theme back from wp_get_theme() before checking ->errors(). see #20103.

#26 @nacin
13 years ago

In [20374]:

Child theme files need to override parent theme files. The array_merge() arguments are swapped. see #20103.

#27 @nacin
13 years ago

In [20375]:

Internally cache themes inside wp_get_themes() by theme_root as well as stylesheet, to avoid conflicts with future calls to wp_get_themes(). Always return only the last stylesheet found, as before. see #20103.

#28 @nacin
13 years ago

In [20508]:

Check for wp_get_theme() in Twenty Eleven before using it, for compatibility down to WP 3.2 when Twenty Eleven was first released. see #20103.

#29 @nacin
13 years ago

In [20551]:

If a cached theme root is no longer registered, skip it. Assume the cache is out of date. see #20103.

#30 @nacin
13 years ago

In [20557]:

Don't allow themes without a style.css editor to be edited. These themes are too broken to be salvaged by the theme editor. see #20103.

#31 @nacin
13 years ago

In [20558]:

A child theme with a missing parent might be is_child_theme() but not have a valid parent() object, so we need to check parent() too. see #20103.

#32 @nacin
13 years ago

In [20559]:

Adjust [20557] and check only parent(), as it is more semantic. We don't care if the theme considers itself to be a child theme, we care if it has a valid parent. see #20103.

#33 @nacin
13 years ago

In [20560]:

Issue a 'This theme is broken' error (when applicable) on the theme editor. see #20103.

#34 @nacin
13 years ago

In [20586]:

Documentation and visibility cleanups in WP_Theme.

  • Declare __toString() as public.
  • Use parent() and error() internally, rather than the properties.
  • Add and correct inline documentation throughout.
  • Attempt to clarify that display() is superior to get().

see #20546, see #20103.

#3 @nacin
13 years ago

In [20668]:

Don't try to translate tags in WP_Theme::translate_header() if we are not in the admin and get_theme_feature_list() isn't defined. see #20103.

#4 @nacin
13 years ago

In [20945]:

WP_Theme: If no 'Domain Path' header is specified, default to looking in the /languages directory. see #20103, see #20448.

#5 @ocean90
11 years ago

#16410 was marked as a duplicate.

Note: See TracTickets for help on using tickets.