When registering custom post type, menu_position isn't honored if it's a number passed as a string

[nathanrice]

When registering a custom post type, if you use '50' instead of just 50, is_int() will fail and the position will be set to ++$_wp_last_object_menu. Seems safer to use isset() instead, since position doesn't technically need to be an integer. ($menu gets sorted before being output, and '50' is the same as 50 in that case)

This has the added benefit of allowing MANY more spaces in the $menu variable. For instance, if you passed '50.555' as menu_position, there's very little chance that this will conflict with another theme/plugin register a custom menu or custom post type, and will successfully go between 50 and 51 in the $menu array.

There are currently no checks on $position when using add_menu_page() ... but the checks on register_post_type() currently make this an invalid universal solution.

Personally, I don't see a downside to letting menu_position be a string. Anything other than a number (whole or decimal) will just go to the bottom of the $menu array, anyway, after the sort.

I'm submitting two patches ... one is type independent, and the other casts menu_position as (int) after the isset() check.

Use isset() instead of is_int() when setting the $menu position for custom post types

Use isset() instead of is_int() when setting the $menu position for custom post types. Cast position as (int).

Both patches are the same?

Also, is_numeric() would preserve the validation aspect (string values for a position sound bad to me) while not requiring an integer.

is_numeric() check

@kawauso
I like that idea too. I'm not as averse to the idea of text strings as you are, but this seems like it would make everyone happy.

It's probably a good idea to add this check on add_menu_page() too.

Adds is_numeric() check to wp-admin/menu.php and wp-admin/includes/plugin.php

16856.2.diff​ looks good.

Allowing strings as menu positions is an interesting idea, but it kind of breaks the paradigm.

@scribu
Thanks!

The current menu position paradigm is weak, IMO, and I think other devs agree. Giving 15 positions between Comments and Appearance for CPTs and custom menus means there is bound to be a conflict at some point. The 15 position limit itself isn't the weakness, it's the fact that there's a limit at all.

Ideally, menu_position would accept a section as a parameter. Main, Content, Settings, for example. And you could register a new section too, if you wanted, that could go at the bottom, or between two other sections.

register_menu_section( 'my-section', array( 'main', 'content' ) );

You get the idea. Register 'my-section' between 'main' and 'content', adding the menu separaters automatically.

Then, top level menus (custom menus, CPTs, etc.) get added to a section (default or custom) based on the order they're registered. Let themers and plugin writers fight over who's menu gets added first/last, but at least they all show up.

Oh, and backward compatibility shouldn't be an issue, since the numeric positions should easily correspond to a menu section. The real weakness is allowing people to position their menus within sections. I understand that a CPT doesn't necessarily belong below Comments, for instance. Probably not an insurmountable problem, but one worth noting.

Yes, problem already noted: #12718 (perfect example of a derayeled ticket, otoh)

Replying to scribu:

(perfect example of a derailed ticket, otoh)

No kidding. Is there a particular ticket that is currently active for refactoring the admin menu system? I would potentially be interested in contributing.

That would be #16342, I think.

Changing menu code in a dot release is rather scary. This seems like an enhancement rather than a fix for a regression. Let's take care of it later. Punted per bug scrub.

#18622 has another conflict example (closed as duplicate).

I've found a possibly related bug to this. Found a theme that registers 3 public custom post types (only one of which has a numeric menu position) and a plugin uses add_object_page (without a numeric position). The plugin's menu overwrites one of the post types, but not the one with the numeric menu position. I had to solve this for a client today.

Conceivably, this means that a plugin registering a CPT without a position, and another using add_object_menu can be caused to conflict by an unrelated 3rd plugin using a CPT with a numeric position. Strange huh?

Disclaimer: I didn't dive into the core code, or deeply test this theory. I might after work.

I think it would be great if this was updated into core. Last updated 18 months ago. +1 tested and working! Is there time to get this into 3.6 before code freeze?

This one is really bothering me, really wish it could handle floats like 20.1 etc.. which would be covered by is_numeric() vs a strict is_int() check.

Can someone who could bless this for 3.7 explain what it would take for this ticket to move forward? Updated patch is one thing, but I wouldn't want to update it and then it not move forward.