Repository navigation
Option page as sub-page of another menu broken in 2.8.0 #1410
Description
Describe the bug
I have a plugin that uses CMB2 to register an options page as a sub-menu of another custom admin page. This works in 2.7.0 but breaks the url in 2.8.0.
Steps to reproduce (I have confirmed I can reproduce this issue on the develop branch):
The following works with 2.7 producing an admin url with /wp-admin/admin.php?page=some_sub_page
but breaks in 2.8 producing an admin url with /wp-admin/some_sub_page
add_action( 'admin_menu', function() {
add_menu_page(
esc_html__( 'Some Page', 'textdomain' ),
esc_html__( 'Some Page', 'textdomain' ),
'edit_posts',
'some-page',
'prefix_render_admin_menu_page'
);
});
add_action( 'cmb2_admin_init', function() {
$cmb = new_cmb2_box( array(
'id' => 'some_sub_page_metabox',
'title' => __( 'Some Sub-Page', 'textdomain' ),
'object_types' => array( 'options-page' ),
'option_key' => 'some_sub_page',
'parent_slug' => 'some-page',
) );
});
function prefix_render_admin_menu_page() {
echo '<h1>Some Page</h1>';
}
Activity
tw2113 commentedon Feb 24, 2021
I'd need to test and try to confirm, but I wonder if it's coming from the changes that were a part of #1380
sonnysunzu commentedon Mar 2, 2021
I'm also having the same issue
tw2113 commentedon Mar 2, 2021
@sonnysunzu can you provide your CMB2 configuration code as well, in case variations are producing different results, and so we could just verify that a change is indeed affecting both.
sonnysunzu commentedon Mar 2, 2021
as soon as i remove the 'parent_slug' the side menu will then show the link as a stand alone menu item outside of the submenu and works. so something has happened for submenu items to no longer work
wp add menu settings
add_menu_page('Woocommerce Image Sales', 'Image Sales', $settingsCap, 'wis', array($this, 'wis_about'), 'dashicons-products');cmb2 setup
$cmb_options = new_cmb2_box(array( 'id' => 'wis_options', 'title' => 'Options', 'menu_title' => __('Options', 'Options'), 'object_types' => array('options-page'), 'option_key' => 'wis_options', 'parent_slug' => 'wis', 'capability' => 'manage_options', 'display_cb' => array($this, 'wis_options_page_output'), 'description' => '', ));I hope this is what you asked for.
tw2113 commentedon Mar 2, 2021
yep that's the part i was looking for. thanks.
sonnysunzu commentedon Mar 3, 2021
@tw2113, @JiveDig, I found the issue.
inside includes/CMB2_Options_Hookup.php line 71
add_action( $hook, array( $this, 'options_page_menu_hooks' ), $this->get_priority() );if you remove
$this->get_priority()from the above line. Submenus then work again.so looking further into this. Too fix my issue without modifying the cmb2 direct i had to add
'priority' => 'low'to mynew_cmb2_boxso like this
$cmb_options = new_cmb2_box(array( 'id' => 'wis_options', 'title' => 'Options', 'menu_title' => __('Options', 'Options'), 'object_types' => array('options-page'), 'option_key' => 'wis_options', 'parent_slug' => 'wis', 'capability' => 'manage_options', 'display_cb' => array($this, 'wis_options_page_output'), 'description' => '', 'priority' => 'low' ));without going to far as its 1am here and i need to be up for work in the morning. I'm going to take a guess that the hook for the submenu is running before the hook for the parent menu item. I could be wrong I'm tired
tw2113 commentedon Mar 3, 2021
I must not be doing something right, because my submenu items with both code samples above put me on 404 pages on the frontend.
For example, I'm ending up on https://wds.test/wp-admin/some_sub_page for @JiveDig's code. I've reverted myself back to the 2.7.0 release as well, just to help. What URLs are you two getting for your submenus?
JiveDig commentedon Mar 3, 2021
I'm on mobile at the moment, but the exact URLs I get with my reduced code example are all in the OP.
sonnylloyd commentedon Mar 3, 2021
@tw2113 have you set the priority to low ? like i suggested as this solved the issue for me. (sorry i have two accounts one for work and a personal one. this is my personal one, and the sonnysunzu is my work)
JiveDig commentedon Mar 3, 2021
Great find @sonnysunzu. Adding any priority works for me. I tested first with
low, and it worked but was in the wrong place. Same withhigh. But just adding'priority' => 10,put it in the correct place with a link that actually works without 404'ing.So it looks like the change to
get_priority()in 94e80d7 may be missing a fallback/default?https://github.com/CMB2/CMB2/blob/develop/includes/CMB2_Options_Hookup.php#L71
Is currently:
But should be:
JiveDig commentedon Mar 3, 2021
Actually, the get_priority method has a default set like
public function get_priority( $default = 10 ) {so I'm a bit lost without diving any deeper in.Fix admin page url not working in CMB2 2.8.0
sonnylloyd commentedon Mar 3, 2021
@JiveDig @tw2113
I strongly believe the default priority of
highis causing theadmin_menu_hookhook found inCMB2_Options_Hookup.phpon line 71. to run before the parent hook has had chance to run. And because of this the parent page does not exist so it cannot add the sub menuhence when setting the priority to low corrected the order the hooks was run. meaning the hook for the parent was ran first then the hook for cmb2
This would explain why. when removing the parent id and having the menu option as standalone it worked. because it does not depend on a parent existing first
tw2113 commentedon Mar 3, 2021
@sonnylloyd i was trying with version 2.7.0 and no priority argument passed and still wasn't seeing successful option pages. I may try again later tonight or tomorrow with all this new information, but I was really hoping to have a
git bisectchance to try and nail down the exact code change that introduced things.JiveDig commentedon Mar 3, 2021
@tw2113 Did you try my original code?
With a clean install of WP and Twenty Twenty One theme active, if I put the following code in functions.php it works in 2.7.0 and doesn't work in 2.8.0.
JiveDig commentedon Mar 3, 2021
jtsternberg commentedon Mar 4, 2021
That's exactly right. Priority defaults to
highfor boxes: https://github.com/CMB2/CMB2/blob/develop/includes/CMB2.php#L77Which is translated to a hook priority of
5: https://github.com/CMB2/CMB2/blob/develop/includes/CMB2_Hookup.php#L814-L817I didn't notice/realize that by adding this priority thing in 94e80d7 that I would be breaking back-compatibility. Ugh. I'll get this updated so that the priority is
defaultfor options pages, that way it will get a priority of10, putting it back to what it was previously.tw2113 commentedon Mar 4, 2021
@JiveDig i was only ever getting the second one, even though I'm pretty sure I checked out the right git tag to go back to 2.7.0.
Regardless it looks like Justin is on the case as well and has a good lead on things, so I'm going to let him take over from here.
Set default priority to 10 for options pages. Fixes #1410
Merge remote-tracking branch 'upstream/develop' into develop