From 0ed174a306e826ccd70f89ded914840e4342dde8 Mon Sep 17 00:00:00 2001
From: Mark Holmquist <mtraceur@member.fsf.org>
Date: Mon, 14 Apr 2014 17:01:03 -0700
Subject: [PATCH] Do not load too-big thumbnails for SVGs

Change-Id: Iae75105151bfcd0e974fc292794802c77eb26ea4
Mingle: https://wikimedia.mingle.thoughtworks.com/projects/multimedia/cards/448
---
 resources/mmv/mmv.js                     |  9 +++++++--
 resources/mmv/ui/mmv.ui.canvas.js        | 15 +--------------
 tests/qunit/mmv/mmv.test.js              | 13 +++++++++++++
 tests/qunit/mmv/ui/mmv.ui.canvas.test.js |  8 ++++----
 4 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/resources/mmv/mmv.js b/resources/mmv/mmv.js
index 0acb63125..e55288b2b 100755
--- a/resources/mmv/mmv.js
+++ b/resources/mmv/mmv.js
@@ -569,14 +569,19 @@
 	 * @returns {jQuery.Promise.<mw.mmv.model.Thumbnail, HTMLImageElement>}
 	 */
 	MMVP.fetchThumbnail = function ( fileTitle, width, originalWidth, originalHeight ) {
-		$.noop( originalWidth, originalHeight ); // keep JSHint happy... will be removed later
+		$.noop( originalHeight ); // keep JSHint happy... will be removed later
 		var viewer = this,
 			thumbnailPromise,
 			imagePromise;
 
+		if ( originalWidth && width > originalWidth ) {
+			// Do not request images larger than the original image
+			width = originalWidth;
+		}
+
 		thumbnailPromise = this.thumbnailInfoProvider.get( fileTitle, width );
 
-		imagePromise = thumbnailPromise.then( function( thumbnail ) {
+		imagePromise = thumbnailPromise.then( function ( thumbnail ) {
 			return viewer.imageProvider.get( thumbnail.url );
 		} );
 
diff --git a/resources/mmv/ui/mmv.ui.canvas.js b/resources/mmv/ui/mmv.ui.canvas.js
index 80d6725ae..e08f5ec4f 100644
--- a/resources/mmv/ui/mmv.ui.canvas.js
+++ b/resources/mmv/ui/mmv.ui.canvas.js
@@ -186,20 +186,7 @@
 		var targetWidth,
 			targetHeight,
 			blowupFactor,
-			blurredThumbnailShown = false,
-			maxSizeFileExtensions = {
-				'svg' : true,
-			};
-
-		// There are some file types (SVG for example) for which there is no concept
-		// of initial size. For these cases we force a max canvas resize and no bluring.
-		if ( maxSizeFileExtensions[ this.imageRawMetadata.filePageTitle.getExtension().toLowerCase() ] ) {
-			$imagePlaceholder.width( imageWidths.cssWidth );
-			$imagePlaceholder.height( imageWidths.cssHeight );
-			this.set( this.imageRawMetadata, $imagePlaceholder.show() );
-
-			return blurredThumbnailShown;
-		}
+			blurredThumbnailShown = false;
 
 		// Assume natural thumbnail size¸
 		targetWidth = imageInfo.width;
diff --git a/tests/qunit/mmv/mmv.test.js b/tests/qunit/mmv/mmv.test.js
index aef1ac61f..37c6a26a1 100644
--- a/tests/qunit/mmv/mmv.test.js
+++ b/tests/qunit/mmv/mmv.test.js
@@ -363,4 +363,17 @@
 			}
 		}
 	} );
+
+	QUnit.test( 'Refuse to load too-big thumbnails', 1, function ( assert ) {
+		var viewer = new mw.mmv.MultimediaViewer(),
+			intendedWidth = 50,
+			title = mw.Title.newFromText( 'File:Foobar.svg' );
+
+		viewer.thumbnailInfoProvider.get = function ( fileTitle, width ) {
+			assert.strictEqual( width, intendedWidth );
+			return $.Deferred().reject();
+		};
+
+		viewer.fetchThumbnail( title, 1000, intendedWidth, 60 );
+	} );
 }( mediaWiki, jQuery ) );
diff --git a/tests/qunit/mmv/ui/mmv.ui.canvas.test.js b/tests/qunit/mmv/ui/mmv.ui.canvas.test.js
index 4945b7251..689560253 100644
--- a/tests/qunit/mmv/ui/mmv.ui.canvas.test.js
+++ b/tests/qunit/mmv/ui/mmv.ui.canvas.test.js
@@ -111,7 +111,7 @@
 		assert.ok( ! canvas.resizeListener, 'resize listener has been removed.' );
 	} );
 
-	QUnit.test( 'maybeDisplayPlaceholder: Max area for SVG files', 5, function ( assert ) {
+	QUnit.test( 'maybeDisplayPlaceholder: Constrained area for SVG files', 4, function ( assert ) {
 		var $image,
 			blurredThumbnailShown,
 			$qf = $( '#qunit-fixture' ),
@@ -124,7 +124,7 @@
 		canvas.imageRawMetadata = imageRawMetadata;
 
 		canvas.set = function () {
-			assert.ok ( true, 'Placeholder is shown');
+			assert.ok ( false, 'Placeholder is not shown');
 		};
 
 		$image = $( '<img>' ).width( 10 ).height( 5 );
@@ -135,8 +135,8 @@
 			{ cssWidth : 300, cssHeight: 150 }
 		);
 
-		assert.strictEqual( $image.width(), 300, 'Placeholder width was set to max' );
-		assert.strictEqual( $image.height(), 150, 'Placeholder height was set to max' );
+		assert.strictEqual( $image.width(), 10, 'Placeholder width was not set to max' );
+		assert.strictEqual( $image.height(), 5, 'Placeholder height was not set to max' );
 		assert.ok( ! $image.hasClass( 'blurred' ), 'Placeholder is not blurred' );
 		assert.ok( ! blurredThumbnailShown, 'Placeholder state is correct' );
 	} );