Opened 7 years ago
Last modified 8 months ago
#47690 reviewing defect (bug)
remove_submenu_page() doesn't remove corresponding entry from $_wp_submenu_nopriv
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| Milestone: | Future Release | Priority: | normal |
| Severity: | normal | Version: | |
| Component: | Administration | Keywords: | has-patch needs-test-info has-unit-tests |
| Focuses: | administration | Cc: |
Description
It can sometimes be desirable to give access to a submenu to a user that wouldn't normally have access to it.
Calling remove_submenu_page() and then calling add_submenu_page() to re-register the screen with a different user capability doesn't work completely because the entry that gets added to the $_wp_submenu_nopriv global by add_submenu_page() doesn't get removed by remove_submenu_page().
This means the menu item appears but access to the screen is denied when user_can_access_admin_page() is called, resulting in a Sorry, you are not allowed to access this page error.
Attachments (1)
Change History (8)
#1
@
7 years ago
- Keywords has-patch added; needs-patch removed
On my attachment 47690.diff, you'll notice on line 1715
remove_all_actions( $hookname );
This is to reciprocate the code from line 1360 of the function add_submenu_page().
I'm not sure if remove_all_actions( $hookname ) is the best solution. If the $hookname from the code
$hookname = get_plugin_page_hookname( $menu_slug, $parent_slug );
is unique for the add_submenu_page() then I suppose there's no harm using remove_all_actions( $hookname ). If it's not then we do need to somehow hold the $function passed on add_submenu_page() and use remove_action( $hookname, $function ) instead.
It is important to remove the action because the submenu page will be accessible using the direct link with the function callback as the output even if the revised remove_submenu_page() is called.
Another thing is, I changed the variable names passed on remove_submenu_page() from $menu_slug and $submenu_slug to $parent_slug and $menu_slug to make it consistent with the variable names used on add_submenu_page().
References:
https://developer.wordpress.org/reference/functions/add_submenu_page/
https://codex.wordpress.org/Function_Reference/remove_all_actions
https://codex.wordpress.org/Function_Reference/remove_action.
This ticket was mentioned in Slack in #core by donmhico. View the logs.
7 years ago
#3
@
6 years ago
- Milestone changed from 5.3 to 5.4
- Owner set to johnbillion
- Status changed from new to reviewing
I haven't had time to review this but thanks for the patch @donmhico , I'll give it some eyes during 5.4.
#4
@
6 years ago
- Keywords needs-testing added
- Milestone changed from 5.4 to Future Release
@donmhico Can you explain the special case handling of tools.php in your patch please?
#6
@
8 months ago
- Keywords needs-test-info needs-refresh added; needs-testing dev-feedback removed
Patch not applying, also some test info would be ideal.
This ticket was mentioned in PR #9191 on WordPress/wordpress-develop by @sainathpoojary.
8 months ago
#7
- Keywords has-unit-tests added; needs-refresh removed
Trac ticket: #47690
Reverse the actions from add_submenu_page() and changed the parameter variable names to be consistent with add_submenu_page()