Custom Post Types break admin menu

[TobiasBg]

I have a plugin that uses add_pages_page() to add a menu item to the "Pages" menu of the admin menu.

Due to the Custom Post Types, the corresponding parent page was renamed from "edit-pages.php" to "edit.php?posttype=page".

This breaks the URLs of plugin menu entries that are added to the "Pages" menu, as they will look like "edit.php?posttype=page?page=plugin-slug" (note the two ?).

The reason for this is in the lower part of _wp_menu_output() in wp-admin/menu-header.php, where the "?page=" is hard-coded, like
href='{$item[2]}?page={$sub_item[2]}'
instead of using add_query_arg().

There are checks for ? in URLs in that function, but they don't seem to get applied for submenu entries.

[TobiasBg]

Demo Plugin to show the issue, no magic in there

[TobiasBg]

I attached a demo plugin.

Everything works fine on WP 2.9.2, but on WP 3.0 the described issue happens.

[TobiasBg]

Patch to fix links in admin menu

[TobiasBg]

I attached a patch that fixes the links in the admin menu, by using add_query_arg, however there seems to remain a problem with user rights, as I now get redirected to /wp-admin/?c=1 when clicking /wp-admin/edit.php?post_type=page&page=demo-plugin-slug (with above demo plugin installed)

Details for the patch:

• Changes strcmp(...) == 0 comparison to plain $string1 == $string2 comparison

• uses add_query_arg to generate URLs (which remedies the problem of two ? in the URL)

• fixes the if/ifelse/else logic, as the else part was never called, as either the first ( if $parent_exists ) or the second condition ( if !$parent_exists ) are always fulfilled.

• $parent_exists is now longer necessary, so I removed it and put the condition into the if-clause

What's left: The links in the menu are looking ok now, but are not accessible (wp_redirect() to /wp-admin/?c=1), most probably coming from somewhere in /wp-admin/menu.php.

[TobiasBg]

To add on:

The redirect to /wp-admin/?c=1 happens on a test site of trunk with Multisite enabled.

On trunk with Singlesite, I get a wp_die: You do not have sufficient permissions to access this page.

[TobiasBg]

I narrowed this down some further:

The above mentioned redirect or wp_die is caused by user_can_access_admin_page() returning false.

It is returning false, because $_registered_pages[$hookname] is not set (​here).

$hookname has the value "posts_page_demo-plugin-slug", while the expected hookname is "edit-phppost_typepage_page_demo-plugin-slug".

Both are generated by get_plugin_page_hookname() (​here for the isset-check and ​here for the population in the array).

In get_plugin_page_hookname(), the difference between the two is caused by differing $admin_page_hooks[$parent] (​here).

The hookname "edit-phppost_typepage_page_demo-plugin-slug" seems to come from ​here, while the hookname "posts_page_demo-plugin-slug" seems to come from ​here.

Any ideas?

[TobiasBg]

I did some more digging:

It looks like the problem is caused by the call of get_admin_page_parent() without an argument ​here. It will then return "edit.php" instead of "edit.php?post_type=page".
Because of the empty argument, the function attempts to use $pagenow to find the $parent. For the "Pages" menu, that also is edit.php now, due to the Custom Post Types (it used to be edit-pages.php). This correlates to the parent of the "Posts" menu. Because of that, it is assumed that the menu entry belongs to the "Posts" menu, but it was registered for the "Pages" menu.

Some other weirdness (that's not of functional but more optical relevance) comes from ​this line. The basename() call on "edit.php?post_type=page" basically fails and just returns the input unmodified, so that sanitize_title cleans it to "edit-phppost_typepage". If basename would deliver "edit" as it does with the argument "edit.php", the problem would vanish, however the "Pages" menu would use an incorrect parent.

I changed the title of this ticket, because there's a more severe issue than just the missing usage of add_query_arg().

[ryan]

That seems to fix parenting and access, but submenu highlight is still broken.

[TobiasBg]

Thanks for the patch.

Access also works fine now.

I investigated on the submenu highlight. Problem is that the check ($item[2] == $self)​in this line is false, because $self is == 'edit.php', while $self is still == 'edit.php?post_type=page'.

$self seems to come from ​here, while $item[2] eventually is defined ​here, I believe.

This might also be related to the change in menu.php of your patch. I don't know if just removing the querystring is a good idea, as then two menu entries could have the same hookname, although they are in different menus. A submenu under "Posts" has "posts_page_plugin-hook" before and after introduction of post types, while a submenu under "Pages" has "pages_page_plugin-hook" before but "posts_page_plugin-hook" after. Instead of removing the querystring, we should maybe map the page post type to "pages", i.e. with the $compat array ​here.

[ryan]

Fixes submenu highlight and implements your suggestions.

[TobiasBg]

Thanks. Your patch works really nice as far as I can see from my tests. The demo plugin works and my actual plugin works, too.
Links, access and highlighting are fine. Your additional work for my suggestion should also make it easier and safer to add a submenu to other main menus created by Custom Post Types in the future.

However, we should probably get some more testing in. I'm changing the tags accordingly and maybe the ticket can be brought up in the dev chat, so that people already working with Custom Post Types are encouraged to test the patch.

[ryan]

(In [13579]) Fix submenus for post types. Props TobiasBg. see #12453

[ryan]

I committed what we have for easier testing. Leaving the ticket open for feedback.

[TobiasBg]

Just noticed another tiny issue:

The $page_title variable of the add_submenu_page() call is not respected for custom post type parents. The <title> tag just has the default WP title.

I guess there is some more logic needed in get_admin_page_title().

[scribu]

RelatedȘ #12985

[dd32]

The $page_title variable of the add_submenu_page() call is not respected for custom post type parents.

See this commit on #12985:

(In [14126]) Check $pagenow?post_type=$typenow for submenu titles, Fixes page titles for plugin pages added to custom post_type's. Fixes #12985

Closing this as fixed due to no other issues popping up.