#23316 closed defect (bug) (fixed)
Top level admin sidebar menu items with conflicting positions bury one another
Reported by: | beautomated | Owned by: | chriscct7 |
---|---|---|---|
Milestone: | 4.4 | Priority: | normal |
Severity: | major | Version: | 3.5.1 |
Component: | Plugins | Keywords: | has-patch commit needs-codex |
Focuses: | administration | Cc: |
Description
I have now seen two separate instances where a top level admin sidebar menu item wasn't showing up when another Plugin or Theme was activated. In the most recent case, using WP 3.5.1. my Plugin was creating a top level menu item with no position specified (blank, default). When I activated my client's Theme, our Plugin sidebar item disappeared and in place came the Theme Options item. I set our Plugin to use position 70, and it came back in place of the Users top level menu item.
In the earlier case, I had a custom post type that was requesting position 20. Whenever I activated Gravity Forms, my custom post type menu item was disappearing. This was with WordPress v3.5
According to the codex page: "WARNING: if two menu items use the same position attribute, one of the items may be overwritten so that only one item displays! Risk of conflict can be reduced by using decimal instead of integer values, e.g. 63.3 instead of 63 (Note: Use quotes in code, IE '63.3')."
This seems like a bug to me. Why should items be allowed to completely overwrite one another? Shouldn't they just fall in line, albeit randomly when two conflict? I can see tons of problems with Themes and Plugins killing one another's top level menu items, and the user not understanding what's going on when they loose something unexpectedly.
Attachments (2)
Change History (16)
#2
@
12 years ago
In the mean time, we could check isset( $menu[ $position] )
and increment $position
until we have an empty space.
#3
follow-up:
↓ 4
@
12 years ago
Floats can't be used as array indicies, so that warning is plain wrong anyway.
#4
in reply to:
↑ 3
@
12 years ago
Replying to nacin:
Floats can't be used as array indicies, so that warning is plain wrong anyway.
Note the "Use quotes in code, IE '63.3'". It's using strings as the indices, then relying on type coercion in the sorting code.
#5
@
11 years ago
- Keywords dev-feedback needs-refresh 2nd-opinion added
Just to add a point; Custom Post Type Menu Positions does not allow float / string positions.
Because of the followinf code snippet in wp-admin/menu.php (Line 108)
$ptype_menu_position = is_int( $ptype_obj->menu_position ) ? $ptype_obj->menu_position : ++$_wp_last_object_menu; // If we're to use $_wp_last_object_menu, increment it first.
It forces to have an integer value otherwise it will assign ext incremented position for the custom post type menu.
#6
@
9 years ago
- Keywords has-patch added; needs-refresh 2nd-opinion removed
Patch: If the menu position is already in use, make the position it registers to a substring of the integer base conversion of the md5 of (the menu slug and menu title of the admin page being registered concatenated), multiplied by 0.00001 to always get a number < 1, and then added to the original menu position. This returns a nearly guaranteed unique number, that is repeatable to generate, and easy to do so
#7
@
9 years ago
- Milestone changed from Awaiting Review to 4.4
- Owner set to chriscct7
- Status changed from new to assigned
#8
@
9 years ago
- Component changed from Administration to Plugins
- Focuses administration added
- Keywords commit added; dev-feedback removed
#9
@
9 years ago
Basic test code:
add_action( 'admin_menu', 'register_my_custom_menu_page' );
function register_my_custom_menu_page(){
add_menu_page( 'custom menu title', 'custom menu', 'manage_options', 'custompage', 'my_custom_menu_page', '', 63 );
}
function my_custom_menu_page(){
echo "Admin Page Test";
}
add_action( 'admin_menu', 'register_my_custom_menu_page2' );
function register_my_custom_menu_page2(){
add_menu_page( 'custom menu title2', 'custom menu 2', 'manage_options', 'custompage2', 'my_custom_menu_page2', '', 63 );
}
function my_custom_menu_page2(){
echo "Admin Page Test2";
}
#10
@
9 years ago
- Keywords needs-codex added
Codex article needs to be updated to remove the guidance that plugins should use a decimal number for the position to avoid conflicts, as after this patch, plugins can register on the same priority without context. Possibility also a make article worthy, since many plugin authors will be happy this is fixed, and to let them know of the change.
#12
follow-up:
↓ 13
@
9 years ago
Rather than perpetuating the Codex, let's future-proof the docs. We can do that by improving the inline docs and/or writing up some supplemental information for the code reference, then redirect the Codex page(s) for add_menu_page()
and add_submenu_page()
.
Inline docs will carry over when the code reference is reparsed for the 4.4 release. Supplemental information can be pushed live anytime.
#13
in reply to:
↑ 12
;
follow-up:
↓ 14
@
9 years ago
Replying to DrewAPicture:
Rather than perpetuating the Codex, let's future-proof the docs. We can do that by improving the inline docs and/or writing up some supplemental information for the code reference, then redirect the Codex page(s) for
add_menu_page()
andadd_submenu_page()
.
Inline docs will carry over when the code reference is reparsed for the 4.4 release. Supplemental information can be pushed live anytime.
The information is custom to the current codex, so the proposal is to just not change the current codex then, since that information wouldn't be auto-carried over?
#14
in reply to:
↑ 13
@
9 years ago
Replying to chriscct7:
The information is custom to the current codex, so the proposal is to just not change the current codex then, since that information wouldn't be auto-carried over?
Correct. The problem is that since these particular Codex articles are already mid-migration (as are many many others), at some point we can't guarantee that anything new that gets added is going to be carried over. The best thing we can do is solidify the inline docs as the canonical source of information. This is why changelog entries for significant functionality changes are important, they preserve the history when read out of context.
If there's supplemental info – that is to say something we don't want to bloat the inline docs with – for the time being, submitting that as a "user contributed note" to the code reference article(s) is the best way to ensure it gets carried over.
I guess #12718 would solve this.