Unhandle post_ids variable in custom bulk actions

[gaupoit]

This error happened when creating a custom bulk actions by using filter "handle_bulk_actions-" then user don't choose any items to apply that custom bulk action. The reason is in ~/wp-admin/upload.php line 170, the post_ids variable has never defined or handled.

<?php
default:
/** This action is documented in wp-admin/edit-comments.php */
$location = apply_filters( 'handle_bulk_actions-' . get_current_screen()->id, $location, $doaction, $post_ids );

[garrett-eclipse]

Thanks @gaupoit welcome to Trac.

This seems to coincide with #45006 to handle Bulk Actions when no items selected.

When reviewing this it appears the trash, untrash and delete actions in the switch case all do isset checks on $post_ids;
​https://github.com/WordPress/WordPress/blob/master/wp-admin/upload.php#L149
​https://github.com/WordPress/WordPress/blob/master/wp-admin/upload.php#L170
​https://github.com/WordPress/WordPress/blob/master/wp-admin/upload.php#L185

So in the interim you can update your custom code to similarly check $post_ids and simply return $location;;

<?php
if ( ! isset( $post_ids ) ) {
    return $location;
}

And for easy reference as the code you referred to is no longer on line 170 in master here's a permalink to 4.9.8;
​https://github.com/WordPress/WordPress/blob/4.9.8/wp-admin/upload.php#L170

[gaupoit]

Thanks, @garrett-eclipse for the reply. I also added the checking for $post_ids you mentioned in my custom code. However, the problem is here ​https://github.com/WordPress/WordPress/blob/4.9.8/wp-admin/upload.php#L170 the variable $post_ids is never handled.

[garrett-eclipse]

Thanks @gaupoit, I tweaked your comment to switch from master branch making it a permalink as master often changes.

Replying to gaupoit:

However, the problem is here ​https://github.com/WordPress/WordPress/blob/4.9.8/wp-admin/upload.php#L170 the variable $post_ids is never handled.

By unhandled I'm assuming you're speaking of the isset check on the $post_ids array which isn't done before firing the action? If I misunderstood please correct me. But with that in mind.

Checking the other actions - ​https://github.com/WordPress/WordPress/search?q=handle_bulk_actions-&type=Code
Only the /wp-admin/edit.php does a global check before entering the conditional logic;
​https://github.com/WordPress/WordPress/blob/56c162fbc9867f923862f64f1b4570d885f1ff03/wp-admin/edit.php#L99-L102

Other instances either use the foreach around the switch statements or use an isset within the switch statement before the foreach.

In most cases it seems the check is avoided prior to the action to allow it to fire even when the ids array is empty. I'm unsure if that's intended by design so adding 2nd-opinion. And wanted to flag it's existing use in Plugins;
​https://wpdirectory.net/search/01CS1BY2N61SNYPJBRVJXX5N7C
*If any do actions when they receive an empty array then introducing a check in core would avoid the action and result in those actions never firing. This is my only concern with adding a global check, it would force a global redirect location without providing the ability to plugins/etc to override that.

And just for reference, the docblock associated to this action is found here;
​https://github.com/WordPress/WordPress/blob/4.9.8/wp-admin/edit-comments.php#L85-L103

[karmatosed]

Looking at this, not surer that it needs ux-feedback. If that is the case please add back in. For now, I am removing that keyword.

[antpb]

This ticket looks to have been added to the Media component but it seems this is best fit for Quick/Bulk Edit. Moving along to get proper visibility on this!

[marybaum]

In the scrub: @davidb says, " I'll track down the commits here and see if my hunch is correct. I'll ping the reporter to see if it's still an issue."

[davidbaumwald]

@gaupoit It looks like this was resolved in r51111. I'm going to flag this as a close candidate, but would like your feedback about whether the issue is fixed for you in the latest version of WordPress.

[hellofromTonya]

Re-reading the ticket and specifically @gaupoit comment here:

Thanks, @garrett-eclipse for the reply. I also added the checking for $post_ids you mentioned in my custom code. However, the problem is here ​https://github.com/WordPress/WordPress/blob/4.9.8/wp-admin/upload.php#L170 the variable $post_ids is never handled.

I agree with @davidbaumwald that [51111] should have resolved the reported issue. Why? It initialized $post_ids to an empty array before the code goes into the switch() of where @gaupoit noted the issue.

I think this ticket can be marked as fixed (by #39589 / [51111]). Given that it's been 10 months with no reporter feedback, thinking it's safe to close the ticket as resolved.

If however this problem persists, @gaupoit please reopen the ticket and provide more information to help contributors further investigate.