jenkins-bot has submitted this change and it was merged. Change subject: Do not load too-big thumbnails for SVGs ......................................................................
Do not load too-big thumbnails for SVGs Change-Id: Iae75105151bfcd0e974fc292794802c77eb26ea4 Mingle: https://wikimedia.mingle.thoughtworks.com/projects/multimedia/cards/448 --- M resources/mmv/mmv.js M resources/mmv/ui/mmv.ui.canvas.js M tests/qunit/mmv/mmv.test.js M tests/qunit/mmv/ui/mmv.ui.canvas.test.js 4 files changed, 25 insertions(+), 20 deletions(-) Approvals: Gilles: Looks good to me, approved jenkins-bot: Verified diff --git a/resources/mmv/mmv.js b/resources/mmv/mmv.js index 0acb631..e55288b 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 80d6725..e08f5ec 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 aef1ac6..37c6a26 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 4945b72..6895602 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' ); } ); -- To view, visit https://gerrit.wikimedia.org/r/125918 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: Iae75105151bfcd0e974fc292794802c77eb26ea4 Gerrit-PatchSet: 3 Gerrit-Project: mediawiki/extensions/MultimediaViewer Gerrit-Branch: master Gerrit-Owner: MarkTraceur <mtrac...@member.fsf.org> Gerrit-Reviewer: Gilles <gdu...@wikimedia.org> Gerrit-Reviewer: MarkTraceur <mtrac...@member.fsf.org> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
