From a21c6e76d63ad97813de8f8f8e7855e586bc79bb Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Fri, 3 Jul 2015 20:46:48 +0000 Subject: [PATCH] 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. git-svn-id: https://develop.svn.wordpress.org/trunk@33071 602fd350-edb4-49c9-b593-d223f7449a82 --- src/wp-admin/css/customize-nav-menus.css | 4 +- src/wp-admin/js/customize-nav-menus.js | 131 +++++++++++------- .../class-wp-customize-nav-menus.php | 1 + .../class-wp-customize-setting.php | 21 ++- .../tests/customize/nav-menu-setting.php | 37 ++++- 5 files changed, 142 insertions(+), 52 deletions(-) diff --git a/src/wp-admin/css/customize-nav-menus.css b/src/wp-admin/css/customize-nav-menus.css index aa0deed41a..fc14598fd7 100644 --- a/src/wp-admin/css/customize-nav-menus.css +++ b/src/wp-admin/css/customize-nav-menus.css @@ -654,7 +654,9 @@ button.not-a-button { } #custom-menu-item-name.invalid, -#custom-menu-item-url.invalid { +#custom-menu-item-url.invalid, +.menu-name-field.invalid, +.menu-name-field.invalid:focus { border: 1px solid #f00; } diff --git a/src/wp-admin/js/customize-nav-menus.js b/src/wp-admin/js/customize-nav-menus.js index 278951df4b..42e362c46b 100644 --- a/src/wp-admin/js/customize-nav-menus.js +++ b/src/wp-admin/js/customize-nav-menus.js @@ -833,6 +833,7 @@ button.removeClass( 'open' ); button.attr( 'aria-expanded', 'false' ); content.slideUp( 'fast' ); + content.find( '.menu-name-field' ).removeClass( 'invalid' ); } } }); @@ -869,7 +870,7 @@ return; } menuId = matches[1]; - option = new Option( setting().name, menuId ); + option = new Option( displayNavMenuName( setting().name ), menuId ); control.container.find( 'select' ).append( option ); }); api.bind( 'remove', function( setting ) { @@ -895,7 +896,7 @@ } control.container.find( 'option[value=' + menuId + ']' ).remove(); } else { - control.container.find( 'option[value=' + menuId + ']' ).text( setting().name ); + control.container.find( 'option[value=' + menuId + ']' ).text( displayNavMenuName( setting().name ) ); } }); } @@ -1605,7 +1606,9 @@ */ ready: function() { var control = this, - menuId = control.params.menu_id; + menuId = control.params.menu_id, + menu = control.setting(), + name; if ( 'undefined' === typeof this.params.menu_id ) { throw new Error( 'params.menu_id was not defined' ); @@ -1636,17 +1639,19 @@ this._setupTitle(); // Add menu to Custom Menu widgets. - if ( control.setting() ) { + if ( menu ) { + name = displayNavMenuName( menu.name ); + api.control.each( function( widgetControl ) { if ( ! widgetControl.extended( api.controlConstructor.widget_form ) || 'nav_menu' !== widgetControl.params.widget_id_base ) { return; } var select = widgetControl.container.find( 'select' ); if ( select.find( 'option[value=' + String( menuId ) + ']' ).length === 0 ) { - select.append( new Option( control.setting().name, menuId ) ); + select.append( new Option( name, menuId ) ); } } ); - $( '#available-widgets-list .widget-inside:has(input.id_base[value=nav_menu]) select:first' ).append( new Option( control.setting().name, menuId ) ); + $( '#available-widgets-list .widget-inside:has(input.id_base[value=nav_menu]) select:first' ).append( new Option( name, menuId ) ); } }, @@ -1677,18 +1682,20 @@ }); control.setting.bind( function( to ) { + var name; if ( false === to ) { control._handleDeletion(); } else { // Update names in the Custom Menu widgets. + name = displayNavMenuName( to.name ); api.control.each( function( widgetControl ) { if ( ! widgetControl.extended( api.controlConstructor.widget_form ) || 'nav_menu' !== widgetControl.params.widget_id_base ) { return; } var select = widgetControl.container.find( 'select' ); - select.find( 'option[value=' + String( menuId ) + ']' ).text( to.name ); + select.find( 'option[value=' + String( menuId ) + ']' ).text( name ); }); - $( '#available-widgets-list .widget-inside:has(input.id_base[value=nav_menu]) select:first option[value=' + String( menuId ) + ']' ).text( to.name ); + $( '#available-widgets-list .widget-inside:has(input.id_base[value=nav_menu]) select:first option[value=' + String( menuId ) + ']' ).text( name ); } } ); @@ -1847,7 +1854,7 @@ if ( ! selectedMenuId || ! menuSetting || ! menuSetting() ) { container.find( '.theme-location-set' ).hide(); } else { - container.find( '.theme-location-set' ).show().find( 'span' ).text( menuSetting().name ); + container.find( '.theme-location-set' ).show().find( 'span' ).text( displayNavMenuName( menuSetting().name ) ); } }; @@ -1879,41 +1886,39 @@ return; } - // Empty names are not allowed (will not be saved), don't update to one. - if ( menu.name ) { - var section = control.container.closest( '.accordion-section' ), - menuId = control.params.menu_id, - controlTitle = section.find( '.accordion-section-title' ), - sectionTitle = section.find( '.customize-section-title h3' ), - location = section.find( '.menu-in-location' ), - action = sectionTitle.find( '.customize-action' ); + var section = control.container.closest( '.accordion-section' ), + menuId = control.params.menu_id, + controlTitle = section.find( '.accordion-section-title' ), + sectionTitle = section.find( '.customize-section-title h3' ), + location = section.find( '.menu-in-location' ), + action = sectionTitle.find( '.customize-action' ), + name = displayNavMenuName( menu.name ); - // Update the control title - controlTitle.text( menu.name ); - if ( location.length ) { - location.appendTo( controlTitle ); - } - - // Update the section title - sectionTitle.text( menu.name ); - if ( action.length ) { - action.prependTo( sectionTitle ); - } - - // Update the nav menu name in location selects. - api.control.each( function( control ) { - if ( /^nav_menu_locations\[/.test( control.id ) ) { - control.container.find( 'option[value=' + menuId + ']' ).text( menu.name ); - } - } ); - - // Update the nav menu name in all location checkboxes. - section.find( '.customize-control-checkbox input' ).each( function() { - if ( $( this ).prop( 'checked' ) ) { - $( '.current-menu-location-name-' + $( this ).data( 'location-id' ) ).text( menu.name ); - } - } ); + // Update the control title + controlTitle.text( name ); + if ( location.length ) { + location.appendTo( controlTitle ); } + + // Update the section title + sectionTitle.text( name ); + if ( action.length ) { + action.prependTo( sectionTitle ); + } + + // Update the nav menu name in location selects. + api.control.each( function( control ) { + if ( /^nav_menu_locations\[/.test( control.id ) ) { + control.container.find( 'option[value=' + menuId + ']' ).text( name ); + } + } ); + + // Update the nav menu name in all location checkboxes. + section.find( '.customize-control-checkbox input' ).each( function() { + if ( $( this ).prop( 'checked' ) ) { + $( '.current-menu-location-name-' + $( this ).data( 'location-id' ) ).text( name ); + } + } ); } ); }, @@ -2151,8 +2156,6 @@ /** * Create the new menu with the name supplied. - * - * @returns {boolean} */ submit: function() { @@ -2164,6 +2167,12 @@ customizeId, placeholderId = api.Menus.generatePlaceholderAutoIncrementId(); + if ( ! name ) { + nameInput.addClass( 'invalid' ); + nameInput.focus(); + return; + } + customizeId = 'nav_menu[' + String( placeholderId ) + ']'; // Register the menu control setting. @@ -2189,7 +2198,7 @@ params: { id: customizeId, panel: 'nav_menus', - title: name, + title: displayNavMenuName( name ), customizeAction: api.Menus.data.l10n.customizingMenus, type: 'nav_menu', priority: 10, @@ -2200,6 +2209,7 @@ // Clear name field. nameInput.val( '' ); + nameInput.removeClass( 'invalid' ); wp.a11y.speak( api.Menus.data.l10n.menuAdded ); @@ -2269,7 +2279,7 @@ var insertedMenuIdMapping = {}; _( data.nav_menu_updates ).each(function( update ) { - var oldCustomizeId, newCustomizeId, oldSetting, newSetting, settingValue, oldSection, newSection; + var oldCustomizeId, newCustomizeId, customizeId, oldSetting, newSetting, setting, settingValue, oldSection, newSection, wasSaved; if ( 'inserted' === update.status ) { if ( ! update.previous_term_id ) { throw new Error( 'Expected previous_term_id' ); @@ -2291,7 +2301,7 @@ if ( ! settingValue ) { throw new Error( 'Did not expect setting to be empty (deleted).' ); } - settingValue = _.clone( settingValue ); + settingValue = $.extend( _.clone( settingValue ), update.saved_value ); insertedMenuIdMapping[ update.previous_term_id ] = update.term_id; newCustomizeId = 'nav_menu[' + String( update.term_id ) + ']'; @@ -2349,6 +2359,20 @@ } // @todo Update the Custom Menu selects, ensuring the newly-inserted IDs are used for any that have selected a placeholder menu. + } else if ( 'updated' === update.status ) { + customizeId = 'nav_menu[' + String( update.term_id ) + ']'; + if ( ! api.has( customizeId ) ) { + throw new Error( 'Expected setting to exist: ' + customizeId ); + } + + // Make sure the setting gets updated with its sanitized server value (specifically the conflict-resolved name). + setting = api( customizeId ); + if ( ! _.isEqual( update.saved_value, setting.get() ) ) { + wasSaved = api.state( 'saved' ).get(); + setting.set( update.saved_value ); + setting._dirty = false; + api.state( 'saved' ).set( wasSaved ); + } } } ); @@ -2496,4 +2520,17 @@ return 'nav_menu_item[' + menuItemId + ']'; } + /** + * Apply sanitize_text_field()-like logic to the supplied name, returning a + * "unnammed" fallback string if the name is then empty. + * + * @param {string} name + * @returns {string} + */ + function displayNavMenuName( name ) { + name = $( '
' ).text( name ).html(); // Emulate esc_html() which is used in wp-admin/nav-menus.php. + name = $.trim( name ); + return name || api.Menus.data.l10n.unnamed; + } + })( wp.customize, wp, jQuery ); diff --git a/src/wp-includes/class-wp-customize-nav-menus.php b/src/wp-includes/class-wp-customize-nav-menus.php index 16531c1574..fa496fda64 100644 --- a/src/wp-includes/class-wp-customize-nav-menus.php +++ b/src/wp-includes/class-wp-customize-nav-menus.php @@ -281,6 +281,7 @@ final class WP_Customize_Nav_Menus { 'itemTypes' => $this->available_item_types(), 'l10n' => array( 'untitled' => _x( '(no label)', 'missing menu item navigation label' ), + 'unnamed' => _x( '(unnamed)', 'Missing menu name.' ), 'custom_label' => __( 'Custom Link' ), /* translators: %s: Current menu location */ 'menuLocation' => __( '(Currently set to: %s)' ), diff --git a/src/wp-includes/class-wp-customize-setting.php b/src/wp-includes/class-wp-customize-setting.php index 5dd5293d31..ce0f711694 100644 --- a/src/wp-includes/class-wp-customize-setting.php +++ b/src/wp-includes/class-wp-customize-setting.php @@ -1182,6 +1182,7 @@ class WP_Customize_Nav_Menu_Item_Setting extends WP_Customize_Setting { if ( false === $nav_menu_setting->save() ) { $this->update_status = 'error'; $this->update_error = new WP_Error( 'nav_menu_setting_failure' ); + return; } if ( $nav_menu_setting->previous_term_id !== intval( $value['nav_menu_term_id'] ) ) { @@ -1207,6 +1208,7 @@ class WP_Customize_Nav_Menu_Item_Setting extends WP_Customize_Setting { if ( false === $parent_nav_menu_item_setting->save() ) { $this->update_status = 'error'; $this->update_error = new WP_Error( 'nav_menu_item_setting_failure' ); + return; } if ( $parent_nav_menu_item_setting->previous_post_id !== intval( $value['menu_item_parent'] ) ) { @@ -1611,6 +1613,10 @@ class WP_Customize_Nav_Menu_Setting extends WP_Customize_Setting { $value['parent'] = max( 0, intval( $value['parent'] ) ); $value['auto_add'] = ! empty( $value['auto_add'] ); + if ( '' === $value['name'] ) { + $value['name'] = _x( '(unnamed)', 'Missing menu name.' ); + } + /** This filter is documented in wp-includes/class-wp-customize-setting.php */ return apply_filters( "customize_sanitize_{$this->id}", $value, $this ); } @@ -1669,11 +1675,19 @@ class WP_Customize_Nav_Menu_Setting extends WP_Customize_Setting { } else { // Insert or update menu. $menu_data = wp_array_slice_assoc( $value, array( 'description', 'parent' ) ); - if ( isset( $value['name'] ) ) { - $menu_data['menu-name'] = $value['name']; + $menu_data['menu-name'] = $value['name']; + + $menu_id = $is_placeholder ? 0 : $this->term_id; + $r = wp_update_nav_menu_object( $menu_id, $menu_data ); + $original_name = $menu_data['menu-name']; + $name_conflict_suffix = 1; + while ( is_wp_error( $r ) && 'menu_exists' === $r->get_error_code() ) { + $name_conflict_suffix += 1; + /* translators: 1: original menu name, 2: duplicate count */ + $menu_data['menu-name'] = sprintf( __( '%1$s (%2$d)' ), $original_name, $name_conflict_suffix ); + $r = wp_update_nav_menu_object( $menu_id, $menu_data ); } - $r = wp_update_nav_menu_object( $is_placeholder ? 0 : $this->term_id, $menu_data ); if ( is_wp_error( $r ) ) { $this->update_status = 'error'; $this->update_error = $r; @@ -1764,6 +1778,7 @@ class WP_Customize_Nav_Menu_Setting extends WP_Customize_Setting { 'previous_term_id' => $this->previous_term_id, 'error' => $this->update_error ? $this->update_error->get_error_code() : null, 'status' => $this->update_status, + 'saved_value' => 'deleted' === $this->update_status ? null : $this->value(), ); return $data; diff --git a/tests/phpunit/tests/customize/nav-menu-setting.php b/tests/phpunit/tests/customize/nav-menu-setting.php index d2f5d9cbc9..5d99fd693f 100644 --- a/tests/phpunit/tests/customize/nav-menu-setting.php +++ b/tests/phpunit/tests/customize/nav-menu-setting.php @@ -94,7 +94,7 @@ class Test_WP_Customize_Nav_Menu_Setting extends WP_UnitTestCase { function test_construct_empty_menus() { do_action( 'customize_register', $this->wp_customize ); $_wp_customize = $this->wp_customize; - unset($_wp_customize->nav_menus); + unset( $_wp_customize->nav_menus ); $exception = null; try { @@ -310,6 +310,10 @@ class Test_WP_Customize_Nav_Menu_Setting extends WP_UnitTestCase { $this->assertEquals( 0, $sanitized['parent'] ); $this->assertEquals( true, $sanitized['auto_add'] ); $this->assertEqualSets( array( 'name', 'description', 'parent', 'auto_add' ), array_keys( $sanitized ) ); + + $value['name'] = ' '; // Blank spaces. + $sanitized = $setting->sanitize( $value ); + $this->assertEquals( '(unnamed)', $sanitized['name'] ); } /** @@ -360,6 +364,8 @@ class Test_WP_Customize_Nav_Menu_Setting extends WP_UnitTestCase { $this->assertArrayHasKey( 'previous_term_id', $update_result ); $this->assertArrayHasKey( 'error', $update_result ); $this->assertArrayHasKey( 'status', $update_result ); + $this->assertArrayHasKey( 'saved_value', $update_result ); + $this->assertEquals( $new_value, $update_result['saved_value'] ); $this->assertEquals( $menu_id, $update_result['term_id'] ); $this->assertNull( $update_result['previous_term_id'] ); @@ -410,6 +416,8 @@ class Test_WP_Customize_Nav_Menu_Setting extends WP_UnitTestCase { $this->assertArrayHasKey( 'previous_term_id', $update_result ); $this->assertArrayHasKey( 'error', $update_result ); $this->assertArrayHasKey( 'status', $update_result ); + $this->assertArrayHasKey( 'saved_value', $update_result ); + $this->assertEquals( $setting->value(), $update_result['saved_value'] ); $this->assertEquals( $menu->term_id, $update_result['term_id'] ); $this->assertEquals( $menu_id, $update_result['previous_term_id'] ); @@ -417,6 +425,31 @@ class Test_WP_Customize_Nav_Menu_Setting extends WP_UnitTestCase { $this->assertEquals( 'inserted', $update_result['status'] ); } + /** + * Test saving a new name that conflicts with an existing nav menu's name. + * + * @see WP_Customize_Nav_Menu_Setting::update() + */ + function test_save_inserted_conflicted_name() { + do_action( 'customize_register', $this->wp_customize ); + + $menu_name = 'Foo'; + wp_update_nav_menu_object( 0, array( 'menu-name' => $menu_name ) ); + + $menu_id = -123; + $setting_id = "nav_menu[$menu_id]"; + $setting = new WP_Customize_Nav_Menu_Setting( $this->wp_customize, $setting_id ); + $this->wp_customize->set_post_value( $setting->id, array( 'name' => $menu_name ) ); + $setting->save(); + + $expected_resolved_menu_name = "$menu_name (2)"; + $new_menu = wp_get_nav_menu_object( $setting->term_id ); + $this->assertEquals( $expected_resolved_menu_name, $new_menu->name ); + + $save_response = apply_filters( 'customize_save_response', array() ); + $this->assertEquals( $expected_resolved_menu_name, $save_response['nav_menu_updates'][0]['saved_value']['name'] ); + } + /** * Test protected update() method via the save() method, for deleted menu. * @@ -448,6 +481,8 @@ class Test_WP_Customize_Nav_Menu_Setting extends WP_UnitTestCase { $this->assertArrayHasKey( 'previous_term_id', $update_result ); $this->assertArrayHasKey( 'error', $update_result ); $this->assertArrayHasKey( 'status', $update_result ); + $this->assertArrayHasKey( 'saved_value', $update_result ); + $this->assertNull( $update_result['saved_value'] ); $this->assertEquals( $menu_id, $update_result['term_id'] ); $this->assertNull( $update_result['previous_term_id'] );