Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#34140 closed defect (bug) (fixed)

Missing return values for WP_Customize_Setting::update( $value )

Reported by: wpweaver Owned by: westonruter
Milestone: 4.4 Priority: normal
Severity: normal Version: 3.4
Component: Customize Keywords: has-patch
Focuses: Cc:

Description (last modified by westonruter)

The docs for WP_Customize_Setting::update() method indicate it returns a value for “The result of saving the value.” For custom types (non-option or theme_mod) it will return via:

return do_action( 'customize_update_' . $this->type, $value, $this );

The problem is that do_action() does not return a value.

Attachments (1)

34140.diff (1.1 KB) - added by westonruter 5 years ago.

Download all attachments as: .zip

Change History (8)

#1 @westonruter
5 years ago

@wpweaver I don't understand what you're trying to do here with this action.

To supply a value for a custom type, you can use the filter 'customize_value_' . $this->id_data['base'] (which as reported elsewhere, is insufficient, see #29316).

To preview a modified value for a custom type, use the customize_preview_{$this->id} or customize_preview_{$this->type} actions.

To save a value for a custom type, use the action customize_update_{$this->type}.

Or, which can be much easier to manage, just subclass WP_Customize_Setting and then override the value, preview, and update methods respectively. To use such a custom setting subclass, instead of doing:

$wp_customize->add_setting( $id, $args );

You instead do:

$wp_customize->add_setting( new My_Custom_Setting( $wp_customize, $id, $args ) );
Last edited 5 years ago by westonruter (previous) (diff)

#2 follow-up: @westonruter
5 years ago

  • Keywords reporter-feedback added

#3 in reply to: ↑ 2 @wpweaver
5 years ago

Replying to westonruter:

My apologies - I missed seeing this reply this morning.

I was trying to add an action for customize_preview, just as you said.

However, the approach to use new MyCustom_Setting as the arg to add_setting works better. However, as far as I can find, that technique is not documented anywhere.

There has been a lot of discussion about using sub classes for customizer, but not much documentation or examples. It is not all that great to have to resort to reading code.

AND, this report is still a valid bug report as any line with "return do_action(xxxx)" is incorrect. One should not return a value from a function that does not return a value.

Last edited 5 years ago by wpweaver (previous) (diff)

5 years ago

#4 @westonruter
5 years ago

  • Description modified (diff)
  • Keywords has-patch added; reporter-feedback removed
  • Milestone changed from Awaiting Review to 4.4
  • Summary changed from WP_Customize_Setting::update( $value ) bad return value to Missing return values for WP_Customize_Setting::update( $value )
  • Type changed from defect (bug) to enhancement
  • Version changed from 4.3.1 to 3.4

The reality is that the return value for WP_Customize_Setting::update() (a protected method) value is not even used in Core. As I can see, it is only used in WP_Customize_Setting::save() and is called as:

$this->update( $value );

So its return value is not used. The WP_Customize_Setting::save() method is called via WP_Customize_Manager::save():

		foreach ( $this->settings as $setting ) {

The @return tag for the update() method says that the return value is supposed to be “The result of saving the value.” Currently this only returns a value (other than null/void) when it calls WP_Customize_Setting::_update_option(), since it does return a value. But WP_Customize_Setting::_update_theme_mod() does not return a value, and for custom types, calling do_action() also does not return a value.

In the latter case, it can just be changed to return whether there was was any callback added for that action to begin with.

See 34140.diff.

#5 @westonruter
5 years ago

  • Owner set to westonruter
  • Status changed from new to accepted

#6 @westonruter
5 years ago

  • Type changed from enhancement to defect (bug)

#7 @westonruter
5 years ago

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

In 34838:

Customizer: Ensure WP_Customize_Setting::update() returns boolean value.

Adds unit tests for WP_Customize_Setting::save() (and WP_Customize_Setting::update()), along with the actions customize_update_{$type}, and customize_save_{$id_base} which they trigger.

Fixes #34140.

Note: See TracTickets for help on using tickets.