Opened 9 years ago
Last modified 6 years ago
#33521 new defect (bug)
manage_${post_type}_posts_columns parameters shifted
Reported by: | Cybr | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | minor | Version: | 4.3 |
Component: | Taxonomy | Keywords: | reporter-feedback has-patch needs-testing |
Focuses: | administration | Cc: |
Description
Hello,
manage_${post_type}_posts_columns requires three parameters, however, as shown in the link below, it only uses 2.
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();
Attachments (2)
Change History (11)
#2
in reply to:
↑ 1
@
9 years ago
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.
#4
@
9 years ago
- Keywords 2nd-opinion added
- Severity changed from normal to minor
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.
#5
@
9 years ago
- Keywords needs-patch added; has-patch needs-testing 2nd-opinion removed
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?
#6
follow-up:
↓ 7
@
9 years ago
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.
#7
in reply to:
↑ 6
@
9 years ago
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/.
This ticket was mentioned in Slack in #core by drew. View the logs.
9 years ago
#9
@
9 years ago
- Keywords has-patch needs-testing added; needs-patch removed
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.
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 themanage_{$post->post_type}_posts_custom_column
hook call in the source, and finally, in your example, referencedmanage_{$slug}_custom_column
where$slug
would need to beposts
orpages
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.