Use get_(template|stylesheet) instead of get_option( '(template|stylesheet)' )

[ocean90]

While working on Gandalf we noticed, that it's not enough to filter the current theme with

add_filter( 'template', array( $this, 'get_template' ) );
add_filter( 'stylesheet', array( $this, 'get_stylesheet' ) );

The reason are some uses of get_option( 'template' ) or get_option( 'stylesheet' ).

Means we need additional filters:

add_filter( 'pre_option_stylesheet', array( $this, 'get_stylesheet' ) );
add_filter( 'pre_option_template', array( $this, 'get_template' ) );

It would be handy if get_option( '(template|stylesheet)' ) could be replaced with get_(template|stylesheet).

[scribu]

No-brainer.

[westi]

Should we deprecate the options too?

[ocean90]

get_template() is more or less a wrapper for get_option('template') but has the filter in it.

[nacin]

Replying to westi:

Should we deprecate the options too?

Can't, as one calls the other.

I wonder if we should instead have never adopted get_stylesheet() and get_template(), and then filters could be placed on option_stylesheet and option_template. We could even maintain compat by applying the existing 'stylesheet' and 'template' filters on a callback for option_stylesheet and option_template, respectively.

It's not like these are theme template functions. The wrappers aren't that necessary and they only cause problems when filtering them.

[koopersmith]

Replying to nacin:

We could even maintain compat by applying the existing 'stylesheet' and 'template' filters on a callback for option_stylesheet and option_template, respectively.

This crossed my mind as well. If the only difference between the wrapper and the option is a single filter, that's an invitation for confusion and an open door for bugs. We should probably unify the two, alias the filters, and make the wrapper call get_option() and nothing else.

[nacin]

20027.diff​ hits at what I was thinking.

[SergeyBiryukov]

Replying to nacin:

20027.diff​ hits at what I was thinking.

• Looks like option_stylesheet should be option_template in line 142.
• Shouldn't it be add_filter() rather than add_action()?

[westi]

Replying to nacin:

Replying to westi:

Should we deprecate the options too?

Can't, as one calls the other.

Well we could but the code would be horrible (scanning backtraces on every call) so yes it isn't feasible.

I wonder if we should instead have never adopted get_stylesheet() and get_template(), and then filters could be placed on option_stylesheet and option_template. We could even maintain compat by applying the existing 'stylesheet' and 'template' filters on a callback for option_stylesheet and option_template, respectively.

It's not like these are theme template functions. The wrappers aren't that necessary and they only cause problems when filtering them.

I still think that the non-get_option calls are better ones to recommend people use and don't think that deprecating get_stylesheet or get_template is helpful.

I think we should encourage people to use get_stylesheet and get_template rather than get_option.

But we should make it so that they can't get different results.

The best way to do this is to make some changes to get_option and add some special cases for these two options for filtering - this is the simplest way to preserve the filter ordering that we have now (otherwise you have to guess at the largest priority something is hooked into the option filter and the filters in the wrapper functions lose out when they previously won)

[nacin]

Found something else while going through theme.php:

	// Prevent theme mods to current theme being used on theme being previewed
	add_filter( 'pre_option_mods_' . get_current_theme(), '__return_empty_array' );

We don't take into account pre_option_mods_{stylesheet} here, the newer mods storage key. I imagine this plays significantly into Gandalf do I don't want to make a change here. Doing get_option('stylesheet') (assuming it is not filtered) makes sense; I imagine you will be substituting in your own filter.

[nacin]

Let's keep get_option() separate from get_stylesheet() and get_template(), and go with the original patch proposed.

I say this because sometimes you might want the raw option. For example, we should not change the code in wp_update_themes() (wp-includes/update.php). This patch won't apply cleanly after #20103. Refresh and commit.

[ocean90]

20027.2.patch​

Not sure if the changes in wp-admin/includes/schema.php are useful.

[nacin]

The issue with the change in get_raw_theme_root() is two-fold:

• One, it's designed to not be a filtered result.

• Two, the get_option('stylesheet') and get_option('template') checks are within a branch where it also checks for get_option('stylesheet_root'). This means if someone is filtering only template or stylesheet, they will mess up the return value of the function as they are not accounting for stylesheet_root and template_root. So I'd almost rather rather leave it at get_option() and force customize to continue to filter both values. (Customize will likely need to disable update checks via wp_update_themes, though.)

Thoughts?

[ocean90]

Since the Customizer handles both values, we can review this later.

[chriscct7]

Any updated thoughts on this?

[ocean90]

#37352 was marked as a duplicate.

[sebastian.pisula]

Patch for 4.8-alpha-40051

[audrasjb]

As per today's old tickets triage session:

From @hellofromTonya: Given the age of this ticket and how long these unfiltered usages have been in Core, why change some of them to filtered? The inconsistencies will still exist in Core. So it doesn't fully solve the problem request of reaching consistency. But it does increase the risks of unknown issues.

From @costdev: Any change to call get_stylesheet() will run the additional filter, so isn't this always going to be a potential BC break/regression because we don't know what someone is hooking to the 'stylesheet' filter?

Let's close this ticket as wontfix, and eventually open a new general ticket to handle inconsistencies in a future release.

Props @hellofromTonya, @joedolson, @SergeyBiryukov, @costdev for the triage discussion.