Opened 11 years ago
Last modified 3 months ago
#32422 new defect (bug)
add_menu_page() returns incorrect $hookname when using plugin filename as $menu_slug
| Reported by: |
|
Owned by: | |
|---|---|---|---|
| Milestone: | Awaiting Review | Priority: | normal |
| Severity: | normal | Version: | 4.2.2 |
| Component: | Plugins | Keywords: | has-patch reporter-feedback 2nd-opinion |
| Focuses: | administration | Cc: |
Description
When using add_menu_page() and add_submenu_page() with filenames in the $menu_slug parameter, the resulting hookname does not match the expected do_action('load-' …) hookname.
In the following examples the expected hookname from do_action( 'load-' . $plugin_page ); (in wp-admin/admin.php) is: load-myplugin/admin/resources.php. However, the hookname returned by $hookname = get_plugin_page_hookname( $menu_slug, $parent_slug); (from add_submenu_page() in wp-admin/includes/plugin.php) is myplugin_page_myplugin/admin/resources.
The result is, this code does not work:
$hookname = add_submenu_page("myplugin/dashboard.php", 'Resources', 'Resources', 'edit_pages', "myplugin/resources.php");
add_action($hookname, 'add_resources_options' );
function add_resources_options() {
add_screen_option('per_page', array(
'label' => 'Resources',
'default' => 10,
'option' => 'resources_per_page'
));
}
However, the hooknames do match when using add_menu_page() and add_submenu_page() with a callback slugname. This code works (notice only the first line has been changed to use callback slugs instead of filename slugs):
$hookname = add_submenu_page('dashboard', 'Resources', 'Resources', 'edit_pages', 'myresources', 'myresources');
add_action($hookname, 'add_resources_options' );
function add_resources_options() {
add_screen_option('per_page', array(
'label' => 'Resources',
'default' => 10,
'option' => 'resources_per_page'
));
}
The solution is either:
- Modify
function get_plugin_page_hookname() …to return a hookname that is caught bydo_action( 'load-' . $plugin_page );. - Or modify
do_action( 'load-' . $plugin_page );to use the hookname currently returned byget_plugin_page_hookname().
Attachments (1)
Change History (6)
#2
@
10 years ago
32422.1.patch introduces the following changes:
- Remove the
-network&-userappending bit inclass-wp-screen.phpWP_Screen::get() - Append
-networkor-useringet_plugin_page_hookname()instead
This assumes get_plugin_page_hookname() is not contextually strict, and might be a problem if you use it in the site context in the network admin. In this case, the -network suffix is added via a not-network-specific hook. Now that I mention it, this might already happen but inversely the way the code is today.
#5
@
3 months ago
- Keywords 2nd-opinion added; needs-testing removed
I attempted to test the attached patch using:
npm run grunt patch:32422
During patching, the following message appeared:
Reversed (or previously applied) patch detected! Assume -R? [y]
This usually indicates that the patch may have already been applied, partially applied via another ticket, or that the patch is too outdated to apply cleanly against current trunk.
This is a very old ticket and core has changed significantly since the patch was created. It is also possible that the original issue has been addressed elsewhere over time.
Because of this:
- Removing
needs-testing - Keeping
reporter-feedbackto confirm whether the issue still exists in current WordPress - Adding
2nd-opiniondue to uncertainty whether the issue is still reproducible or already solved indirectly
A confirmation that the issue still exists in current trunk would help move this ticket forward.
Related: #16238, #28226.
Perhaps also related, the menu manipulation functions also do not return adequate
-networkor-usersuffixes.Incoming patch will attempt to address all of these issues together.