Make WordPress Core

Opened 7 years ago

Closed 3 years ago

#41271 closed defect (bug) (maybelater)

Customizer sanitize_callback gets called multiple times on setting change

Reported by: kylejennings83's profile kylejennings83 Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.8
Component: Customize Keywords: needs-patch
Focuses: performance Cc:

Description (last modified by westonruter)

I've noticed that the my Customizer setting sanitize_callback get's called multiple times (4). I think I have deduced that it's not anything weird I'm doing and that it's not a result of a conflicting plugin.

Using VVV, I spun up a new site, deactivated all the plugins and using only the default Twenty Seventeen theme I added the following to functions.php file:

<?php
function wp_132423_customize_settings($wp_customize){
    $wp_customize->add_setting( 'sidebar_size_setting', array(
        'default' => 'wide',
        'sanitize_callback' => function($val) {
            error_log('boom'); // to test
            },
        )
    );

    $wp_customize->add_control( 'sidebar_size_control', array(
            'label'   => 'Sizebar Size',
            'section' => 'title_tagline',
            'settings' => 'sidebar_size_setting',
            'type' => 'select',
            'choices' => array(
                'wide' => 'Wide',
                'narrow' => 'Narrow',
            ),
        )
    );
}
add_action('customize_register', 'wp_132423_customize_settings');

Then I opened the Customizer and changed my "Sizebar Size" control from wide to narrow while tailing the error log which shows:

[07-Jul-2017 18:12:22 UTC] boom
[07-Jul-2017 18:12:22 UTC] boom
[07-Jul-2017 18:12:22 UTC] boom
[07-Jul-2017 18:12:22 UTC] boom

I noticed this while trying to find a way to run a function in the backend whenever a particular setting was changed. Seemed like the sanitize_callback was a good place to hook into and noticed this.


Related: #32103, #42558.

Attachments (1)

blogname-setting-sanitize-calls.txt (14.6 KB) - added by westonruter 7 years ago.

Download all attachments as: .zip

Change History (9)

#1 @westonruter
7 years ago

@kylejennings83 The reason why it is being called multiple times is that the validated/sanitized value is not cached for a given changeset setting value. If you output the current_action() (or even debug_backtrace()) in the error log for each instance, this should illuminate better the times the situation that this is being called.

#2 @kylejennings83
7 years ago

@westonruter not quite sure I understand why that would cause it to be called multiple times. If I change only a single setting (and only once) why would the sanitize_callback get called 4 times in a row? I also noticed that validate_callback is called something like 6 times. I would imagine the callback only gets called a single time.

I threw error_log(current_action()) and only customize_sanitize_sidebar_size_setting was logged (and again 4 times)

#3 @westonruter
7 years ago

  • Focuses performance added

@kylejennings83 It's hard to say why the specific number is called without seeing your full codebase. If you look at WP_Customize_Setting::preview() you'll see that it does:

add_filter( "theme_mod_{$id_base}", array( $this, '_preview_filter' ) );

That means whenever you do get_theme_mod( 'sidebar_size_setting' ) it is going to call the WP_Customize_Setting::_preview_filter() method, which in turn is going to grab the unsanitized value and pass it through the sanitize_callback. So each time you try to read from the setting the sanitize logic will be re-run.

Here is a more robust way to log these sanitize calls:

<?php
$calls = array();

add_action( 'wp_footer', function() use ( &$calls ) {
        error_log( print_r( $calls, true ) );
} );

add_filter( 'customize_sanitize_blogname', function( $value ) use ( &$calls ) {
        global $wp_current_filter;
        $calls[] = array(
                'hook_stack' => $wp_current_filter,
                'call_stack' => array_slice( array_map(
                        function( $call ) {
                                return sprintf( '%s() at %s:%d', $call['function'], preg_replace( '#.+/src/#', '', $call['file'] ), $call['line'] );
                        },
                        debug_backtrace( DEBUG_BACKTRACE_IGNORE_ARGS )
                ), 1 ),
        );
        return $value;
} );

I've attached blogname-setting-sanitize-calls.txt to show the output. You can see the get_option() call is present in each call stack.

To reduce the number of times the sanitization is done, we could add a caching layer to \WP_Customize_Manager::post_value(). So it could check to see if a setting had previously been validated or sanitized, and then short-circuit to return the previously computed values. The cached values can then be cleared whenever the customize_post_value_set_{$setting_id} action is fired.

There hasn't been a clear demand for these changes in terms of any performance gains, but I'm happy to be shown otherwise.

#4 @westonruter
7 years ago

  • Milestone changed from Awaiting Review to 5.0

#5 @westonruter
7 years ago

  • Description modified (diff)

#6 @peterwilsoncc
6 years ago

  • Milestone changed from 5.0 to 5.1

Switching milestone due to the focus on the new editor (Gutenberg) for WordPress 5.0.

#7 @pento
6 years ago

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

#8 @celloexpressions
3 years ago

  • Milestone Future Release deleted
  • Resolution set to maybelater
  • Status changed from new to closed

Closing for now given lack of interest/movement. This issue seems to only cause issues with relatively unique use cases. If anyone wants to investigate potential solutions further, please reopen.

Note: See TracTickets for help on using tickets.