WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 months ago

Last modified 4 months ago

#14721 closed enhancement (fixed)

set_theme_mod() needs a filter

Reported by: mikeschinkel Owned by: nacin
Milestone: 3.9 Priority: normal
Severity: normal Version: 3.0.1
Component: Themes Keywords: has-patch
Focuses: Cc:

Description

set_theme_mod() needs a filter!!!

Background: I was answering this question on WordPress Answers about how to create a second background option in the admin. I was able to do so relatively easily except for the fact that the only hook available for swapping the value to save for set_theme_mod() is within get_option() and get_option() has no context for the name of the theme mod so I had to resort to the highly brittle technique of determining what was being saved based on the order it was called. One small change to /wp-admin/custom-background.php and it all breaks.

Prospective Update:
Here's what the filters might look like:

function set_theme_mod($name, $value) {
	$theme = get_current_theme();

	$mods = get_option("mods_$theme");

	$value = apply_filters( "set_theme_mod", $value, $name );
	$value = apply_filters( "set_theme_mod_$name", $value );
	$mods[$name] = $value;

	update_option("mods_$theme", $mods);
	wp_cache_delete("mods_$theme", 'options');
}

I would submit a patch but I always struggle with that. I even asked others to help me streamline and understand the patching process a bit better.

Attachments (2)

14721-r23302.diff (665 bytes) - added by ssmathias 18 months ago.
Patch against revision 23302 in trunk (cleaned and revised)
14721.diff (834 bytes) - added by obenland 7 months ago.

Download all attachments as: .zip

Change History (18)

comment:1 mikeschinkel4 years ago

  • Cc mikeschinkel@… added
  • Component changed from General to Themes
  • Type changed from defect (bug) to enhancement
  • Version set to 3.0.1

comment:2 Denis-de-Bernardy4 years ago

There's the option_mods_$theme filter in the meanwhile.

comment:3 follow-up: Denis-de-Bernardy4 years ago

and update_option_mods_$theme...

comment:4 in reply to: ↑ 3 mikeschinkel4 years ago

Replying to Denis-de-Bernardy:

There's the option_mods_$theme filter in the meanwhile.

Replying to Denis-de-Bernardy:

and update_option_mods_$theme...

Those are no better than the hooks I am using; If you read my ticket and look at the code I linked to you'll see the problem is there is no context for what theme setting name is being set, hence why I submitted the ticket.

comment:5 sorich874 years ago

  • Cc sorich87@… added

comment:6 nacin4 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release

comment:7 ssmathias18 months ago

  • Keywords has-patch added; needs-patch removed

I've generated a patch for this ticket adding two new filters, and two new actions to the set_theme_mod function:

$value = apply_filters('set_theme_mod', $value, $name, $mods[ $name ]);

$value = apply_filters("set_theme_mod_$name", $value, $mods[ $name ]);

do_action('post_set_theme_mod', $mods, $name, $value);

do_action("post_set_theme_mod_$name", $mods, $value);

comment:8 ssmathias18 months ago

  • Cc steve@… added

ssmathias18 months ago

Patch against revision 23302 in trunk (cleaned and revised)

obenland7 months ago

comment:9 obenland7 months ago

  • Milestone changed from Future Release to 3.9

Added simplified patch with filter documentation.

comment:10 ircbot5 months ago

This ticket was mentioned in IRC in #wordpress-dev by obenland. View the logs.

comment:11 follow-up: MikeHansenMe5 months ago

Is there a use case where it is better to filter before saving rather than filtering on retrieval? There is a filter in get_theme_mod() on line 896 that looks like

apply_filters( "theme_mod_{$name}", $mods[$name] );

I would think that could be used in any situation where the proposed filter would be used.

comment:12 in reply to: ↑ 11 ; follow-up: ssmathias5 months ago

Replying to MikeHansenMe:

Is there a use case where it is better to filter before saving rather than filtering on retrieval? There is a filter in get_theme_mod() on line 896 that looks like

apply_filters( "theme_mod_{$name}", $mods[$name] );

I would think that could be used in any situation where the proposed filter would be used.

Using a filter on save can be more performant if something is saved less frequently than it is read. Running filters does take time, so if it saves once, or even once a month, versus being read on page load, you can see how frequently the latter filters would run compared to the former.

comment:13 in reply to: ↑ 12 ; follow-up: MikeHansenMe5 months ago

Replying to ssmathias:

Using a filter on save can be more performant if something is saved less frequently than it is read. Running filters does take time, so if it saves once, or even once a month, versus being read on page load, you can see how frequently the latter filters would run compared to the former.

So in your situation you are filtering it in order to save it. Why would you not just update the setting and avoid a filter all together?

comment:14 in reply to: ↑ 13 ssmathias5 months ago

Replying to MikeHansenMe:

Replying to ssmathias:

Using a filter on save can be more performant if something is saved less frequently than it is read. Running filters does take time, so if it saves once, or even once a month, versus being read on page load, you can see how frequently the latter filters would run compared to the former.

So in your situation you are filtering it in order to save it. Why would you not just update the setting and avoid a filter all together?

In the use case that came up when I first got involved with the ticket, we actually cared more about an action than a filter. We needed to know when one theme mod was changed so that we could trigger some setting updates in another option. My patch added both filters and actions because there may be a conceivable use case in which someone's custom theme may want to control the input of these options.

comment:15 nacin4 months ago

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

In 27393:

Add a pre_set_theme_mod_$name filter to set_theme_mod().

This is modeled after pre_update_option_$option in update_option() and pre_set_transient_$transient in set_transient().

props obenland.
fixes #14721.

comment:16 nacin4 months ago

In 27402:

In set_theme_mod() account for when there is no old value.

fixes #14721. see [27393].

Note: See TracTickets for help on using tickets.