WordPress.org

Make WordPress Core

Opened 8 months ago

Last modified 8 months ago

#39589 new defect (bug)

PHP Notice if nothing selected on custom bulk action in Media Library.

Reported by: gitlost Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.7
Component: Media Keywords: has-patch needs-testing needs-docs
Focuses: Cc:

Description

If you add a custom bulk action to the Media Library:

<?php
add_filter( 'bulk_actions-upload', function( $actions ) {
        $actions['foo'] = 'Foo';
        return $actions;
} );

Then if nothing selected when you apply "Foo" (and you have WP_DEBUG on) you'll get a

PHP Notice:  Undefined variable: post_ids in .../wp-admin/upload.php on line 168

Attachments (2)

39589.patch (713 bytes) - added by achbed 8 months ago.
39589_b.patch (633 bytes) - added by achbed 8 months ago.
Revised based on feedback

Download all attachments as: .zip

Change History (6)

@achbed
8 months ago

#1 @achbed
8 months ago

  • Keywords has-patch needs-testing needs-docs added

This is a first pass at fixing this. This version does change behavior in two key ways.

1) It always runs the filter, even for those cases where there is nothing selected. In this case, it passes an empty array for $post_ids. This allows the filter to put up an appropriate error message.

2) It also checks the filter return value. If the filter returns false, null, or empty string, it will use the previously defined $location instead.

This behavior change may be unexpected to some. We should discuss these changes to make sure these are appropriate.

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


8 months ago

#3 @gitlost
8 months ago

Looking at [38647] none of the other handle_bulk_actions filters checks the return so perhaps it's best to leave it as is and just do the isset() check (which should be before the comment). Also I don't think it constitutes a change of behaviour as it always runs now - adding the check just saves one having to do something similar in the handler...

@achbed
8 months ago

Revised based on feedback

#4 @achbed
8 months ago

Patch is revised to not check the return value, but blindly assign to $location, matching behavior of non-filtered handlers.

Note: See TracTickets for help on using tickets.