[wp-trac] [WordPress Trac] #35451: Customizer: aggregated multidimensional settings overwrite changes made outside the Customizer

[WordPress Trac]

#35451: Customizer: aggregated multidimensional settings overwrite changes made
outside the Customizer
-------------------------------+------------------------------
 Reporter:  mattwiebe          |       Owner:
     Type:  defect (bug)       |      Status:  new
 Priority:  normal             |   Milestone:  Awaiting Review
Component:  Customize          |     Version:  4.4
 Severity:  normal             |  Resolution:
 Keywords:  reporter-feedback  |     Focuses:
-------------------------------+------------------------------
Changes (by westonruter):

 * keywords:   => reporter-feedback


Old description:

> 4.4's aggregated multidimensional settings naïvely assumes that no other
> code will touch the root setting, but that's not necessarily true. Here's
> some pseudo code from WP.com:
>
> {{{
> $wp_customize->add_setting( 'custom_colors[colors]', array(
>   'sanitize_callback' => 'wpcom_sanitize_colors'
> ) );
> function wpcom_sanitize_colors( $colors ) {
>   // do some sanitization
>   // update the CSS used to output colors
>   $opts = get_option( 'custom_colors', array() );
>   $opts['css'] = wpcom_create_css_for_colors( $colors );
>   update_option( 'custom_colors', $opts );
>   return $colors;
> }
> }}}
>
> Unfortunately, the new aggregated multidimensional setting code assumes
> that the Customizer is the only thing that might make a setting change in
> WP and blindly overwrites the base option/theme_mod. In the above code,
> the `css` member will either reflect outdated colors, or not be set at
> all.
>
> What should probably happen is that the root value should be freshly
> grabbed in `set_root_value()` and `array_merge`'d with the current
> aggregate. Or maybe that code should be in `update()`? In any case, this
> broke a few things on WP.com when we finally started pushing out 4.4
> code.

New description:

 4.4's aggregated multidimensional settings naïvely assumes that no other
 code will touch the root setting, but that's not necessarily true. Here's
 some pseudo code from WP.com:

 {{{#!php
 <?php
 $wp_customize->add_setting( 'custom_colors[colors]', array(
   'sanitize_callback' => 'wpcom_sanitize_colors'
 ) );
 function wpcom_sanitize_colors( $colors ) {
   // do some sanitization
   // update the CSS used to output colors
   $opts = get_option( 'custom_colors', array() );
   $opts['css'] = wpcom_create_css_for_colors( $colors );
   update_option( 'custom_colors', $opts );
   return $colors;
 }
 }}}

 Unfortunately, the new aggregated multidimensional setting code assumes
 that the Customizer is the only thing that might make a setting change in
 WP and blindly overwrites the base option/theme_mod. In the above code,
 the `css` member will either reflect outdated colors, or not be set at
 all.

 What should probably happen is that the root value should be freshly
 grabbed in `set_root_value()` and `array_merge`'d with the current
 aggregate. Or maybe that code should be in `update()`? In any case, this
 broke a few things on WP.com when we finally started pushing out 4.4 code.

--

Comment:

 @mattwiebe: So are you saying that a `sanitize` callback is actually doing
 a ''write'' operation on the value for another setting, or rather here the
 setting root? If so, then I would say this would be a bad practice since
 as far as I see it, the sanitize should be a read-only operation. I'd
 suggest a better approach to be to hook into the `customize_save_after`
 action, like this:

 {{{#!php
 <?php
 $wp_customize->add_setting( 'custom_colors[colors]', array(
         'type' => 'option',
         'default' => array(),
         'sanitize_callback' => function( $colors ) { /* ... */ return
 $colors; },
 ) );
 add_action( 'customize_save_after', function ( $wp_customize ) {
         if ( array_key_exists( 'custom_colors[colors]',
 $wp_customize->unsanitized_post_values() ) ) {
                 // update the CSS used to output colors
                 $opts = get_option( 'custom_colors', array() );
                 $opts['css'] = wpcom_create_css_for_colors( $colors );
                 update_option( 'custom_colors', $opts );
         }
 } );
 }}}

--
Ticket URL: <https://core.trac.wordpress.org/ticket/35451#comment:1>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform