From 4f290ecd64e0e30bb52bbaf63446254e68e577f3 Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Fri, 8 Apr 2022 06:15:02 +0000 Subject: [PATCH] Administration: Allow floats for menu positions. Permit plugin authors to pass the menu position as a float in `add_menu_page()` and `add_submenu_page()`. This allows for a common practice within major plugins to avoid menu collisions by passing a float. Follow up to [52569]. Props justinbusa, dd32, welcher, SergeyBiryukov, kirtan95, audrasjb, Cybr, chaion07, costdev, peterwilsoncc. See #40927. git-svn-id: https://develop.svn.wordpress.org/trunk@53104 602fd350-edb4-49c9-b593-d223f7449a82 --- src/wp-admin/includes/plugin.php | 145 +++++++++---------- tests/phpunit/tests/admin/includesPlugin.php | 25 +++- 2 files changed, 87 insertions(+), 83 deletions(-) diff --git a/src/wp-admin/includes/plugin.php b/src/wp-admin/includes/plugin.php index 22c5fb9b4d..3712d22679 100644 --- a/src/wp-admin/includes/plugin.php +++ b/src/wp-admin/includes/plugin.php @@ -1284,20 +1284,20 @@ function uninstall_plugin( $plugin ) { * @global array $_registered_pages * @global array $_parent_pages * - * @param string $page_title The text to be displayed in the title tags of the page when the menu is selected. - * @param string $menu_title The text to be used for the menu. - * @param string $capability The capability required for this menu to be displayed to the user. - * @param string $menu_slug The slug name to refer to this menu by. Should be unique for this menu page and only - * include lowercase alphanumeric, dashes, and underscores characters to be compatible - * with sanitize_key(). - * @param callable $function Optional. The function to be called to output the content for this page. - * @param string $icon_url Optional. The URL to the icon to be used for this menu. - * * Pass a base64-encoded SVG using a data URI, which will be colored to match - * the color scheme. This should begin with 'data:image/svg+xml;base64,'. - * * Pass the name of a Dashicons helper class to use a font icon, - * e.g. 'dashicons-chart-pie'. - * * Pass 'none' to leave div.wp-menu-image empty so an icon can be added via CSS. - * @param int $position Optional. The position in the menu order this item should appear. + * @param string $page_title The text to be displayed in the title tags of the page when the menu is selected. + * @param string $menu_title The text to be used for the menu. + * @param string $capability The capability required for this menu to be displayed to the user. + * @param string $menu_slug The slug name to refer to this menu by. Should be unique for this menu page and only + * include lowercase alphanumeric, dashes, and underscores characters to be compatible + * with sanitize_key(). + * @param callable $function Optional. The function to be called to output the content for this page. + * @param string $icon_url Optional. The URL to the icon to be used for this menu. + * * Pass a base64-encoded SVG using a data URI, which will be colored to match + * the color scheme. This should begin with 'data:image/svg+xml;base64,'. + * * Pass the name of a Dashicons helper class to use a font icon, + * e.g. 'dashicons-chart-pie'. + * * Pass 'none' to leave div.wp-menu-image empty so an icon can be added via CSS. + * @param int|float $position Optional. The position in the menu order this item should appear. * @return string The resulting page's hook_suffix. */ function add_menu_page( $page_title, $menu_title, $capability, $menu_slug, $function = '', $icon_url = '', $position = null ) { @@ -1323,27 +1323,22 @@ function add_menu_page( $page_title, $menu_title, $capability, $menu_slug, $func $new_menu = array( $menu_title, $capability, $menu_slug, $page_title, 'menu-top ' . $icon_class . $hookname, $hookname, $icon_url ); - if ( null === $position ) { + if ( null === $position || ! is_numeric( $position ) ) { $menu[] = $new_menu; - } elseif ( isset( $menu[ "$position" ] ) ) { - $position = $position + substr( base_convert( md5( $menu_slug . $menu_title ), 16, 10 ), -5 ) * 0.00001; - $menu[ "$position" ] = $new_menu; + } elseif ( isset( $menu[ (string) $position ] ) ) { + $collision_avoider = base_convert( substr( md5( $menu_slug . $menu_title ), -4 ), 16, 10 ) * 0.00001; + $position = (string) ( $position + $collision_avoider ); + $menu[ $position ] = $new_menu; } else { - if ( ! is_int( $position ) ) { - _doing_it_wrong( - __FUNCTION__, - sprintf( - /* translators: %s: add_menu_page() */ - __( 'The seventh parameter passed to %s should be an integer representing menu position.' ), - 'add_menu_page()' - ), - '6.0.0' - ); - // If the position is not a string (i.e. float), convert it to string. - if ( ! is_string( $position ) ) { - $position = (string) $position; - } - } + /* + * Cast menu position to a string. + * + * This allows for floats to be passed as the position. PHP will normally cast a float to an + * integer value, this ensures the float retains its mantissa (positive fractional part). + * + * A string containing an integer value, eg "10", is treated as a numeric index. + */ + $position = (string) $position; $menu[ $position ] = $new_menu; } @@ -1374,17 +1369,17 @@ function add_menu_page( $page_title, $menu_title, $capability, $menu_slug, $func * @global array $_registered_pages * @global array $_parent_pages * - * @param string $parent_slug The slug name for the parent menu (or the file name of a standard - * WordPress admin page). - * @param string $page_title The text to be displayed in the title tags of the page when the menu - * is selected. - * @param string $menu_title The text to be used for the menu. - * @param string $capability The capability required for this menu to be displayed to the user. - * @param string $menu_slug The slug name to refer to this menu by. Should be unique for this menu - * and only include lowercase alphanumeric, dashes, and underscores characters - * to be compatible with sanitize_key(). - * @param callable $function Optional. The function to be called to output the content for this page. - * @param int $position Optional. The position in the menu order this item should appear. + * @param string $parent_slug The slug name for the parent menu (or the file name of a standard + * WordPress admin page). + * @param string $page_title The text to be displayed in the title tags of the page when the menu + * is selected. + * @param string $menu_title The text to be used for the menu. + * @param string $capability The capability required for this menu to be displayed to the user. + * @param string $menu_slug The slug name to refer to this menu by. Should be unique for this menu + * and only include lowercase alphanumeric, dashes, and underscores characters + * to be compatible with sanitize_key(). + * @param callable $function Optional. The function to be called to output the content for this page. + * @param int|float $position Optional. The position in the menu order this item should appear. * @return string|false The resulting page's hook_suffix, or false if the user does not have the capability required. */ function add_submenu_page( $parent_slug, $page_title, $menu_title, $capability, $menu_slug, $function = '', $position = null ) { @@ -1418,43 +1413,43 @@ function add_submenu_page( $parent_slug, $page_title, $menu_title, $capability, } $new_sub_menu = array( $menu_title, $capability, $menu_slug, $page_title ); - if ( ! is_int( $position ) ) { - if ( null !== $position ) { - _doing_it_wrong( - __FUNCTION__, - sprintf( - /* translators: %s: add_submenu_page() */ - __( 'The seventh parameter passed to %s should be an integer representing menu position.' ), - 'add_submenu_page()' - ), - '5.3.0' - ); - } + if ( null !== $position && ! is_numeric( $position ) ) { + _doing_it_wrong( + __FUNCTION__, + sprintf( + /* translators: %s: add_submenu_page() */ + __( 'The seventh parameter passed to %s should be an integer representing menu position.' ), + 'add_submenu_page()' + ), + '5.3.0' + ); + $position = null; + } + + if ( + null === $position || + ( ! isset( $submenu[ $parent_slug ] ) || $position >= count( $submenu[ $parent_slug ] ) ) + ) { $submenu[ $parent_slug ][] = $new_sub_menu; } else { - // Append the submenu if the parent item is not present in the submenu, - // or if position is equal or higher than the number of items in the array. - if ( ! isset( $submenu[ $parent_slug ] ) || $position >= count( $submenu[ $parent_slug ] ) ) { - $submenu[ $parent_slug ][] = $new_sub_menu; + // Test for a negative position. + $position = max( $position, 0 ); + if ( 0 === $position ) { + // For negative or `0` positions, prepend the submenu. + array_unshift( $submenu[ $parent_slug ], $new_sub_menu ); } else { - // Test for a negative position. - $position = max( $position, 0 ); - if ( 0 === $position ) { - // For negative or `0` positions, prepend the submenu. - array_unshift( $submenu[ $parent_slug ], $new_sub_menu ); - } else { - // Grab all of the items before the insertion point. - $before_items = array_slice( $submenu[ $parent_slug ], 0, $position, true ); - // Grab all of the items after the insertion point. - $after_items = array_slice( $submenu[ $parent_slug ], $position, null, true ); - // Add the new item. - $before_items[] = $new_sub_menu; - // Merge the items. - $submenu[ $parent_slug ] = array_merge( $before_items, $after_items ); - } + // Grab all of the items before the insertion point. + $before_items = array_slice( $submenu[ $parent_slug ], 0, $position, true ); + // Grab all of the items after the insertion point. + $after_items = array_slice( $submenu[ $parent_slug ], $position, null, true ); + // Add the new item. + $before_items[] = $new_sub_menu; + // Merge the items. + $submenu[ $parent_slug ] = array_merge( $before_items, $after_items ); } } + // Sort the parent array. ksort( $submenu[ $parent_slug ] ); diff --git a/tests/phpunit/tests/admin/includesPlugin.php b/tests/phpunit/tests/admin/includesPlugin.php index 18e08ec03c..8dd009ddc3 100644 --- a/tests/phpunit/tests/admin/includesPlugin.php +++ b/tests/phpunit/tests/admin/includesPlugin.php @@ -256,8 +256,16 @@ class Tests_Admin_IncludesPlugin extends WP_UnitTestCase { array( 0, 0 ), // Insert at the beginning of the menu if 0 is passed. array( -1, 0 ), // Negative numbers are treated the same as passing 0. array( -7, 0 ), // Negative numbers are treated the same as passing 0. + array( '-7', 0 ), // Negative numbers are treated the same as passing 0. array( 1, 1 ), // Insert as the second item. + array( '1', 1 ), // Insert as the second item. + array( 1.5, 1 ), // Insert as the second item. + array( '1.5', 1 ), // Insert as the second item. array( 3, 3 ), // Insert as the 4th item. + array( '3', 3 ), // Insert as the 4th item. + array( '3e0', 3 ), // Insert as the 4th item. + array( 3.5, 3 ), // Insert as the 4th item. + array( '3.5', 3 ), // Insert as the 4th item. array( $menu_count, $menu_count ), // Numbers equal to the number of items are added at the end. array( 123456, $menu_count ), // Numbers higher than the number of items are added at the end. ); @@ -314,7 +322,7 @@ class Tests_Admin_IncludesPlugin extends WP_UnitTestCase { // Setup a menu with some items. add_menu_page( 'Main Menu', 'Main Menu', 'manage_options', 'main_slug', 'main_page_callback' ); - add_submenu_page( 'main_slug', 'SubMenu 1', 'SubMenu 1', 'manage_options', 'submenu_page_1', 'submenu_callback_1', '2' ); + add_submenu_page( 'main_slug', 'SubMenu 1', 'SubMenu 1', 'manage_options', 'submenu_page_1', 'submenu_callback_1', 'First' ); // Clean up the temporary user. wp_set_current_user( $current_user ); @@ -329,8 +337,7 @@ class Tests_Admin_IncludesPlugin extends WP_UnitTestCase { * * @ticket 54798 */ - public function test_passing_string_as_position_fires_doing_it_wrong_menu() { - $this->setExpectedIncorrectUsage( 'add_menu_page' ); + public function test_passing_float_as_position_does_not_override_int() { global $submenu, $menu; // Reset menus. @@ -342,17 +349,19 @@ class Tests_Admin_IncludesPlugin extends WP_UnitTestCase { set_current_screen( 'dashboard' ); // Setup a menu with some items. - add_menu_page( 'Main Menu', 'Main Menu', 'manage_options', 'main_slug', 'main_page_callback', 'icon_url', '1' ); - add_menu_page( 'Main Menu 1', 'Main Menu 1', 'manage_options', 'main1_slug', 'main1_page_callback', 'icon_url1', 1.5 ); + add_menu_page( 'Main Menu 1', 'Main Menu 1', 'manage_options', 'main_slug_1', 'main_page_callback_1', 'icon_url_1', 1 ); + add_menu_page( 'Main Menu 2', 'Main Menu 2', 'manage_options', 'main_slug_2', 'main_page_callback_2', 'icon_url_2', 2 ); + add_menu_page( 'Main Menu 1.5', 'Main Menu 1.5', 'manage_options', 'main_slug_15', 'main_page_callback_15', 'icon_url_15', 1.5 ); // Clean up the temporary user. wp_set_current_user( $current_user ); wp_delete_user( $admin_user ); - // Verify the menu was inserted. - $this->assertSame( 'main_slug', $menu[1][2] ); + // Verify the menus were inserted. + $this->assertSame( 'main_slug_1', $menu[1][2] ); + $this->assertSame( 'main_slug_2', $menu[2][2] ); // Verify the menu was inserted correctly on passing float as position. - $this->assertSame( 'main1_slug', $menu['1.5'][2] ); + $this->assertSame( 'main_slug_15', $menu['1.5'][2] ); } public function test_is_plugin_active_true() {