Reduce reliance on global variables in list tables

[ryan]

The list tables rely on several global variables such as $post_type, $taxonomy, $post_type_object, and $tax. Instead of globals, list tables should consult the WP_Screen object. Further, to accommodate ajax handlers, the list table constructors should accept screen IDs as an arg and create and store a WP_Screen object internally.

[ryan]

Previous work on this: ​http://core.trac.wordpress.org/attachment/ticket/21742/21742.5.diff

[ryan]

Refresh of nacin's patch

[ryan]

In [21914]:

Reduce reliance on global variables in the list tables. Allow passing a screen ID to the list tables so that ajax handlers can set the needed screen.

Props nacin
fixes #21871

[ocean90]

When adding a tag or category on edit-tags.php you will get a notice:

Notice: Trying to get property of non-object in /wp-admin/includes/class-wp-terms-list-table.php on line 290

$this->screen->post_type is null.

[nacin]

I was afraid of that. I think this is actually fixable in WP_Screen.

When we aren't given a hook_name, we try to clean the post type and taxonomy from $_REQUEST. We also try to set $post_type to 'post' on edit-tags.php when we have no $post_type. But we should always try to set $post_type to 'post', whether or not we're given a hook name.

See 21871.4.diff​.

[ocean90]

21871.4.diff works well for posts, but not for custom post types.

[ryan]

Time to finally do edit-tax-type?

[ryan]

Append post type to edit taxonomy screen ids. edit-post_tag-post

[nacin]

As hyphens are allowed in taxonomy and post type names, this could be more trouble than it is worth.

What about allowing a query string to be passed with as the hookname, from which we can then glean post type or taxonomy?

<?php
$hookname = 'edit-post_tag?post_type=foo';
$query_string = null;
if ( false !== strpos( $hookname, '?' ) )
        list( $hookname, $query_string ) = explode( '?', $hookname, 2 );
parse_str( $query_string, $query_args );
var_dump( $hookname, $query_args );

[nacin]

For the record, I feel really dirty suggesting that.

[ryan]

Or a different separator in the screen ID that taxonomies don't use? edit-post_tag:post, edit-post_tag|post, edit-post_tag/post?

[nacin]

My first thought was indeed to use a colon. But then I started thinking about parse_str() and it went downhill from there.

[ryan]

I like keeping it in the ID string because it allows set_current_screen( 'edit-post_tag:post' ) to set all of the needed bits. Back compat problems are possible though. Looking into that.

[ryan]

Hmm, stuff like this might kill the notion.

"manage_{$this->screen->id}_columns"

[ryan]

I think we need a guid property that is actually unique.

[ryan]

Incomplete, barely test PoC for screen guid

[ryan]

Now with mess to guid translation

[ocean90]

#21968 was marked as a duplicate.

[scribu]

Replying to nacin:

What about allowing a query string to be passed with as the hookname, from which we can then glean post type or taxonomy?

I think that's actually the most elegant solution, since that's the information that's originally used to generate the screen object.

That is, of course, assuming that all the logic is contained within the WP_Screen class, as opposed to scatered all over the place.

[scribu]

Oh, I was actually thinking of passing the original query string, not a hookname or whatever.

[scribu]

So, in the admin page, you'd do:

$screen = WP_Screen::get_instance( basename(__FILE__), $_GET );

and in an AJAX request, you'd do:

$screen = WP_Screen::get_instance( $_POST['admin_file'], $_POST['admin_file_args'] );

Or something along those lines.

[ryan]

The screen basically needs four things: the admin context ( user | network | site | front ), the particular admin page, post_type, and taxonomy. Currently, get() attempts to get these in several ways.

• The global hook_suffix along with post_type and taxonomy from the request and the *_ADMIN constants
• A hook name that is pretty much hook_suffix with the .php being optional plus post_type and taxonomy from request and the *_ADMIN constants
• An "ID" that is a little more removed from the hook suffix that encodes the admin context and the post_type and taxonomy. It currently does not provide both the post_type and the taxonomy to edit-tags.php. Only taxonomy is provided.
• The guid patches introduce a new id that provides everything needed in one string.

There are several methods used for passing screen info along to ajax handlers.

• Inline JS in admin-header.php also attempts to provide some screen related information via pagenow and typenow variables.

• WP_List_Table::_js_vars() passes along the screen id and base.

• edit-tags.php passes the screen ID in a hidden input.

get() is already pretty much a query processor so passing query strings to it won't require much work. Once we parse that info we still need to create an ID of some sort to use for option names, filters, and so on and also to provide some insulation from file naming changes. The existing id property in the screen object seems sufficient for these purposes. I doubt anyone will want to target to guid specificity.

So, I guess we need to answer the question of how we want to pass screen info to ajax handlers and JS. We're currently all over the place. A guid is one simple string, but passing some sort of struct or query string is pretty simple too.

[ryan]

Minimal fix for edit-tags

[nacin]

In [21974]:

Add back the space between class names removed in [21914]. see #21871.

[ryan]

In [21982]:

The edit-tags screen ID does not contain the post_type. Fallback to post_type in the REQUEST. Fixes AJAX term addtions from edit-tags.php. fixes #21871

[ryan]

Decided to do a minimum fix and discuss ID improvements for another release. We don't need yet another rushed screen ID system.