Cover both width and height for the sidebar preview

3 files changed, 72 insertions, 25 deletions

diff --git a/apps/files/js/mainfileinfodetailview.js b/apps/files/js/mainfileinfodetailview.js
index 82cca0d0fb3..c15b1fb2d63 100644
--- a/apps/files/js/mainfileinfodetailview.js
+++ b/apps/files/js/mainfileinfodetailview.js
@@ -140,7 +140,8 @@
 		},
 
 		loadPreview: function(path, mime, etag, $iconDiv, $container, isImage) {
-			var maxImageHeight = ($container.parent().width() + 50) / (16/9); // 30px for negative margin
+			var maxImageWidth  = $container.parent().width() + 50;  // 50px for negative margins
+			var maxImageHeight = maxImageWidth / (16/9);
 			var smallPreviewSize = 75;
 
 			var isLandscape = function(img) {
@@ -164,8 +165,9 @@
 				mime: mime,
 				etag: etag,
 				y: isImage ? maxImageHeight : smallPreviewSize,
-				x: isImage ? 99999 /* only limit on y */ : smallPreviewSize,
+				x: isImage ? maxImageWidth : smallPreviewSize,
 				a: isImage ? 1 : null,
+				mode: isImage ? 'cover' : null,
 				callback: function (previewUrl, img) {
 					$iconDiv.previewImg = previewUrl;
 
diff --git a/lib/private/preview.php b/lib/private/preview.php
index de964b72df2..cd92f15b921 100644
--- a/lib/private/preview.php
+++ b/lib/private/preview.php
@@ -837,14 +837,6 @@ class Preview {
 		$askedWidth = $this->getMaxX();
 		$askedHeight = $this->getMaxY();
 
-		/**
-		 * Phase 1: If required, adjust boundaries to keep aspect ratio
-		 */
-		if ($this->keepAspect) {
-			list($askedWidth, $askedHeight) =
-				$this->applyAspectRatio($askedWidth, $askedHeight, $previewWidth, $previewHeight);
-		}
-
 		if ($this->mode === self::MODE_COVER) {
 			list($scaleWidth, $scaleHeight) =
 				$this->applyCover($askedWidth, $askedHeight, $previewWidth, $previewHeight);
@@ -854,6 +846,14 @@ class Preview {
 		}
 
 		/**
+		 * Phase 1: If required, adjust boundaries to keep aspect ratio
+		 */
+		if ($this->keepAspect) {
+			list($scaleWidth, $scaleHeight) =
+				$this->applyAspectRatio($scaleWidth, $scaleHeight, $previewWidth, $previewHeight);
+		}
+
+		/**
 		 * Phase 2: Resizes preview to try and match requirements.
 		 * Takes the scaling ratio into consideration
 		 */
@@ -870,26 +870,30 @@ class Preview {
 
 		/**
 		 * Phase 3: We're still not there yet, so we're clipping and filling
-		 * to match the asked dimensions
+		 * to match the asked dimensions if we're not asked to keep aspect ratio
 		 */
-		// It turns out the scaled preview is now too big, so we crop the image
-		if ($newPreviewWidth >= $askedWidth && $newPreviewHeight >= $askedHeight) {
-			$this->crop($image, $askedWidth, $askedHeight, $newPreviewWidth, $newPreviewHeight);
-			$this->storePreview($fileId, $askedWidth, $askedHeight);
 
-			return;
-		}
+		if (!$this->keepAspect) {
+			// It turns out the scaled preview is now too big, so we crop the image
+			if ($newPreviewWidth >= $askedWidth && $newPreviewHeight >= $askedHeight) {
+				$this->crop($image, $askedWidth, $askedHeight, $newPreviewWidth, $newPreviewHeight);
+				$this->storePreview($fileId, $askedWidth, $askedHeight);
 
-		// At least one dimension of the scaled preview is too small,
-		// so we fill the space with a transparent background
-		if (($newPreviewWidth < $askedWidth || $newPreviewHeight < $askedHeight)) {
-			$this->cropAndFill(
-				$image, $askedWidth, $askedHeight, $newPreviewWidth, $newPreviewHeight
-			);
-			$this->storePreview($fileId, $askedWidth, $askedHeight);
+				return;
+			}
 
-			return;
+			// At least one dimension of the scaled preview is too small,
+			// so we fill the space with a transparent background
+			if (($newPreviewWidth < $askedWidth || $newPreviewHeight < $askedHeight)) {
+				$this->cropAndFill(
+					$image, $askedWidth, $askedHeight, $newPreviewWidth, $newPreviewHeight
+				);
+				$this->storePreview($fileId, $askedWidth, $askedHeight);
+
+				return;
+			}
 		}
+
 		// The preview is smaller, but we can't touch it
 		$this->storePreview($fileId, $newPreviewWidth, $newPreviewHeight);
 	}
diff --git a/tests/lib/preview.php b/tests/lib/preview.php
index 9e118014bac..a135ed40d0a 100644
--- a/tests/lib/preview.php
+++ b/tests/lib/preview.php
@@ -874,4 +874,45 @@ class Preview extends TestCase {
 
 		return [(int)$askedWidth, (int)$askedHeight];
 	}
+
+	public function testKeepAspectRatio() {
+		$originalWidth = 1680;
+		$originalHeight = 1050;
+		$originalAspectRation = $originalWidth / $originalHeight;
+
+		$preview = new \OC\Preview(
+			self::TEST_PREVIEW_USER1, 'files/', 'testimage.jpg',
+			150,
+			150
+		);
+		$preview->setKeepAspect(true);
+		$image = $preview->getPreview();
+
+		$aspectRatio = $image->width() / $image->height();
+		$this->assertEquals(round($originalAspectRation, 2), round($aspectRatio, 2));
+
+		$this->assertLessThanOrEqual(150, $image->width());
+		$this->assertLessThanOrEqual(150, $image->height());
+	}
+
+	public function testKeepAspectRatioCover() {
+		$originalWidth = 1680;
+		$originalHeight = 1050;
+		$originalAspectRation = $originalWidth / $originalHeight;
+
+		$preview = new \OC\Preview(
+			self::TEST_PREVIEW_USER1, 'files/', 'testimage.jpg',
+			150,
+			150
+		);
+		$preview->setKeepAspect(true);
+		$preview->setMode(\OC\Preview::MODE_COVER);
+		$image = $preview->getPreview();
+
+		$aspectRatio = $image->width() / $image->height();
+		$this->assertEquals(round($originalAspectRation, 2), round($aspectRatio, 2));
+
+		$this->assertGreaterThanOrEqual(150, $image->width());
+		$this->assertGreaterThanOrEqual(150, $image->height());
+	}
 }