Submenu highlighting in ACP By URI/Query to highlight correctly for some plugins

[ixiter]

When a plugin creates an own Menu with own SubMenus in the ACP, and uses links with the same filename but different parameters for the SubMenu entries, the highlighting for the "current" page fails for these SubMenus.
You can reproduce this bug with the statpress plugin. Submenus will not be highlighted. Always and only the "Overview" Submenu is highlighted!

In wp-admin/menu-header.php the "current" status of an item is just chosen from plugin name and plugin filename. The filename is taken from the page parameter from the URI. Other parameters are ignored. This causes the bug.
My patch compares the URI Parameters with $sub_item[2], if there is more than one parameter, instead of the filename only. That way also additional parameters with same filenames are recognized and teh SubMenus are highlighted correctly.

File wp-admin/menu-header.php
FIND: (Lines 97 and 98)

if ( !current_user_can($sub_item[1]) )
continue;

INSERT AFTER:

$params = substr($_SERVER['QUERY_STRING'], 5); // remove the 'page='

FIND: (Line 110)

} else if ( (isset($plugin_page) && $plugin_page == $sub_item[2] && (!file_exists($item[2]) || ($item[2] == $self))) || (!isset($plugin_page) && $self == $sub_item[2]) ) {

REPLACE WITH:

} else if ( (isset($plugin_page) && ($params == '' && $plugin_page == $sub_item[2] || $params != '' && $params == $sub_item[2]) && (!file_exists($item[2]) || ($item[2] == $self))) || (!isset($plugin_page) && $self == $sub_item[2]) ){

Please can you provide a simple plugin that reproduces the issue for us to test against.

Just something that adds the menus themselves will do.

Replying to westi:

Please can you provide a simple plugin that reproduces the issue for us to test against.

Just something that adds the menus themselves will do.

The statPress Plugin does. ​http://wordpress.org/extend/plugins/statpress/

I can confirm this as well. Those entries / files must end with .php or they couldn't be compared against. this is because the other file parameter is sanitized and then used for comparison. see #9426

could figure out on how to prevent having the bug. attached you find a plugin file that is a usage-case. highlighting works, and even the parameter can be read out. you can not name the parameter page and it must be added with & and not ?.

test plugin that creates a working menu in ACP with submenu entries that can be selected.

Replying to hakre:

it must be added with & and not ?.

correction: it must be added with ? not &. this produces an URL that might not be valid, but this works with WordPress as far as I've tested it.

big wontfix on this one.

my issue with the suggested patch is it'll work with statspress and others, but not necessarily other plugins that might toss the arguments in a random order.

further, any patch is bound to fail with a plugin that defines a new type of data and passes the ID of the data in the url. picture something like:

admin.php?page=edit_product.php&id=2

anything short of using something other than the file is bound to fail. "my-plugin" should be used as the "file" instead of the FILE constant.

Replying to Denis-de-Bernardy:

big wontfix on this one.

my issue with the suggested patch is it'll work with statspress and others, but not necessarily other plugins that might toss the arguments in a random order.

This was not a patch. This was just a demonstration that submenu entries work. I re-tested it right now. It works with 2.8 Bleeding and 2.7 alpha. Random order works as long as the first parameter is set properly (page parameter). That one is used to signal wordpress's menu which item to hightlight. This demonstrates that the Bugreport is Invalid.

further, any patch is bound to fail with a plugin that defines a new type of data and passes the ID of the data in the url. picture something like:

admin.php?page=edit_product.php&id=2

works with no problems: admin.php?page=edit_product.php?ppage=2&id=2

anything short of using something other than the file is bound to fail. "my-plugin" should be used as the "file" instead of the FILE constant.

it is a constant, therefore you can exchange it with another constant value. no problem with that.

To make my statement clear I will close this Report as waorksforme. Feel free to proof the opposite and reopen then when you got more infos. As demonstrated with the test plugin, this is not a bug.