WordPress.org
Get WordPress

Make WordPress Core

Changeset 33071


Ignore:
Timestamp:
07/03/2015 08:46:48 PM (10 years ago)
Author:
westonruter
Message:

Customizer: Fix saving menus with empty names or names that are already used.

Adds validation for initially-supplied nav menu name, blocking empty names from being supplied. If later an empty name is supplied and the nav menu is saved, the name "(unnamed)" will be supplied instead and supplied back to the client. If a name is supplied for the menu which is currently used by another menu, then the name conflict is resolved by adding a numerical counter similar to how post_name conflicts are resolved. Includes unit tests.

Fixes #32760.

Location:
trunk
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/src/wp-admin/css/customize-nav-menus.css

    r33028 r33071  
    655655
    656656#custom-menu-item-name.invalid,
    657 #custom-menu-item-url.invalid {
     657#custom-menu-item-url.invalid,
     658.menu-name-field.invalid,
     659.menu-name-field.invalid:focus {
    658660    border: 1px solid #f00;
    659661}

  • trunk/src/wp-admin/js/customize-nav-menus.js

    r33070 r33071  
    834834                button.attr( 'aria-expanded', 'false' );
    835835                content.slideUp( 'fast' );
     836                content.find( '.menu-name-field' ).removeClass( 'invalid' );
    836837            }
    837838        }
    … …  
    870871                }
    871872                menuId = matches[1];
    872                 option = new Option( setting().name, menuId );
     873                option = new Option( displayNavMenuName( setting().name ), menuId );
    873874                control.container.find( 'select' ).append( option );
    874875            });
    … …  
    896897                    control.container.find( 'option[value=' + menuId + ']' ).remove();
    897898                } else {
    898                     control.container.find( 'option[value=' + menuId + ']' ).text( setting().name );
     899                    control.container.find( 'option[value=' + menuId + ']' ).text( displayNavMenuName( setting().name ) );
    899900                }
    900901            });
    … …  
    16061607        ready: function() {
    16071608            var control = this,
    1608                 menuId = control.params.menu_id;
     1609                menuId = control.params.menu_id,
     1610                menu = control.setting(),
     1611                name;
    16091612
    16101613            if ( 'undefined' === typeof this.params.menu_id ) {
    … …  
    16371640
    16381641            // Add menu to Custom Menu widgets.
    1639             if ( control.setting() ) {
     1642            if ( menu ) {
     1643                name = displayNavMenuName( menu.name );
     1644
    16401645                api.control.each( function( widgetControl ) {
    16411646                    if ( ! widgetControl.extended( api.controlConstructor.widget_form ) || 'nav_menu' !== widgetControl.params.widget_id_base ) {
    … …  
    16441649                    var select = widgetControl.container.find( 'select' );
    16451650                    if ( select.find( 'option[value=' + String( menuId ) + ']' ).length === 0 ) {
    1646                         select.append( new Option( control.setting().name, menuId ) );
     1651                        select.append( new Option( name, menuId ) );
    16471652                    }
    16481653                } );
    1649                 $( '#available-widgets-list .widget-inside:has(input.id_base[value=nav_menu]) select:first' ).append( new Option( control.setting().name, menuId ) );
     1654                $( '#available-widgets-list .widget-inside:has(input.id_base[value=nav_menu]) select:first' ).append( new Option( name, menuId ) );
    16501655            }
    16511656        },
    … …  
    16781683
    16791684            control.setting.bind( function( to ) {
     1685                var name;
    16801686                if ( false === to ) {
    16811687                    control._handleDeletion();
    16821688                } else {
    16831689                    // Update names in the Custom Menu widgets.
     1690                    name = displayNavMenuName( to.name );
    16841691                    api.control.each( function( widgetControl ) {
    16851692                        if ( ! widgetControl.extended( api.controlConstructor.widget_form ) || 'nav_menu' !== widgetControl.params.widget_id_base ) {
    … …  
    16871694                        }
    16881695                        var select = widgetControl.container.find( 'select' );
    1689                         select.find( 'option[value=' + String( menuId ) + ']' ).text( to.name );
     1696                        select.find( 'option[value=' + String( menuId ) + ']' ).text( name );
    16901697                    });
    1691                     $( '#available-widgets-list .widget-inside:has(input.id_base[value=nav_menu]) select:first option[value=' + String( menuId ) + ']' ).text( to.name );
     1698                    $( '#available-widgets-list .widget-inside:has(input.id_base[value=nav_menu]) select:first option[value=' + String( menuId ) + ']' ).text( name );
    16921699                }
    16931700            } );
    … …  
    18481855                        container.find( '.theme-location-set' ).hide();
    18491856                    } else {
    1850                         container.find( '.theme-location-set' ).show().find( 'span' ).text( menuSetting().name );
     1857                        container.find( '.theme-location-set' ).show().find( 'span' ).text( displayNavMenuName( menuSetting().name ) );
    18511858                    }
    18521859                };
    … …  
    18801887                }
    18811888
    1882                 // Empty names are not allowed (will not be saved), don't update to one.
    1883                 if ( menu.name ) {
    1884                     var section = control.container.closest( '.accordion-section' ),
    1885                         menuId = control.params.menu_id,
    1886                         controlTitle = section.find( '.accordion-section-title' ),
    1887                         sectionTitle = section.find( '.customize-section-title h3' ),
    1888                         location = section.find( '.menu-in-location' ),
    1889                         action = sectionTitle.find( '.customize-action' );
    1890 
    1891                     // Update the control title
    1892                     controlTitle.text( menu.name );
    1893                     if ( location.length ) {
    1894                         location.appendTo( controlTitle );
     1889                var section = control.container.closest( '.accordion-section' ),
     1890                    menuId = control.params.menu_id,
     1891                    controlTitle = section.find( '.accordion-section-title' ),
     1892                    sectionTitle = section.find( '.customize-section-title h3' ),
     1893                    location = section.find( '.menu-in-location' ),
     1894                    action = sectionTitle.find( '.customize-action' ),
     1895                    name = displayNavMenuName( menu.name );
     1896
     1897                // Update the control title
     1898                controlTitle.text( name );
     1899                if ( location.length ) {
     1900                    location.appendTo( controlTitle );
     1901                }
     1902
     1903                // Update the section title
     1904                sectionTitle.text( name );
     1905                if ( action.length ) {
     1906                    action.prependTo( sectionTitle );
     1907                }
     1908
     1909                // Update the nav menu name in location selects.
     1910                api.control.each( function( control ) {
     1911                    if ( /^nav_menu_locations\[/.test( control.id ) ) {
     1912                        control.container.find( 'option[value=' + menuId + ']' ).text( name );
    18951913                    }
    1896 
    1897                     // Update the section title
    1898                     sectionTitle.text( menu.name );
    1899                     if ( action.length ) {
    1900                         action.prependTo( sectionTitle );
     1914                } );
     1915
     1916                // Update the nav menu name in all location checkboxes.
     1917                section.find( '.customize-control-checkbox input' ).each( function() {
     1918                    if ( $( this ).prop( 'checked' ) ) {
     1919                        $( '.current-menu-location-name-' + $( this ).data( 'location-id' ) ).text( name );
    19011920                    }
    1902 
    1903                     // Update the nav menu name in location selects.
    1904                     api.control.each( function( control ) {
    1905                         if ( /^nav_menu_locations\[/.test( control.id ) ) {
    1906                             control.container.find( 'option[value=' + menuId + ']' ).text( menu.name );
    1907                         }
    1908                     } );
    1909 
    1910                     // Update the nav menu name in all location checkboxes.
    1911                     section.find( '.customize-control-checkbox input' ).each( function() {
    1912                         if ( $( this ).prop( 'checked' ) ) {
    1913                             $( '.current-menu-location-name-' + $( this ).data( 'location-id' ) ).text( menu.name );
    1914                         }
    1915                     } );
    1916                 }
     1921                } );
    19171922            } );
    19181923        },
    … …  
    21522157        /**
    21532158         * Create the new menu with the name supplied.
    2154          *
    2155          * @returns {boolean}
    21562159         */
    21572160        submit: function() {
    … …  
    21642167                customizeId,
    21652168                placeholderId = api.Menus.generatePlaceholderAutoIncrementId();
     2169
     2170            if ( ! name ) {
     2171                nameInput.addClass( 'invalid' );
     2172                nameInput.focus();
     2173                return;
     2174            }
    21662175
    21672176            customizeId = 'nav_menu[' + String( placeholderId ) + ']';
    … …  
    21902199                    id: customizeId,
    21912200                    panel: 'nav_menus',
    2192                     title: name,
     2201                    title: displayNavMenuName( name ),
    21932202                    customizeAction: api.Menus.data.l10n.customizingMenus,
    21942203                    type: 'nav_menu',
    … …  
    22012210            // Clear name field.
    22022211            nameInput.val( '' );
     2212            nameInput.removeClass( 'invalid' );
    22032213
    22042214            wp.a11y.speak( api.Menus.data.l10n.menuAdded );
    … …  
    22702280
    22712281        _( data.nav_menu_updates ).each(function( update ) {
    2272             var oldCustomizeId, newCustomizeId, oldSetting, newSetting, settingValue, oldSection, newSection;
     2282            var oldCustomizeId, newCustomizeId, customizeId, oldSetting, newSetting, setting, settingValue, oldSection, newSection, wasSaved;
    22732283            if ( 'inserted' === update.status ) {
    22742284                if ( ! update.previous_term_id ) {
    … …  
    22922302                    throw new Error( 'Did not expect setting to be empty (deleted).' );
    22932303                }
    2294                 settingValue = _.clone( settingValue );
     2304                settingValue = $.extend( _.clone( settingValue ), update.saved_value );
    22952305
    22962306                insertedMenuIdMapping[ update.previous_term_id ] = update.term_id;
    … …  
    23502360
    23512361                // @todo Update the Custom Menu selects, ensuring the newly-inserted IDs are used for any that have selected a placeholder menu.
     2362            } else if ( 'updated' === update.status ) {
     2363                customizeId = 'nav_menu[' + String( update.term_id ) + ']';
     2364                if ( ! api.has( customizeId ) ) {
     2365                    throw new Error( 'Expected setting to exist: ' + customizeId );
     2366                }
     2367
     2368                // Make sure the setting gets updated with its sanitized server value (specifically the conflict-resolved name).
     2369                setting = api( customizeId );
     2370                if ( ! _.isEqual( update.saved_value, setting.get() ) ) {
     2371                    wasSaved = api.state( 'saved' ).get();
     2372                    setting.set( update.saved_value );
     2373                    setting._dirty = false;
     2374                    api.state( 'saved' ).set( wasSaved );
     2375                }
    23522376            }
    23532377        } );
    … …  
    24972521    }
    24982522
     2523    /**
     2524     * Apply sanitize_text_field()-like logic to the supplied name, returning a
     2525     * "unnammed" fallback string if the name is then empty.
     2526     *
     2527     * @param {string} name
     2528     * @returns {string}
     2529     */
     2530    function displayNavMenuName( name ) {
     2531        name = $( '<div>' ).text( name ).html(); // Emulate esc_html() which is used in wp-admin/nav-menus.php.
     2532        name = $.trim( name );
     2533        return name || api.Menus.data.l10n.unnamed;
     2534    }
     2535
    24992536})( wp.customize, wp, jQuery );

  • trunk/src/wp-includes/class-wp-customize-nav-menus.php

    r33036 r33071  
    282282            'l10n'                 => array(
    283283                'untitled'          => _x( '(no label)', 'missing menu item navigation label' ),
     284                'unnamed'           => _x( '(unnamed)', 'Missing menu name.' ),
    284285                'custom_label'      => __( 'Custom Link' ),
    285286                /* translators: %s: Current menu location */

  • trunk/src/wp-includes/class-wp-customize-setting.php

    r32893 r33071  
    11831183                    $this->update_status = 'error';
    11841184                    $this->update_error  = new WP_Error( 'nav_menu_setting_failure' );
     1185                    return;
    11851186                }
    11861187
    … …  
    12081209                    $this->update_status = 'error';
    12091210                    $this->update_error  = new WP_Error( 'nav_menu_item_setting_failure' );
     1211                    return;
    12101212                }
    12111213
    … …  
    16121614        $value['auto_add']    = ! empty( $value['auto_add'] );
    16131615
     1616        if ( '' === $value['name'] ) {
     1617            $value['name'] = _x( '(unnamed)', 'Missing menu name.' );
     1618        }
     1619
    16141620        /** This filter is documented in wp-includes/class-wp-customize-setting.php */
    16151621        return apply_filters( "customize_sanitize_{$this->id}", $value, $this );
    … …  
    16701676            // Insert or update menu.
    16711677            $menu_data = wp_array_slice_assoc( $value, array( 'description', 'parent' ) );
    1672             if ( isset( $value['name'] ) ) {
    1673                 $menu_data['menu-name'] = $value['name'];
    1674             }
    1675 
    1676             $r = wp_update_nav_menu_object( $is_placeholder ? 0 : $this->term_id, $menu_data );
     1678            $menu_data['menu-name'] = $value['name'];
     1679
     1680            $menu_id = $is_placeholder ? 0 : $this->term_id;
     1681            $r = wp_update_nav_menu_object( $menu_id, $menu_data );
     1682            $original_name = $menu_data['menu-name'];
     1683            $name_conflict_suffix = 1;
     1684            while ( is_wp_error( $r ) && 'menu_exists' === $r->get_error_code() ) {
     1685                $name_conflict_suffix += 1;
     1686                /* translators: 1: original menu name, 2: duplicate count */
     1687                $menu_data['menu-name'] = sprintf( __( '%1$s (%2$d)' ), $original_name, $name_conflict_suffix );
     1688                $r = wp_update_nav_menu_object( $menu_id, $menu_data );
     1689            }
     1690
    16771691            if ( is_wp_error( $r ) ) {
    16781692                $this->update_status = 'error';
    … …  
    17651779            'error'            => $this->update_error ? $this->update_error->get_error_code() : null,
    17661780            'status'           => $this->update_status,
     1781            'saved_value'      => 'deleted' === $this->update_status ? null : $this->value(),
    17671782        );
    17681783

  • trunk/tests/phpunit/tests/customize/nav-menu-setting.php

    r32806 r33071  
    9595        do_action( 'customize_register', $this->wp_customize );
    9696        $_wp_customize = $this->wp_customize;
    97         unset($_wp_customize->nav_menus);
     97        unset( $_wp_customize->nav_menus );
    9898
    9999        $exception = null;
    … …  
    311311        $this->assertEquals( true, $sanitized['auto_add'] );
    312312        $this->assertEqualSets( array( 'name', 'description', 'parent', 'auto_add' ), array_keys( $sanitized ) );
     313
     314        $value['name'] = '    '; // Blank spaces.
     315        $sanitized = $setting->sanitize( $value );
     316        $this->assertEquals( '(unnamed)', $sanitized['name'] );
    313317    }
    314318
    … …  
    361365        $this->assertArrayHasKey( 'error', $update_result );
    362366        $this->assertArrayHasKey( 'status', $update_result );
     367        $this->assertArrayHasKey( 'saved_value', $update_result );
     368        $this->assertEquals( $new_value, $update_result['saved_value'] );
    363369
    364370        $this->assertEquals( $menu_id, $update_result['term_id'] );
    … …  
    411417        $this->assertArrayHasKey( 'error', $update_result );
    412418        $this->assertArrayHasKey( 'status', $update_result );
     419        $this->assertArrayHasKey( 'saved_value', $update_result );
     420        $this->assertEquals( $setting->value(), $update_result['saved_value'] );
    413421
    414422        $this->assertEquals( $menu->term_id, $update_result['term_id'] );
    … …  
    416424        $this->assertNull( $update_result['error'] );
    417425        $this->assertEquals( 'inserted', $update_result['status'] );
     426    }
     427
     428    /**
     429     * Test saving a new name that conflicts with an existing nav menu's name.
     430     *
     431     * @see WP_Customize_Nav_Menu_Setting::update()
     432     */
     433    function test_save_inserted_conflicted_name() {
     434        do_action( 'customize_register', $this->wp_customize );
     435
     436        $menu_name = 'Foo';
     437        wp_update_nav_menu_object( 0, array( 'menu-name' => $menu_name ) );
     438
     439        $menu_id = -123;
     440        $setting_id = "nav_menu[$menu_id]";
     441        $setting = new WP_Customize_Nav_Menu_Setting( $this->wp_customize, $setting_id );
     442        $this->wp_customize->set_post_value( $setting->id, array( 'name' => $menu_name ) );
     443        $setting->save();
     444
     445        $expected_resolved_menu_name = "$menu_name (2)";
     446        $new_menu = wp_get_nav_menu_object( $setting->term_id );
     447        $this->assertEquals( $expected_resolved_menu_name, $new_menu->name );
     448
     449        $save_response = apply_filters( 'customize_save_response', array() );
     450        $this->assertEquals( $expected_resolved_menu_name, $save_response['nav_menu_updates'][0]['saved_value']['name'] );
    418451    }
    419452
    … …  
    449482        $this->assertArrayHasKey( 'error', $update_result );
    450483        $this->assertArrayHasKey( 'status', $update_result );
     484        $this->assertArrayHasKey( 'saved_value', $update_result );
     485        $this->assertNull( $update_result['saved_value'] );
    451486
    452487        $this->assertEquals( $menu_id, $update_result['term_id'] );
Note: See TracChangeset for help on using the changeset viewer.