list_table_primary_column doesn't work following some manage_$screen_columns

[soulseekah]

The list_table_primary_column filter short-circuits when using the manage_users_columns filter to add/remove columns instead of extending the WP_List_Table class.

For example, let's add a column to WP_Users_List_Table and try setting it as primary:

add_filter( 'manage_users_columns', function( $columns ) {
    $columns['company'] = 'Company';
    return $columns;
} );

So far so good. Now let's set the 'company' column as primary.

add_filter( 'list_table_primary_column', function( $column, $screen ) {
    if ( $screen != 'users' ) return;
    return 'company';
}, null, 2 );

Expect it to work and yet it doesn't. Why?

Let's take a look at the get_primary_column_name function in ​https://github.com/WordPress/WordPress/blob/master/wp-admin/includes/class-wp-list-table.php#L939

See the short circuit?

if ( empty( $column ) || ! isset( $columns[ $column ] ) ) {
    $column = $default;
}

Well turns out that the $columns are retrieved via get_columns which is unfiltered in some subclasses. Namely WP_Users_List_Table, which returns:

public function get_columns() {
    $c = array(
        'cb'       => '<input type="checkbox" />',
        'username' => __( 'Username' ),
        'name'     => __( 'Name' ),
        'email'    => __( 'Email' ),
        'role'     => __( 'Role' ),
        'posts'    => __( 'Posts' )
    );
    if ( $this->is_site_users )
        unset( $c['posts'] );
    return $c;
}

Without reusing the filter there we're simply screwed. Note that this doesn't happen for all builtin subclasses. For example WP_MS_Users_List_Table does it right: ​https://github.com/WordPress/WordPress/blob/master/wp-admin/includes/class-wp-ms-users-list-table.php#L160

So let's add the missing filters to get_columns, shall we?

[soulseekah]

Affected:

• wp-admin/includes/class-wp-plugin-install-list-table.php
• wp-admin/includes/class-wp-users-list-table.php
• wp-admin/includes/class-wp-comments-list-table.php
• wp-admin/includes/class-wp-plugins-list-table.php
• wp-admin/includes/class-wp-themes-list-table.php
• wp-admin/includes/class-wp-terms-list-table.php
• wp-admin/includes/class-wp-ms-themes-list-table.php
• wp-admin/includes/class-wp-ms-themes-list-table.php
• wp-admin/includes/class-wp-links-list-table.php

As far as I can tell.

[SergeyBiryukov]

As a simple fix, WP_List_Table::get_primary_column_name() could use get_column_headers()​ which does apply the filter. We already use it in WP_List_Table::get_column_info()​.

With 34508.patch​, your snippets work for me.

[wonderboymusic]

In 35565:

List Tables: to ensure that get_primary_column_name() can match columns that were added via the "manage_{$screen->id}_columns" filer, call get_column_headers() instead of $this->get_columns().

List Table classes and WP_Screen are already tangled together. The parent list table constructor adds a filter that is called by a function that references the instance globally, even though we have access to it via composition directly in the class that is adding the filter. So the fact that functions here have to be called that reference a screen instance we already have access to is what one might call... less than elegant.

#OOP

Props SergeyBiryukov.
Fixes #34508.