From c4b8ad087f0b25d6e09d1a1b8a8dc96d184ef41e Mon Sep 17 00:00:00 2001 From: Scott Taylor Date: Sun, 30 Aug 2015 02:40:40 +0000 Subject: [PATCH] Improve the reliability of the crop returned by `image_get_intermediate_size()`. Add a bunch of unit tests to `tests/image/intermediate_size.php`. Props joemcgill, ericlewis, kitchin, SergeyBiryukov, chipbennett. Fixes #17626. git-svn-id: https://develop.svn.wordpress.org/trunk@33807 602fd350-edb4-49c9-b593-d223f7449a82 --- src/wp-includes/media.php | 45 ++-- .../phpunit/tests/image/intermediate_size.php | 194 ++++++++++++++++++ 2 files changed, 220 insertions(+), 19 deletions(-) diff --git a/src/wp-includes/media.php b/src/wp-includes/media.php index 2c426bf729..aafeb20981 100644 --- a/src/wp-includes/media.php +++ b/src/wp-includes/media.php @@ -607,35 +607,42 @@ function image_get_intermediate_size( $post_id, $size = 'thumbnail' ) { // get the best one for a specified set of dimensions if ( is_array($size) && !empty($imagedata['sizes']) ) { - $areas = array(); + $candidates = array(); foreach ( $imagedata['sizes'] as $_size => $data ) { - // already cropped to width or height; so use this size - if ( ( $data['width'] == $size[0] && $data['height'] <= $size[1] ) || ( $data['height'] == $size[1] && $data['width'] <= $size[0] ) ) { + // If there's an exact match to an existing image size, short circuit. + if ( $data['width'] == $size[0] && $data['height'] == $size[1] ) { $file = $data['file']; list($width, $height) = image_constrain_size_for_editor( $data['width'], $data['height'], $size ); return compact( 'file', 'width', 'height' ); } - // add to lookup table: area => size - $areas[$data['width'] * $data['height']] = $_size; + // If it's not an exact match but it's at least the dimensions requested. + if ( $data['width'] >= $size[0] && $data['height'] >= $size[1] ) { + $candidates[ $data['width'] * $data['height'] ] = $_size; + } } - if ( !$size || !empty($areas) ) { + + if ( ! empty( $candidates ) ) { // find for the smallest image not smaller than the desired size - ksort($areas); - foreach ( $areas as $_size ) { + ksort( $candidates ); + foreach ( $candidates as $_size ) { $data = $imagedata['sizes'][$_size]; - if ( $data['width'] >= $size[0] || $data['height'] >= $size[1] ) { - // Skip images with unexpectedly divergent aspect ratios (crops) - // First, we calculate what size the original image would be if constrained to a box the size of the current image in the loop - $maybe_cropped = image_resize_dimensions($imagedata['width'], $imagedata['height'], $data['width'], $data['height'], false ); - // If the size doesn't match within one pixel, then it is of a different aspect ratio, so we skip it, unless it's the thumbnail size - if ( 'thumbnail' != $_size && ( !$maybe_cropped || ( $maybe_cropped[4] != $data['width'] && $maybe_cropped[4] + 1 != $data['width'] ) || ( $maybe_cropped[5] != $data['height'] && $maybe_cropped[5] + 1 != $data['height'] ) ) ) - continue; - // If we're still here, then we're going to use this size - $file = $data['file']; - list($width, $height) = image_constrain_size_for_editor( $data['width'], $data['height'], $size ); - return compact( 'file', 'width', 'height' ); + + // Skip images with unexpectedly divergent aspect ratios (crops) + // First, we calculate what size the original image would be if constrained to a box the size of the current image in the loop + $maybe_cropped = image_resize_dimensions($imagedata['width'], $imagedata['height'], $data['width'], $data['height'], false ); + // If the size doesn't match within one pixel, then it is of a different aspect ratio, so we skip it, unless it's the thumbnail size + if ( 'thumbnail' != $_size && + ( ! $maybe_cropped + || ( $maybe_cropped[4] != $data['width'] && $maybe_cropped[4] + 1 != $data['width'] ) + || ( $maybe_cropped[5] != $data['height'] && $maybe_cropped[5] + 1 != $data['height'] ) + ) ) { + continue; } + // If we're still here, then we're going to use this size + $file = $data['file']; + list( $width, $height ) = image_constrain_size_for_editor( $data['width'], $data['height'], $size ); + return compact( 'file', 'width', 'height' ); } } } diff --git a/tests/phpunit/tests/image/intermediate_size.php b/tests/phpunit/tests/image/intermediate_size.php index 73728e53a0..4458f6a76d 100644 --- a/tests/phpunit/tests/image/intermediate_size.php +++ b/tests/phpunit/tests/image/intermediate_size.php @@ -10,6 +10,40 @@ class Tests_Image_Intermediate_Size extends WP_UnitTestCase { parent::tearDown(); } + /** + * Upload files and create attachements for testing + */ + private function _make_attachment( $file, $parent_post_id = 0 ) { + $contents = file_get_contents( $file ); + $upload = wp_upload_bits( basename( $file ), null, $contents ); + + $type = ''; + if ( ! empty( $upload['type'] ) ) { + $type = $upload['type']; + } else { + $mime = wp_check_filetype( $upload['file'] ); + if ( $mime ) { + $type = $mime['type']; + } + } + + $attachment = array( + 'post_title' => basename( $upload['file'] ), + 'post_content' => '', + 'post_type' => 'attachment', + 'post_parent' => $parent_post_id, + 'post_mime_type' => $type, + 'guid' => $upload['url'], + ); + + // Save the data + $id = wp_insert_attachment( $attachment, $upload[ 'file' ], $parent_post_id ); + wp_update_attachment_metadata( $id, wp_generate_attachment_metadata( $id, $upload['file'] ) ); + + $this->ids[] = $id; + return $id; + } + function test_make_intermediate_size_no_size() { $image = image_make_intermediate_size( DIR_TESTDATA . '/images/a2-small.jpg', 0, 0, false ); @@ -49,4 +83,164 @@ class Tests_Image_Intermediate_Size extends WP_UnitTestCase { unlink( DIR_TESTDATA . '/images/a2-small-100x75.jpg' ); } + + /** + * @ticket 17626 + */ + function test_get_intermediate_sizes_by_name() { + add_image_size( 'test-size', 330, 220, true ); + + $file = DIR_TESTDATA . '/images/waffles.jpg'; + $id = $this->_make_attachment( $file, 0 ); + + // look for a size by name + $image = image_get_intermediate_size( $id, 'test-size' ); + + // test for the expected string because the array will by definition + // return with the correct height and width attributes + $this->assertNotFalse( strpos( $image['file'], '330x220' ) ); + + // cleanup + remove_image_size( 'test-size' ); + } + + /** + * @ticket 17626 + */ + function test_get_intermediate_sizes_by_array_exact() { + // Only one dimention match shouldn't return false positive (see: 17626) + add_image_size( 'test-size', 330, 220, true ); + add_image_size( 'false-height', 330, 400, true ); + add_image_size( 'false-width', 600, 220, true ); + + $file = DIR_TESTDATA . '/images/waffles.jpg'; + $id = $this->_make_attachment( $file, 0 ); + + // look for a size by array that exists + // note: staying larger than 300px to miss default medium crop + $image = image_get_intermediate_size( $id, array( 330, 220 ) ); + + // test for the expected string because the array will by definition + // return with the correct height and width attributes + $this->assertNotFalse( strpos( $image['file'], '330x220' ) ); + + // cleanup + remove_image_size( 'test-size' ); + remove_image_size( 'false-height' ); + remove_image_size( 'false-width' ); + } + + /** + * @ticket 17626 + */ + function test_get_intermediate_sizes_by_array_nearest() { + // If an exact size is not found, it should be returned + // If not, find nearest size that is larger (see: 17626) + add_image_size( 'test-size', 450, 300, true ); + add_image_size( 'false-height', 330, 100, true ); + add_image_size( 'false-width', 150, 220, true ); + + $file = DIR_TESTDATA . '/images/waffles.jpg'; + $id = $this->_make_attachment( $file, 0 ); + + // look for a size by array that doesn't exist + // note: staying larger than 300px to miss default medium crop + $image = image_get_intermediate_size( $id, array( 330, 220 ) ); + + // you have to test for the string because the image will by definition + // return with the correct height and width attributes + $this->assertNotFalse( strpos( $image['file'], '450x300' ) ); + + // cleanup + remove_image_size( 'test-size' ); + remove_image_size( 'false-height' ); + remove_image_size( 'false-width' ); + } + + /** + * @ticket 17626 + */ + function test_get_intermediate_sizes_by_array_nearest_false() { + // If an exact size is not found, it should be returned + // If not, find nearest size that is larger, otherwise return false (see: 17626) + add_image_size( 'false-height', 330, 100, true ); + add_image_size( 'false-width', 150, 220, true ); + + $file = DIR_TESTDATA . '/images/waffles.jpg'; + $id = $this->_make_attachment( $file, 0 ); + + // look for a size by array that doesn't exist + // note: staying larger than 300px to miss default medium crop + $image = image_get_intermediate_size( $id, array( 330, 220 ) ); + + // you have to test for the string because the image will by definition + // return with the correct height and width attributes + $this->assertFalse( $image ); + + // cleanup + remove_image_size( 'false-height' ); + remove_image_size( 'false-width' ); + } + + /** + * @ticket 17626 + */ + function test_get_intermediate_sizes_by_array_zero_height() { + // Generate random width + $random_w = rand( 300, 400 ); + + // Only one dimention match shouldn't return false positive (see: 17626) + add_image_size( 'test-size', $random_w, 0, false ); + add_image_size( 'false-height', $random_w, 100, true ); + + $file = DIR_TESTDATA . '/images/waffles.jpg'; + $id = $this->_make_attachment( $file, 0 ); + + $original = wp_get_attachment_metadata( $id ); + $image_w = $random_w; + $image_h = round( ( $image_w / $original['width'] ) * $original['height'] ); + + // look for a size by array that exists + // note: staying larger than 300px to miss default medium crop + $image = image_get_intermediate_size( $id, array( $random_w, 0 ) ); + + // test for the expected string because the array will by definition + // return with the correct height and width attributes + $this->assertNotFalse( strpos( $image['file'], $image_w . 'x' . $image_h ) ); + + // cleanup + remove_image_size( 'test-size' ); + remove_image_size( 'false-height' ); + } + + /** + * @ticket 17626 + */ + function test_get_intermediate_sizes_by_array_zero_width() { + // Generate random height + $random_h = rand( 200, 300 ); + + // Only one dimention match shouldn't return false positive (see: 17626) + add_image_size( 'test-size', 0, $random_h, false ); + add_image_size( 'false-height', 300, $random_h, true ); + + $file = DIR_TESTDATA . '/images/waffles.jpg'; + $id = $this->_make_attachment( $file, 0 ); + + $original = wp_get_attachment_metadata( $id ); + $image_h = $random_h; + $image_w = round( ( $image_h / $original['height'] ) * $original['width'] ); + + // look for a size by array that exists + // note: staying larger than 300px to miss default medium crop + $image = image_get_intermediate_size( $id, array( 0, $random_h ) ); + + // test for the expected string because the array will by definition + // return with the correct height and width attributes + $this->assertNotFalse( strpos( $image['file'], $image_w . 'x' . $image_h ) ); + + // cleanup + remove_image_size( 'test-size' ); + remove_image_size( 'false-height' ); + } }