manage_${post_type}_posts_columns parameters shifted

[Cybr]

Hello,

manage_${post_type}_posts_columns requires three parameters, however, as shown in the link below, it only uses 2.

https://core.trac.wordpress.org/browser/tags/4.2.2/src/wp-admin/includes/class-wp-posts-list-table.php#L978

The paremeters are also shifted.

The example below shows the var dump outputs in render_column_with_bug():

<?php
class Example_Class {

    public function __construct() {   
       add_action( 'admin_init', array( $this, 'init_columns') );
    }
    
    public function init_columns() {

        $supported_post_types = array(
            'posts' => 'posts',
            'pages' => 'pages',
            'category' => 'edit-category',
            'post_tag'  => 'edit-post_tag',
        );

        foreach ( $supported_post_types as $slug => $type ) {
            add_action( "manage_{$type}_columns", array( $this, 'add_column'), 10, 1 );
            
            // Add 3rd parameter because of bug
            add_action( "manage_{$slug}_custom_column", array( $this, 'render_column_with_bug'), 10, 3 );
        }
     }
    
    // Add new column name here
    public function add_column( $columns ) {
        $columns = array_merge( array(
            'my_column_name' => 'column_slug', 
            ), $columns );
  
        return $columns;
    }

    // Output column contents here here
    public function render_column_with_bug( $column, $post_id, $tax_id = '' ) {

        $status = '';

        var_dump ( $column );
        /* 
            Outputs: column name on posts/pages
            Outputs: empty string on taxonomies
        */
        
        var_dump ( $post_id );
        /* 
            Outputs: post id posts/pages
            Outputs: column name taxonomies
        */
        
        var_dump ( $tax_id );
        /* 
            Outputs: empty string on posts/pages
            Outputs: taxonomy id on taxonomies
        */

        if ( $column == 'my_column_name' )
            $status = $this->post_status();

        echo $status;
    }
    
    // Output column contents here here
    public function render_column_bug_fixed( $column, $post_id, $tax_id = '' ) {

        $status = '';

        $type = get_post_type( $post_id );

        if ( !empty ( $tax_id ) && ! $type ) {
            $column = $post_id;

            $screen = get_current_screen();

            $type = $screen->id;
        }

        if ( $column == 'my_column_name' )
            $status = $this->post_status();

        echo $status;
    }
    
    public function post_status() {
        // render output
    }

}
new Example_Class();

[DrewAPicture]

Hi @Cybr, welcome to Trac!

Not sure I'm following. There appears to be some discrepancy in the hooks you're referencing in your report.

First you reference the manage_${post_type}_posts_columns hook, then you've linked to the manage_{$post->post_type}_posts_custom_column hook call in the source, and finally, in your example, referenced manage_{$slug}_custom_column where $slug would need to be posts or pages for post types and a screen ID for taxonomies.

So, which hook exactly are you reporting an issue with? And where are you seeing an indication that that hook "requires" 3 parameters?

Side note:
In your example, you're using manage_{$slug}_custom_column for both post types and taxonomies, in which case you'd want to pass the $type value instead of $slug because the hook in the Terms list table is expecting a screen ID.

[Cybr]

Replying to DrewAPicture:

So, which hook exactly are you reporting an issue with? And where are you seeing an indication that that hook "requires" 3 parameters?

Whoops! Sorry about the mix-up. I've been working for too long :3

This is regarding the manage_{$slug}_posts_custom_column action hook as stated in: ​https://codex.wordpress.org/Plugin_API/Action_Reference/manage_$post_type_posts_custom_column

Replying to DrewAPicture:

In your example, you're using manage_{$slug}_custom_column for both post types and taxonomies, in which case you'd want to pass the $type value instead of $slug because the hook in the Terms list table is expecting a screen ID.

Passing $type instead of $slug will not work on taxonomies, unfortunately. The whole function returns void.

Therefor I'm passing the $slug, which does work, from there I fetch the screen using $screen = get_current_screen(); and then $type = $screen->taxonomy; or $type = $screen->id; as in the example above within the render_column_bug_fixed() function.

[Cybr]

class-wp-terms-list-table.php manage_{$post_type}_custom_column fix

[Cybr]

WP 4.3.0

[Cybr]

Tested out my plugins on WP3.8, it seems that this misinformation has been present since at least then. I think it's best to leave this "bug" open for now and edit the codex accordingly to eliminate confusion.

[DrewAPicture]

Hi @Cybr,

I'm sorry, I'm still not understanding what bug you're pointing out. The patch you uploaded is showing as a PHP file, not a diff, and is for WP_Terms_List_Table, yet your comment on the patch says it's for the manage_{$post_type}_custom_column hook, which isn't even in that file.

Also, you should be patching against ​WordPress trunk. Assuming you're talking about the manage_{$this->screen->taxonomy}_custom_column hook, the version in trunk has three arguments: https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/class-wp-terms-list-table.php#L509

Can you please clarify what you're trying to accomplish and where you think the bug is?

[Cybr]

Hi @DrewAPicture,

The file I uploaded is where the bug resides. I'll need to take some time to see how .diff uploading works.

It's named manage_{$this->screen->taxonomy}_custom_column as you correctly assumed.

Corresponding to class-wp-posts-list-table, function column_default doesn't return a value but rather does an action.

At line 530 and 531 you can see the changes.

I'll see if I can find a way to make it both backwards compatible and where the first two parameters are set correctly.

[SergeyBiryukov]

Replying to Cybr:

I'll need to take some time to see how .diff uploading works.

Please take a look at ​https://make.wordpress.org/core/handbook/tutorials/trac/submitting-a-patch/.

[Cybr]

In the example in the first post, add_action( "manage_{$type}_columns", array( $this, 'add_column'), 10, 1 ); now accepts $slug too (category, post_tag).

With the note of being backwards compatible (as per since WP2.1.9), it accepts both edit-category as category.

It now also accepts both the action and the filter for manage_{$this->screen->taxonomy}_custom_column. Needs further testing, but it should work as intended without duplicate outputs.

[SirLouen]

Replying to Cybr:

In the example in the first post, add_action( "manage_{$type}_columns", array( $this, 'add_column'), 10, 1 ); now accepts $slug too (category, post_tag).

With the note of being backwards compatible (as per since WP2.1.9), it accepts both edit-category as category.

It now also accepts both the action and the filter for manage_{$this->screen->taxonomy}_custom_column. Needs further testing, but it should work as intended without duplicate outputs.

After inspecting the code, the problem I find and probably happend to @DrewAPicture back in the day, is that is hard to figure out something at a first glance without going deeper into your context

My recommendation for this kind of scenarios is to ​build a Testing Use Case, in this case, multiple, because we must be covering the different scenarios that these 3 hooks are providing.