Make WordPress Core

Opened 9 years ago

Last modified 5 years ago

#31570 assigned defect (bug)

Infinite loop when filtering Media Library images by size in a modal (using wp_prepare_attachment_for_js)

Reported by: silb3r's profile silb3r Owned by: fuhton's profile fuhton
Milestone: Priority: normal
Severity: normal Version: 4.1.1
Component: Media Keywords: has-patch dev-feedback
Focuses: javascript, administration Cc:

Description

In an attempt to restrict a post's Featured Image dimensions to imagers wider than 100px I implement the following code in functions.php:

function restrict_media_library_by_width($response, $attachment, $meta) {
  if(isset($response['width']) && isset($response['height']) && $response['width'] > 100) {
    return $response;
  }
  return false;
}
add_filter('wp_prepare_attachment_for_js', 'restrict_media_library_by_width', 10, 3);

I then click "Set featured image" and the Media Library modal that appears only loads one empty thumbnail and my Network panel in Chrome Dev Tools reveals it makes continued, repeated, infinite AJAX requests.

The only viable alternative I've found was to run a separate query within ajax_query_attachments_args, which is needed because the _wp_attachment_metadata key contains serialized data and that leaves no way to compare dimensions within a meta_query.

Obviously running this additional query is inefficient and more resource intensive than it should be. More details here: http://wordpress.stackexchange.com/questions/180500/filter-media-library-items-by-size/.

Attachments (3)

31570.diff (475 bytes) - added by fuhton 8 years ago.
array_filter removes any values that are false, but doesn't reset the array indexes and will eventually cause ajax response errors.
31570.2.diff (3.0 KB) - added by adamsilverstein 8 years ago.
refresh patch from trunk
31570.3.diff (487 bytes) - added by fuhton 8 years ago.
Refresh from trunk, but with only necessary changes

Download all attachments as: .zip

Change History (9)

#1 @wonderboymusic
9 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release

This could use a deeper look

#2 @adamsilverstein
8 years ago

  • Owner set to fuhton
  • Status changed from new to assigned

@fuhton
8 years ago

array_filter removes any values that are false, but doesn't reset the array indexes and will eventually cause ajax response errors.

#3 @fuhton
8 years ago

  • Keywords has-patch dev-feedback added; needs-patch removed

#4 @adamsilverstein
8 years ago

Thanks @fuhton for your patch! We certainly need more help with these JavaScript related tickets. This seems like a reasonable fix. Before your fix we were getting a malformed array in the callback, right?

I tested this fix, verified the issue is fixed. I've attached screencasts before and after your patch below.

ps. In the future, please generate your patch one level up, so its easier to apply (this is how we expect them).

Before:

http://cl.ly/image/1X0r2K0j0t0B/Screen%20Recording%202015-12-05%20at%2002.55%20PM.gif

After:

http://cl.ly/image/34122x281p12/Screen%20Recording%202015-12-05%20at%2002.58%20PM.gif

@adamsilverstein
8 years ago

refresh patch from trunk

#5 follow-up: @fuhton
8 years ago

@adamsilverstein Awesome! Looks like your patch is also including a change to twenty-eleven ( the code that was causing this issue )? Was that intentional?

@fuhton
8 years ago

Refresh from trunk, but with only necessary changes

#6 in reply to: ↑ 5 @adamsilverstein
8 years ago

Replying to fuhton:

@adamsilverstein Awesome! Looks like your patch is also including a change to twenty-eleven ( the code that was causing this issue )? Was that intentional?

nope! whooops, thanks for catching that :) 31570.3.diff looks good.

Note: See TracTickets for help on using tickets.