Bug between CPT menu_position and add_object_page()

[momo360modena]

I write a demo plugin for describe the bug. See attachment.

[momo360modena]

Proof of bug

[scribu]

Reproduced. The problem is that the last position isn't incremented, when using register_post_type().

[mpvanwinkle77]

Ok so this was my first patch submission so be gentle if i'm going about it all wrong. Cheers!

PS The first attachment still had a debugging echo statement in it. Cleaned and re-uploaded.

[mpvanwinkle77]

Patch to wp-admin/menu.php

[scribu]

Hi mpvanwinkle77,

A patch is a diff file, not an entire file.

You will find tutorials for creating diffs here:

​http://core.trac.wordpress.org/#HowtoSubmitPatches

[mpvanwinkle77]

Yikes. My bad. Will resubmit.

[mpvanwinkle77]

Patch to wp-admin/menu.php

[scribu]

Cool. One more thing about diffs: please make them against the root wp dir, not against wp-admin etc.

Also, we love white-space: ​http://codex.wordpress.org/WordPress_Coding_Standards#Space_Usage

[blepoxp]

I was apparently working on this at the same time mpvanwinkle77 was. Submitting my patch too - a little different approach:

It could really use some extra testing though. I had to refactor it several times myself as I continued to find unintended consequences with my first attempts at patching. I'd love confirmation that the current patch doesn't cause problems elsewhere.

It does two things: it prevents dynamically written $menu array keys from overwriting hardcoded keys provided by core. It also prevents keys previously added by custom post types and other add_menu_item calls from being overwritten. To accomplish this, I created a new function. Wasn't sure of the best name and place to define this function so let me know if you think it fits better elsewhere.

[scribu]

I'm not sure using get_unique_menu_key() in add_menu_page() is a good idea.

Some plugins intentionally overwrite core menus, for various reasons. Disallowing that from add_menu_page() will just force them to manipulate $menu directly.

[blepoxp]

This is true. I wonder if we should provide extra param to add_menu_page to force an overwrite. I could see people overwriting without realizing it. That might get into backwards compat issues though.

[scribu]

I have no idea and I think we should concentrate on the bug at hand.

Therefore, I prefer menu.diff​.

[johnjamesjacoby]

Comment, whitespace, and patched from wp root

[ryan]

I tried this patch with my_bug.php and another plugin that does:

register_post_type('thing-y', array('label' => __('Thingies'), 'public' => true, 'taxonomies' => array('post_tag') ) );

And the Thingies menu ended up above the Dashboard menu. I haven't investigated further.

[westi]

This has missed the boat for 3.2

Moving to Future Release for a refreshed patch to address the issues ryan raises.

[layotte]

The last patch added does not seem to fix this for me. I just ran into this problem with another developer's plugin overwriting my plugin's menu position. His plugin is available here: ​http://wordpress.org/extend/plugins/twitter-plugin/ mine is here: ​http://wordpress.org/extend/plugins/leenkme/

In his most recent version, he is calling:

add_menu_page(__('BWS Plugins'), __('BWS Plugins'), 'manage_options', 'bws_plugins', 'bws_add_menu_render', WP_CONTENT_URL."/plugins/twitter-plugin/images/px.png", 101); 

Which for some reason is over witing my plugin, which does not use the position argument.

For what it's worth, blepoxp's patch works fine for me. Of course, I've emailed the developer and told him to remove the position argument. I'm sure it affects more than just my plugin.

[scribu]

Related: #12718.

[Mte90]

patch refreshed

[Mte90]

I refreshed the patch based on 4.5.

[Mte90]

I can refresh but if there is no interest for this ticket I prefer to work on other tickets.

[DrewAPicture]

Let's centralize the discussion #12718.