Issue #1338428 by jbrown, BTMash, Tor Arne Thune: Fixed image_dimensions_scale() and image_scale_effect() are ungrokable and buggy.

2 files changed, 143 insertions, 4 deletions

diff --git a/modules/image/image.effects.inc b/modules/image/image.effects.inc
index ea898f91f..35a6a74c7 100644
--- a/modules/image/image.effects.inc
+++ b/modules/image/image.effects.inc
@@ -124,13 +124,11 @@ function image_resize_dimensions(array &$dimensions, array $data) {
 function image_scale_effect(&$image, $data) {
   // Set sane default values.
   $data += array(
+    'width' => NULL,
+    'height' => NULL,
     'upscale' => FALSE,
   );
 
-  // Set impossibly large values if the width and height aren't set.
-  $data['width'] = empty($data['width']) ? PHP_INT_MAX : $data['width'];
-  $data['height'] = empty($data['height']) ? PHP_INT_MAX : $data['height'];
-
   if (!image_scale($image, $data['width'], $data['height'], $data['upscale'])) {
     watchdog('image', 'Image scale failed using the %toolkit toolkit on %path (%mimetype, %dimensions)', array('%toolkit' => $image->toolkit, '%path' => $image->source, '%mimetype' => $image->info['mime_type'], '%dimensions' => $image->info['width'] . 'x' . $image->info['height']), WATCHDOG_ERROR);
     return FALSE;
diff --git a/modules/image/image.test b/modules/image/image.test
index a29b4f3a1..c9a709033 100644
--- a/modules/image/image.test
+++ b/modules/image/image.test
@@ -1129,3 +1129,144 @@ class ImageDimensionsUnitTest extends DrupalWebTestCase {
     $this->assertEqual($img_tag, '<img typeof="foaf:Image" src="' . $url . '" alt="" />', t('Expected img tag was found.'));
   }
 }
+
+/**
+ * Tests image_dimensions_scale().
+ */
+class ImageDimensionsScaleTestCase extends DrupalUnitTestCase {
+  public static function getInfo() {
+    return array(
+      'name' => 'image_dimensions_scale()',
+      'description' => 'Tests all control flow branches in image_dimensions_scale().',
+      'group' => 'Image',
+    );
+  }
+
+  /**
+   * Tests all control flow branches in image_dimensions_scale().
+   */
+  function testImageDimensionsScale() {
+    // Define input / output datasets to test different branch conditions.
+    $test = array();
+
+    // Test branch conditions:
+    // - No height.
+    // - Upscale, don't need to upscale.
+    $tests[] = array(
+      'input' => array(
+        'dimensions' => array(
+          'width' => 1000,
+          'height' => 2000,
+        ),
+        'width' => 200,
+        'height' => NULL,
+        'upscale' => TRUE,
+      ),
+      'output' => array(
+        'dimensions' => array(
+          'width' => 200,
+          'height' => 400,
+        ),
+        'return_value' => TRUE,
+      ),
+    );
+
+    // Test branch conditions:
+    // - No width.
+    // - Don't upscale, don't need to upscale.
+    $tests[] = array(
+      'input' => array(
+        'dimensions' => array(
+          'width' => 1000,
+          'height' => 800,
+        ),
+        'width' => NULL,
+        'height' => 140,
+        'upscale' => FALSE,
+      ),
+      'output' => array(
+        'dimensions' => array(
+          'width' => 175,
+          'height' => 140,
+        ),
+        'return_value' => TRUE,
+      ),
+    );
+
+    // Test branch conditions:
+    // - Source aspect ratio greater than target.
+    // - Upscale, need to upscale.
+    $tests[] = array(
+      'input' => array(
+        'dimensions' => array(
+          'width' => 8,
+          'height' => 20,
+        ),
+        'width' => 200,
+        'height' => 140,
+        'upscale' => TRUE,
+      ),
+      'output' => array(
+        'dimensions' => array(
+          'width' => 56,
+          'height' => 140,
+        ),
+        'return_value' => TRUE,
+      ),
+    );
+
+    // Test branch condition: target aspect ratio greater than source.
+    $tests[] = array(
+      'input' => array(
+        'dimensions' => array(
+          'width' => 2000,
+          'height' => 800,
+        ),
+        'width' => 200,
+        'height' => 140,
+        'upscale' => FALSE,
+      ),
+      'output' => array(
+        'dimensions' => array(
+          'width' => 200,
+          'height' => 80,
+        ),
+        'return_value' => TRUE,
+      ),
+    );
+
+    // Test branch condition: don't upscale, need to upscale.
+    $tests[] = array(
+      'input' => array(
+        'dimensions' => array(
+          'width' => 100,
+          'height' => 50,
+        ),
+        'width' => 200,
+        'height' => 140,
+        'upscale' => FALSE,
+      ),
+      'output' => array(
+        'dimensions' => array(
+          'width' => 100,
+          'height' => 50,
+        ),
+        'return_value' => FALSE,
+      ),
+    );
+
+    foreach ($tests as $test) {
+      // Process the test dataset.
+      $return_value = image_dimensions_scale($test['input']['dimensions'], $test['input']['width'], $test['input']['height'], $test['input']['upscale']);
+
+      // Check the width.
+      $this->assertEqual($test['output']['dimensions']['width'], $test['input']['dimensions']['width'], t('Computed width (@computed_width) equals expected width (@expected_width)', array('@computed_width' => $test['output']['dimensions']['width'], '@expected_width' => $test['input']['dimensions']['width'])));
+
+      // Check the height.
+      $this->assertEqual($test['output']['dimensions']['height'], $test['input']['dimensions']['height'], t('Computed height (@computed_height) equals expected height (@expected_height)', array('@computed_height' => $test['output']['dimensions']['height'], '@expected_height' => $test['input']['dimensions']['height'])));
+
+      // Check the return value.
+      $this->assertEqual($test['output']['return_value'], $return_value, t('Correct return value.'));
+    }
+  }
+}