Make WordPress Core

Opened 7 years ago

Last modified 5 years ago

#40649 new defect (bug)

parent_file filter seems to be overwritten by get_admin_page_parent call

Reported by: jontis's profile jontis Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.7.4
Component: Menus Keywords:
Focuses: administration Cc:

Description (last modified by welcher)

In wp-admin/menu-header.php the filter parent_file seems to have little to no impact most of the time.
This is because the global $parent_file variable often gets immediately overwritten by the get_admin_page_parent function call.

<?php
/**
 * Filters the parent file of an admin menu sub-menu item.
 *
 * Allows plugins to move sub-menu items around.
 *
 * @since MU
 *
 * @param string $parent_file The parent file.
 */
$parent_file = apply_filters( 'parent_file', $parent_file );

/**
 * Filters the file of an admin menu sub-menu item.
 *
 * @since 4.4.0
 *
 * @param string $submenu_file The submenu file.
 * @param string $parent_file  The submenu item's parent file.
 */
$submenu_file = apply_filters( 'submenu_file', $submenu_file, $parent_file );

get_admin_page_parent();

I am not completely sure what the purpose of the get_admin_page_parent is, but maybe the filter should be called after or inside that function?

To try to give an example. If I have the "Broken Link Checker" plugin installed and try to move it's settings page to the Tools menu I would want use this code:

<?php
add_filter('parent_file', 'move_to_tools');
function move_to_tools($parent_file){
  global $hook_suffix;

  if($hook_suffix === 'settings_page_link-checker-settings'){
    $parent_file = 'tools.php';
  }

  return $parent_file;
};

However, because of the above mentioned issue this code does nothing.
I am aware completely moving a settings page requires more work than this, this is purely an example.

Change History (5)

#1 @jontis
7 years ago

  • Component changed from General to Plugins

#2 @jontis
7 years ago

  • Component changed from Plugins to Menus

#3 @welcher
7 years ago

  • Description modified (diff)

#4 @welcher
7 years ago

  • Description modified (diff)

Thanks for the report @jontis!

Moving filters around can have unexpected consequences for plugins using them. That said, did you want to try and get a patch in place?

#5 @nohns
5 years ago

So this is still an issue. I made really ugly hack to fix this, but it works ¯\_(ツ)_/¯

What it does:

  • It changes globals temporarily to fit the needs of the get_admin_page_parent() function
  • Then changes the globals back to their previous state after side menu has rendered

Basically I think the the only function using these globals while they are modified is get_admin_page_parent(), which only sets the $parent_file variable, so it should be safe however pretty ugly and dirty.

I made a little class to house this hack:

<?php


class Parent_File_Hack
{
    public $old_nopriv      = null;
    public $old_real_parent = null;
    public $old_plugin_page = null;
    public $old_pagenow     = null;

    /**
     * Setup action and filters to complete the hack 
     * 
     * @return void
     */
    public function listen()
    {
        // Fire the parent_file filter as late as possible to start the hack
        add_filter('parent_file', array($this, 'start_parent_file_hack'), 1000001);

        // End the hack right after the menu is rendered
        add_action('adminmenu', array($this, 'end_parent_file_hack'));
    }

    /**
     * The function to call when we want to start preparing and setting the new parent_file value
     */
    public function start_parent_file_hack($parent_file)
    {
        global $_wp_menu_nopriv, $_wp_real_parent_file, $plugin_page, $pagenow;

        // Save the old variables
        $this->old_nopriv      = $_wp_menu_nopriv;
        $this->old_real_parent = $_wp_real_parent_file;
        $this->old_plugin_page = $plugin_page;
        $this->old_pagenow     = $pagenow;

        // Set globals to artificial names to force the parent_file variable to be assigned to what we want
        $pagenow = 'admin.php';
        $plugin_page = 'wgy_ambassadors';
        $_wp_menu_nopriv[$plugin_page] = $plugin_page;
        $_wp_real_parent_file[$plugin_page] = $parent_file;

        return $parent_file;
    }

    /**
     * The function to call when we want to revert the changes that were made by the hack
     * 
     * @return void
     */
    public function end_parent_file_hack()
    {
        global $_wp_menu_nopriv, $_wp_real_parent_file, $plugin_page, $pagenow;

        // Change back the globals to their previous states
        $_wp_menu_nopriv      = $this->old_nopriv;
        $_wp_real_parent_file = $this->old_real_parent;
        $plugin_page          = $this->old_plugin_page;
        $pagenow              = $this->old_pagenow;
    }
}

To use this hack, simply initialize a Parent_File_Hack object, and run the listen() method on it, somewhere in your code you see fit. Just "set it and forget it", and then just use your filters as you normally would (Make sure you don't pick a priority over 1000001):

<?php

// Initialize the object and listen for the 'parent_file' and 'adminmenu' hooks
$hack = new Parent_File_Hack();
$hack->listen();

// ...

add_filter('parent_file', function($parent_file) {
    // ...
    return 'your_parent_file';
});

add_filter('submenu_file', function($submenu_file) {
    // ...
    return 'your_submenu_file';
});
Note: See TracTickets for help on using tickets.