<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[38086] trunk: Media: Prevent `image_get_intermediate_size()` from returning cropped images.</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta" style="font-size: 105%">
<dt style="float: left; width: 6em; font-weight: bold">Revision</dt> <dd><a style="font-weight: bold" href="https://core.trac.wordpress.org/changeset/38086">38086</a><script type="application/ld+json">{"@context":"http://schema.org","@type":"EmailMessage","description":"Review this Commit","action":{"@type":"ViewAction","url":"https://core.trac.wordpress.org/changeset/38086","name":"Review Commit"}}</script></dd>
<dt style="float: left; width: 6em; font-weight: bold">Author</dt> <dd>joemcgill</dd>
<dt style="float: left; width: 6em; font-weight: bold">Date</dt> <dd>2016-07-18 02:13:45 +0000 (Mon, 18 Jul 2016)</dd>
</dl>
<pre style='padding-left: 1em; margin: 2em 0; border-left: 2px solid #ccc; line-height: 1.25; font-size: 105%; font-family: sans-serif'>Media: Prevent `image_get_intermediate_size()` from returning cropped images.
When `$size` is passed to `image_get_intermediate_size()` as an array of width
and height values and an exact image size matching those values isn't available,
the function loops through the available attachment sizes and returns the
smallest image larger than the requested dimensions with the same aspect ratio.
The aspect ratio check is skipped for the 'thumbnail' size to provide a fallback
for small sizes when no other image option is available. This resulted in a poor
selection when the size requested was smaller than the 'thumbnail' dimensions
but a larger size matching the requested ratio existed.
This refactors the internals of `image_get_intermediate_size()` to ensure the
'thumbnail' size is only returned as a fallback to small sizes once all other
options have been considered, and makes the control flow easier to follow.
This also introduces a new helper function, `wp_image_matches_ratio()` for
testing whether the aspect ratios of two sets of dimensions match. This function
is also now used in `wp_calculate_image_srcset()` during the selection process.
Props flixos, joemcgill.
Fixes <a href="https://core.trac.wordpress.org/ticket/34384">#34384</a>, <a href="https://core.trac.wordpress.org/ticket/34980">#34980</a>.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunksrcwpincludesmediaphp">trunk/src/wp-includes/media.php</a></li>
<li><a href="#trunktestsphpunittestsimageintermediate_sizephp">trunk/tests/phpunit/tests/image/intermediate_size.php</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunksrcwpincludesmediaphp"></a>
<div class="modfile"><h4 style="background-color: #eee; color: inherit; margin: 1em 0; padding: 1.3em; font-size: 115%">Modified: trunk/src/wp-includes/media.php</h4>
<pre class="diff"><span>
<span class="info" style="display: block; padding: 0 10px; color: #888">--- trunk/src/wp-includes/media.php 2016-07-17 23:29:11 UTC (rev 38085)
+++ trunk/src/wp-includes/media.php 2016-07-18 02:13:45 UTC (rev 38086)
</span><span class="lines" style="display: block; padding: 0 10px; color: #888">@@ -592,6 +592,36 @@
</span><span class="cx" style="display: block; padding: 0 10px"> }
</span><span class="cx" style="display: block; padding: 0 10px">
</span><span class="cx" style="display: block; padding: 0 10px"> /**
</span><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ * Helper function to test if aspect ratios for two images match.
+ *
+ * @since 4.6.0
+ *
+ * @param int $source_width Width of the first image in pixels.
+ * @param int $source_height Height of the first image in pixels.
+ * @param int $target_width Width of the second image in pixels.
+ * @param int $target_height Height of the second image in pixels.
+ * @return bool True if aspect ratios match within 1px. False if not.
+ */
+function wp_image_matches_ratio( $source_width, $source_height, $target_width, $target_height ) {
+ /*
+ * To test for varying crops, we constrain the dimensions of the larger image
+ * to the dimensions of the smaller image and see if they match.
+ */
+ if ( $source_width > $target_width ) {
+ $constrained_size = wp_constrain_dimensions( $source_width, $source_height, $target_width );
+ $expected_size = array( $target_width, $target_height );
+ } else {
+ $constrained_size = wp_constrain_dimensions( $target_width, $target_height, $source_width );
+ $expected_size = array( $source_width, $source_height );
+ }
+
+ // If the image dimensions are within 1px of the expected size, we consider it a match.
+ $matched = ( abs( $constrained_size[0] - $expected_size[0] ) <= 1 && abs( $constrained_size[1] - $expected_size[1] ) <= 1 );
+
+ return $matched;
+}
+
+/**
</ins><span class="cx" style="display: block; padding: 0 10px"> * Retrieves the image's intermediate size (resized) path, width, and height.
</span><span class="cx" style="display: block; padding: 0 10px"> *
</span><span class="cx" style="display: block; padding: 0 10px"> * The $size parameter can be an array with the width and height respectively.
</span><span class="lines" style="display: block; padding: 0 10px; color: #888">@@ -623,64 +653,73 @@
</span><span class="cx" style="display: block; padding: 0 10px"> * @type string $file Image's path relative to uploads directory
</span><span class="cx" style="display: block; padding: 0 10px"> * @type int $width Width of image
</span><span class="cx" style="display: block; padding: 0 10px"> * @type int $height Height of image
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- * @type string $path Optional. Image's absolute filesystem path. Only returned if registered
- * size is passed to `$size` parameter.
- * @type string $url Optional. Image's URL. Only returned if registered size is passed to `$size`
- * parameter.
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ * @type string $path Image's absolute filesystem path.
+ * @type string $url Image's URL.
</ins><span class="cx" style="display: block; padding: 0 10px"> * }
</span><span class="cx" style="display: block; padding: 0 10px"> */
</span><span class="cx" style="display: block; padding: 0 10px"> function image_get_intermediate_size( $post_id, $size = 'thumbnail' ) {
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- if ( !is_array( $imagedata = wp_get_attachment_metadata( $post_id ) ) )
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ if ( ! $size || ! is_array( $imagedata = wp_get_attachment_metadata( $post_id ) ) || empty( $imagedata['sizes'] ) ) {
</ins><span class="cx" style="display: block; padding: 0 10px"> return false;
</span><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ }
</ins><span class="cx" style="display: block; padding: 0 10px">
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- // get the best one for a specified set of dimensions
- if ( is_array($size) && !empty($imagedata['sizes']) ) {
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ $data = array();
+
+ // Find the best match when '$size' is an array.
+ if ( is_array( $size ) ) {
</ins><span class="cx" style="display: block; padding: 0 10px"> $candidates = array();
</span><span class="cx" style="display: block; padding: 0 10px">
</span><span class="cx" style="display: block; padding: 0 10px"> foreach ( $imagedata['sizes'] as $_size => $data ) {
</span><span class="cx" style="display: block; padding: 0 10px"> // If there's an exact match to an existing image size, short circuit.
</span><span class="cx" style="display: block; padding: 0 10px"> if ( $data['width'] == $size[0] && $data['height'] == $size[1] ) {
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- list( $data['width'], $data['height'] ) = image_constrain_size_for_editor( $data['width'], $data['height'], $size );
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ $candidates[ $data['width'] * $data['height'] ] = $data;
+ break;
+ }
</ins><span class="cx" style="display: block; padding: 0 10px">
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- /** This filter is documented in wp-includes/media.php */
- return apply_filters( 'image_get_intermediate_size', $data, $post_id, $size );
- }
- // If it's not an exact match but it's at least the dimensions requested.
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ // If it's not an exact match, consider larger sizes with the same aspect ratio.
</ins><span class="cx" style="display: block; padding: 0 10px"> if ( $data['width'] >= $size[0] && $data['height'] >= $size[1] ) {
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- $candidates[ $data['width'] * $data['height'] ] = $_size;
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ // If '0' is passed to either size, we test ratios against the original file.
+ if ( 0 === $size[0] || 0 === $size[1] ) {
+ $same_ratio = wp_image_matches_ratio( $data['width'], $data['height'], $imagedata['width'], $imagedata['height'] );
+ } else {
+ $same_ratio = wp_image_matches_ratio( $data['width'], $data['height'], $size[0], $size[1] );
+ }
+
+ if ( $same_ratio ) {
+ $candidates[ $data['width'] * $data['height'] ] = $data;
+ }
</ins><span class="cx" style="display: block; padding: 0 10px"> }
</span><span class="cx" style="display: block; padding: 0 10px"> }
</span><span class="cx" style="display: block; padding: 0 10px">
</span><span class="cx" style="display: block; padding: 0 10px"> if ( ! empty( $candidates ) ) {
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- // find for the smallest image not smaller than the desired size
- ksort( $candidates );
- foreach ( $candidates as $_size ) {
- $data = $imagedata['sizes'][$_size];
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ // Sort the array by size if we have more than one candidate.
+ if ( 1 < count( $candidates ) ) {
+ ksort( $candidates );
+ }
</ins><span class="cx" style="display: block; padding: 0 10px">
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- // 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.
- list( $data['width'], $data['height'] ) = image_constrain_size_for_editor( $data['width'], $data['height'], $size );
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ $data = array_shift( $candidates );
+ /*
+ * When the size requested is smaller than the thumbnail dimensions, we
+ * fall back to the thumbnail size to maintain backwards compatibility with
+ * pre 4.6 versions of WordPress.
+ */
+ } elseif ( ! empty( $imagedata['sizes']['thumbnail'] ) && $imagedata['sizes']['thumbnail']['width'] >= $size[0] && $imagedata['sizes']['thumbnail']['width'] >= $size[1] ) {
+ $data = $imagedata['sizes']['thumbnail'];
+ } else {
+ return false;
+ }
</ins><span class="cx" style="display: block; padding: 0 10px">
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- /** This filter is documented in wp-includes/media.php */
- return apply_filters( 'image_get_intermediate_size', $data, $post_id, $size );
- }
- }
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ // Constrain the width and height attributes to the requested values.
+ list( $data['width'], $data['height'] ) = image_constrain_size_for_editor( $data['width'], $data['height'], $size );
+
+ } elseif ( ! empty( $imagedata['sizes'][ $size ] ) ) {
+ $data = $imagedata['sizes'][ $size ];
</ins><span class="cx" style="display: block; padding: 0 10px"> }
</span><span class="cx" style="display: block; padding: 0 10px">
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- if ( is_array($size) || empty($size) || empty($imagedata['sizes'][$size]) )
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ // If we still don't have a match at this point, return false.
+ if ( empty( $data ) ) {
</ins><span class="cx" style="display: block; padding: 0 10px"> return false;
</span><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ }
</ins><span class="cx" style="display: block; padding: 0 10px">
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- $data = $imagedata['sizes'][$size];
</del><span class="cx" style="display: block; padding: 0 10px"> // include the full filesystem path of the intermediate file
</span><span class="cx" style="display: block; padding: 0 10px"> if ( empty($data['path']) && !empty($data['file']) ) {
</span><span class="cx" style="display: block; padding: 0 10px"> $file_url = wp_get_attachment_url($post_id);
</span><span class="lines" style="display: block; padding: 0 10px; color: #888">@@ -1092,21 +1131,8 @@
</span><span class="cx" style="display: block; padding: 0 10px"> continue;
</span><span class="cx" style="display: block; padding: 0 10px"> }
</span><span class="cx" style="display: block; padding: 0 10px">
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- /**
- * To check for varying crops, we calculate the expected size of the smaller
- * image if the larger were constrained by the width of the smaller and then
- * see if it matches what we're expecting.
- */
- if ( $image_width > $image['width'] ) {
- $constrained_size = wp_constrain_dimensions( $image_width, $image_height, $image['width'] );
- $expected_size = array( $image['width'], $image['height'] );
- } else {
- $constrained_size = wp_constrain_dimensions( $image['width'], $image['height'], $image_width );
- $expected_size = array( $image_width, $image_height );
- }
-
</del><span class="cx" style="display: block; padding: 0 10px"> // If the image dimensions are within 1px of the expected size, use it.
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- if ( abs( $constrained_size[0] - $expected_size[0] ) <= 1 && abs( $constrained_size[1] - $expected_size[1] ) <= 1 ) {
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ if ( wp_image_matches_ratio( $image_width, $image_height, $image['width'], $image['height'] ) ) {
</ins><span class="cx" style="display: block; padding: 0 10px"> // Add the URL, descriptor, and value to the sources array to be returned.
</span><span class="cx" style="display: block; padding: 0 10px"> $source = array(
</span><span class="cx" style="display: block; padding: 0 10px"> 'url' => $image_baseurl . $image['file'],
</span></span></pre></div>
<a id="trunktestsphpunittestsimageintermediate_sizephp"></a>
<div class="modfile"><h4 style="background-color: #eee; color: inherit; margin: 1em 0; padding: 1.3em; font-size: 115%">Modified: trunk/tests/phpunit/tests/image/intermediate_size.php</h4>
<pre class="diff"><span>
<span class="info" style="display: block; padding: 0 10px; color: #888">--- trunk/tests/phpunit/tests/image/intermediate_size.php 2016-07-17 23:29:11 UTC (rev 38085)
+++ trunk/tests/phpunit/tests/image/intermediate_size.php 2016-07-18 02:13:45 UTC (rev 38086)
</span><span class="lines" style="display: block; padding: 0 10px; color: #888">@@ -224,4 +224,38 @@
</span><span class="cx" style="display: block; padding: 0 10px">
</span><span class="cx" style="display: block; padding: 0 10px"> $this->assertTrue( strpos( $image['file'], $width . 'x' . $height ) > 0 );
</span><span class="cx" style="display: block; padding: 0 10px"> }
</span><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+
+ /**
+ * @ticket 34384
+ */
+ public function test_get_intermediate_size_with_small_size_array() {
+ // Add a hard cropped size that matches the aspect ratio we're going to test.
+ add_image_size( 'test-size', 200, 100, true );
+
+ $file = DIR_TESTDATA . '/images/waffles.jpg';
+ $id = $this->_make_attachment( $file, 0 );
+
+ // Request a size by array that doesn't exist and is smaller than the 'thumbnail'
+ $image = image_get_intermediate_size( $id, array( 50, 25 ) );
+
+ // We should get the 'test-size' file and not the thumbnail.
+ $this->assertTrue( strpos( $image['file'], '200x100' ) > 0 );
+ }
+
+ /**
+ * @ticket 34384
+ */
+ public function test_get_intermediate_size_with_small_size_array_fallback() {
+ $file = DIR_TESTDATA . '/images/waffles.jpg';
+ $id = $this->_make_attachment( $file, 0 );
+
+ $original = wp_get_attachment_metadata( $id );
+ $thumbnail_file = $original['sizes']['thumbnail']['file'];
+
+ // Request a size by array that doesn't exist and is smaller than the 'thumbnail'
+ $image = image_get_intermediate_size( $id, array( 50, 25 ) );
+
+ // We should get the 'thumbnail' file as a fallback.
+ $this->assertSame( $image['file'], $thumbnail_file );
+ }
</ins><span class="cx" style="display: block; padding: 0 10px"> }
</span></span></pre>
</div>
</div>
</body>
</html>