Make WordPress Core

Opened 3 years ago

Last modified 7 months ago

#44990 new defect (bug)

Unhandle post_ids variable in custom bulk actions

Reported by: gaupoit Owned by:
Milestone: Awaiting Review Priority: normal
Severity: minor Version: 4.9.8
Component: Quick/Bulk Edit Keywords: dev-feedback 2nd-opinion
Focuses: Cc:


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.

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

Change History (7)

#1 @garrett-eclipse
3 years ago

  • Keywords ux-feedback added

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;

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

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;

#2 follow-up: @gaupoit
3 years ago

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.

Last edited 3 years ago by garrett-eclipse (previous) (diff)

#3 in reply to: ↑ 2 @garrett-eclipse
3 years ago

  • Keywords 2nd-opinion added

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;

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;
*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;

#4 @karmatosed
3 years ago

  • Keywords ux-feedback removed

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.

This ticket was mentioned in Slack in #core-media by antpb. View the logs.

7 months ago

#6 @joedolson
7 months ago

  • Component changed from Media to Quick/Bulk Edit

#7 @antpb
7 months ago

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!

Note: See TracTickets for help on using tickets.