Menus: Avoid a PHP warning in add_submenu_page() when the same value is passed for both $parent_slug and $menu_slug parameters.

SergeyBiryukov · SergeyBiryukov · commit 1775d3fe55dd · 2019-12-09T21:04:07.000Z
Props welcher, ispreview, ayeshrajans, NextScripts, adamsilverstein, garrett-eclipse, 123host. Merges [46868] to the 5.3 branch. Fixes #48599. git-svn-id: https://develop.svn.wordpress.org/branches/5.3@46869 602fd350-edb4-49c9-b593-d223f7449a82
diff --git a/src/wp-admin/includes/plugin.php b/src/wp-admin/includes/plugin.php
@@ -1388,8 +1388,9 @@ function add_submenu_page( $parent_slug, $page_title, $menu_title, $capability,
 
 		$submenu[ $parent_slug ][] = $new_sub_menu;
 	} else {
-		// If position is equal or higher than the number of items in the array, append the submenu.
-		if ( $position >= count( $submenu[ $parent_slug ] ) ) {
+		// 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;
 		} else {
 			// Test for a negative position.
diff --git a/tests/phpunit/tests/admin/includesPlugin.php b/tests/phpunit/tests/admin/includesPlugin.php
@@ -259,6 +259,67 @@ function data_submenu_priority() {
 		);
 	}
 
+	/**
+	 * Test that when a submenu has the same slug as a parent item, that it's just appended and ignores the priority.
+	 *
+	 * @ticket 48599
+	 */
+	function test_priority_when_parent_slug_child_slug_are_the_same() {
+		global $submenu, $menu;
+
+		// Reset menus.
+		$submenu      = array();
+		$menu         = array();
+		$current_user = get_current_user_id();
+		$admin_user   = self::factory()->user->create( array( 'role' => 'administrator' ) );
+		wp_set_current_user( $admin_user );
+		set_current_screen( 'dashboard' );
+
+		// 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', 'main_slug', 'submenu_callback_1', 1 );
+		add_submenu_page( 'main_slug', 'SubMenu 2', 'SubMenu 2', 'manage_options', 'submenu_page2', 'submenu_callback_2', 2 );
+		add_submenu_page( 'main_slug', 'SubMenu 3', 'SubMenu 3', 'manage_options', 'submenu_page3', 'submenu_callback_3', 3 );
+
+		// Clean up the temporary user.
+		wp_set_current_user( $current_user );
+		wp_delete_user( $admin_user );
+
+		// Verify the menu was inserted at the expected position.
+		$this->assertSame( 'main_slug', $submenu['main_slug'][0][2] );
+		$this->assertSame( 'submenu_page2', $submenu['main_slug'][1][2] );
+		$this->assertSame( 'submenu_page3', $submenu['main_slug'][2][2] );
+	}
+
+	/**
+	 * Passing a string as priority will fail.
+	 *
+	 * @ticket 48599
+	 */
+	function test_passing_string_as_priority_fires_doing_it_wrong() {
+		$this->setExpectedIncorrectUsage( 'add_submenu_page' );
+		global $submenu, $menu;
+
+		// Reset menus.
+		$submenu      = array();
+		$menu         = array();
+		$current_user = get_current_user_id();
+		$admin_user   = self::factory()->user->create( array( 'role' => 'administrator' ) );
+		wp_set_current_user( $admin_user );
+		set_current_screen( 'dashboard' );
+
+		// 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' );
+
+		// Clean up the temporary user.
+		wp_set_current_user( $current_user );
+		wp_delete_user( $admin_user );
+
+		// Verify the menu was inserted at the expected position.
+		$this->assertSame( 'submenu_page_1', $submenu['main_slug'][1][2] );
+	}
+
 	function test_is_plugin_active_true() {
 		activate_plugin( 'hello.php' );
 		$test = is_plugin_active( 'hello.php' );
