Make WordPress Core

Opened 17 years ago

Closed 16 years ago

Last modified 16 years ago

#5001 closed defect (bug) (invalid)

Admin menu generation sometimes shows wrong page as current

Reported by: jhodgdon's profile jhodgdon Owned by:
Milestone: Priority: normal
Severity: normal Version: 2.3
Component: Administration Keywords: admin, menu
Focuses: Cc:

Description

When generating the menus at the top of admin pages, you can create a situation where the menu system gets confused about which page is the current page.

The issue comes up when the plugin has both sub-menu pages for built-in WordPress menus and its own main menu section, and you add the sub-menu pages first in the plugin code.

The attached plugin (courtesy of Mike Walsh) illustrates the problem:

(1) Install and activate the plugin.

(2) Click on the "Menu Tester" link in the top menu line (the URL is blogurl/wp-admin/admin.php?page=menutester.php ).

(3) You will see the correct screen (which says Menu Tester Tab 1), but the menu system will show that you are on Options / Menu Tester instead of Menu Tester / Menu Tester. And there is no way to click on the other sub-menus of your main menu section.

Here is why this is happening (hope this makes sense -- have to read the code!)

Basically, when the plugin first calls add_options_page to add the Options submenu, the add_submenu_page function adds a new item to the global $menu array, and then ends up calling get_plugin_page_hookname, which calls get_admin_page_parent() (adll these functions are currently located in wp-admin/includes/plugin.php as of [6124]).

At this point, the global $pagenow variable is set to 'admin.php', and the $plugin_page variable is set to 'menutester.php'.

So, the get_admin_page_parent function goes through the $menu array, and searches for a menu item whose plugin file is set to 'menutester.php'. It finds the newly-added options page, and sets the global $parent_file variable to the parent file, i.e. options-general.php.

The issue is that subsequent calls to get_admin_page_parent use the pre-set global variable value of $parent_file, rather than searching through $menu again. So when the menu system calls get_admin_page_parent to figure out what the current page is, it thinks the current page is options-general.php's Menu Tester page. Which is not correct.

Resetting $parent_file to an empty string right before calling get_admin_page_parent in the menu system fixes the problem -- the function then finds the correct menu items to highlight. I'll attach a patch.

Attachments (2)

menutester.php (4.2 KB) - added by jhodgdon 17 years ago.
Plugin for testing
admin-menu.diff (2.1 KB) - added by jhodgdon 17 years ago.
Patches to fix this problem

Download all attachments as: .zip

Change History (6)

@jhodgdon
17 years ago

Plugin for testing

#1 @jhodgdon
17 years ago

Hmmm... The suggested patch of resetting the $parent_now variable doesn't entirely work. It fixes the main menu problem, but now when I click on the Options page, it shows that the current menu item is the Manage submenu page, which is wrong.

I fixed this by adding a bit to the matching code in the get_admin_page_parent function, being careful not to wreck the matching for submenu pages.

Patch coming shortly -- it was tested in [6124] with the previously-attached plugin.

@jhodgdon
17 years ago

Patches to fix this problem

#2 @westi
17 years ago

I think the real problem here is maybe that get_admin_page_parent() is designed to provide the root parent page of the currently displayed admin page rather than a random page in the menu system.

Maybe a different patch that resolves the parent discovery stuff in the addition code will be a better fix?

#3 @mrmist
16 years ago

  • Keywords has-patch removed
  • Milestone 2.9 deleted
  • Resolution set to invalid
  • Status changed from new to closed

Relates to pre-2.7 menu system so closing. Re-open if it's still an issue against 2.7.

#4 @jhodgdon
16 years ago

Just to be sure, I retested with the sample plugin that is attached to this bug, and it could not be reproduced. So I agree it is fixed in 2.7.

Note: See TracTickets for help on using tickets.