Changeset 38513

Timestamp:

09/02/2016 10:34:48 PM (8 years ago)

Author:

westonruter

Message:

Customize: Fix php warning due to WP_Customize_Manager::prepare_setting_validity_for_js() incorrectly assuming that WP_Error will only ever have arrays in its $error_data.

• Eliminates the server mutating the a WP_Error's $error_data to merge-in a $from_server flag (since it may not be an array to begin with). Instead it defers to the client to add a fromServer param on any Notification instances created from server-sent errors.
• Ensures that notifications will be re-rendered if a notification's message changes but the data and type remain the same.
• Adds explicit support for the Notification class to have a setting property, ensuring that the property is set whereas previously it was dropped.

Fixes #37890.
Props westonruter, dlh.

Location:

trunk

Files:

• src/wp-admin/js/customize-controls.js (modified) (2 diffs)
• src/wp-includes/class-wp-customize-manager.php (modified) (1 diff)
• src/wp-includes/js/customize-base.js (modified) (1 diff)
• tests/phpunit/tests/customize/manager.php (modified) (2 diffs)
• tests/qunit/wp-admin/js/customize-base.js (modified) (2 diffs)

trunk/src/wp-admin/js/customize-controls.js

@@ -3462 +3462 @@
                 if ( _.isObject( validity ) ) {
                     _.each( validity, function( params, code ) {
-                        var notification = new api.Notification( code, params ), existingNotification, needsReplacement = false;
+                        var notification, existingNotification, needsReplacement = false;
+                        notification = new api.Notification( code, _.extend( { fromServer: true }, params ) );
 
                         // Remove existing notification if already exists for code but differs in parameters.
                         existingNotification = setting.notifications( notification.code );
                         if ( existingNotification ) {
-                            needsReplacement = ( notification.type !== existingNotification.type ) || ! _.isEqual( notification.data, existingNotification.data );
+                            needsReplacement = notification.type !== existingNotification.type || notification.message !== existingNotification.message || ! _.isEqual( notification.data, existingNotification.data );
                         }
                         if ( needsReplacement ) {
@@ -3716 +3717 @@
                     api.each( function( setting ) {
                         setting.notifications.each( function( notification ) {
-                            if ( 'error' === notification.type && ( ! notification.data || ! notification.data.from_server ) ) {
+                            if ( 'error' === notification.type && ! notification.fromServer ) {
                                 invalidSettings.push( setting.id );
                             }

trunk/src/wp-includes/class-wp-customize-manager.php

@@ -1047 +1047 @@
             $notification = array();
             foreach ( $validity->errors as $error_code => $error_messages ) {
-                $error_data = $validity->get_error_data( $error_code );
-                if ( is_null( $error_data ) ) {
-                    $error_data = array();
-                }
-                $error_data = array_merge(
-                    $error_data,
-                    array( 'from_server' => true )
-                );
                 $notification[ $error_code ] = array(
                     'message' => join( ' ', $error_messages ),
-                    'data' => $error_data,
+                    'data' => $validity->get_error_data( $error_code ),
                 );
             }

trunk/src/wp-includes/js/customize-base.js

@@ -763 +763 @@
      * @since 4.6.0
      *
-     * @param {string} code                The error code.
-     * @param {object} params              Params.
-     * @param {string} params.message      The error message.
-     * @param {string} [params.type=error] The notification type.
-     * @param {*}      [params.data]       Any additional data.
+     * @param {string}  code - The error code.
+     * @param {object}  params - Params.
+     * @param {string}  params.message=null - The error message.
+     * @param {string}  [params.type=error] - The notification type.
+     * @param {boolean} [params.fromServer=false] - Whether the notification was server-sent.
+     * @param {string}  [params.setting=null] - The setting ID that the notification is related to.
+     * @param {*}       [params.data=null] - Any additional data.
      */
     api.Notification = api.Class.extend({
         initialize: function( code, params ) {
+            var _params;
             this.code = code;
-            this.message = params.message;
-            this.type = params.type || 'error';
-            this.data = params.data || null;
+            _params = _.extend(
+                {
+                    message: null,
+                    type: 'error',
+                    fromServer: false,
+                    data: null,
+                    setting: null
+                },
+                params
+            );
+            delete _params.code;
+            _.extend( this, _params );
         }
     });

trunk/tests/phpunit/tests/customize/manager.php

@@ -304 +304 @@
         $this->assertTrue( $this->manager->prepare_setting_validity_for_js( true ) );
         $error = new WP_Error();
-        $error->add( 'bad_letter', 'Bad letter' );
-        $error->add( 'bad_letter', 'Bad letra' );
+        $error->add( 'bad_letter', 'Bad letter', 'A' );
+        $error->add( 'bad_letter', 'Bad letra', 123 );
         $error->add( 'bad_number', 'Bad number', array( 'number' => 123 ) );
         $validity = $this->manager->prepare_setting_validity_for_js( $error );
@@ -314 +314 @@
             $this->assertEquals( join( ' ', $messages ), $validity[ $code ]['message'] );
             $this->assertArrayHasKey( 'data', $validity[ $code ] );
-            $this->assertArrayHasKey( 'from_server', $validity[ $code ]['data'] );
+            $this->assertEquals( $validity[ $code ]['data'], $error->get_error_data( $code ) );
         }
         $this->assertArrayHasKey( 'number', $validity['bad_number']['data'] );

trunk/tests/qunit/wp-admin/js/customize-base.js

@@ -165 +165 @@
             'message': 'Hello World',
             'type': 'update',
+            'setting': 'blogname',
+            'fromServer': true,
             'data': { 'foo': 'bar' }
         } );
@@ -171 +173 @@
         assert.equal( 'Hello World', notification.message );
         assert.equal( 'update', notification.type );
+        assert.equal( 'blogname', notification.setting );
+        assert.equal( true, notification.fromServer );
         assert.deepEqual( { 'foo': 'bar' }, notification.data );