[wp-trac] [WordPress Trac] #46081: get_admin_page_title() returns nothing

[WordPress Trac]

#46081: get_admin_page_title() returns nothing
----------------------------+-----------------------------
 Reporter:  grapestain      |      Owner:  (none)
     Type:  defect (bug)    |     Status:  new
 Priority:  normal          |  Milestone:  Awaiting Review
Component:  Administration  |    Version:  5.0.3
 Severity:  minor           |   Keywords:
  Focuses:  administration  |
----------------------------+-----------------------------
 The function `get_admin_page_title()` returns nothing in case a menu page
 is registered using `add_menu_page()` and accessed with an url such as
 .../wp-admin/**options.php**?page=my_page_slug

 Normally one should access the custom admin page using .../wp-
 admin/**admin.php**?page=my_page_slug, so it's not really that big of an
 issue. Actually, I've encountered this, because I've been using some old
 code samples, which pointed me to the options.php link. I didn't notice
 the difference, only that sometimes I have a title (from
 `get_admin_page_title()`) and sometimes not.

 While debugging the issue, I found this:

 On .../wp-admin/**admin.php**?page=my_page_slug the `$title` global
 already have the title populated at the time `get_admin_page_title()` is
 called, so it just returns that, all is good.

 On .../wp-admin/**options.php**?page=my_page_slug however the `$title`
 global is empty, so the `get_admin_page_title()` function tries to
 determine the title from the `$menu` global, which contains all registered
 menu entries. Nice.

 {{{#!php
 foreach ( (array)$menu as $menu_array ) {
         if ( isset( $menu_array[3] ) ) {
                 if ( $menu_array[2] == $pagenow ) {
                         $title = $menu_array[3];
                         return $menu_array[3];
                 } elseif ( isset( $plugin_page ) && ($plugin_page ==
 $menu_array[2] ) && ($hook == $menu_array[3] ) ) {
                         $title = $menu_array[3];
                         return $menu_array[3];
                 }
         /*...*/
         }
 /*...*/
 }
 }}}
 (from https://core.trac.wordpress.org/browser/tags/5.0.3/src/wp-
 admin/includes/plugin.php#L1608)

 The problem, however, is that when it loops over the `$menu` array, it
 first compares `$pagenow` to `$menu_array[2]` (which contains the
 menu_slug), but as `$pagenow` is "options.php" this check fails, so then
 it moves on to another check.

 Now this is the part I think the implementation has a mistake. The check
 consist of 3 parts:

 - `isset( $plugin_page )` -> true, as this variable contains the page_slug
 (as well).

 - `($plugin_page == $menu_array[2] )` -> true, when the for loop is at the
 right menu entry, because `$menu_array[2]` has the page_slug too.

 - `$hook == $menu_array[3]` -> **false, but it should be true**. See the
 `$hook` contains the actual page hook, however the menu array does not
 store the page hooks under index 3, but index 5. Index 3 is actually the
 Title, we are trying to find out, as it is visible from being the element
 of the `$menu_array` being set as the global `$title` var and returned in
 the next line.

 You can confirm this by looking at the `add_menu_page()` function, whihc
 defines the same array referenced above as `$menu_array` as:
 {{{#!php
 $new_menu = array( $menu_title, $capability, $menu_slug, $page_title,
 'menu-top ' . $icon_class . $hookname, $hookname, $icon_url );
 }}}
 (https://core.trac.wordpress.org/browser/tags/5.0.3/src/wp-
 admin/includes/plugin.php#L1112)

 `$hookname` is the 6th element of the array, not the 4th as assumed in the
 check above.

 Now, please note, that I'm not trying to say WordPress should support
 loading admin pages on the options.php the same way as admin.php, however:
 - The main thing is that L1612 above compares potatoes to apples when
 comparing `$hook == $menu_array[3]`, so regardless of what special cases
 this code should run other than trying to load an admin page using
 options.php, it cannot be good. Either the whole criteria is pointless, or
 it should be `$hook == $menu_array[5]` instead, because in it's current
 form it'd not evaluate to true. (Or if it does, that indicates something
 else is being broken, and should be fixed too!)
 - If accessing an admin page over options.php is not adequate, maybe
 WordPress should make a warning or notice about it, or simply refuse to
 render the page. Normally I'd not suggest WordPress should be responsible
 for enforcing admin pages to be properly implemented, however, this is
 very misleading to be able to render an admin page almost correctly, in an
 incorrect way. I have `WP_DEBUG` on, but I saw no other symptoms of using
 a wrong URL, other than the missing Title. Everything else worked as
 expected.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/46081>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform