Fix mmv qunit tests

This heavily relied on deferreds getting resolved synchroneously,
which (for .then) is no longer the case with jQuery 3.
There's also a difference in how chained .then's get resolved,
and $.when no longer propagates notify().

The changes in here are basically:
* fix use of $.when, manually passing along notify()
* use .then in some places, instead of .done, .fail, .progress
* fix progress bar hiding in setupProgressBar, which assumed sync
* fixed tests, mostly by using fake timers to give async stuff a
  chance to execute

Bug: T164473
Change-Id: Ib51ddd8bc6254a477861588fb16f57565353afe1

resources/mmv/mmv.js

@@ -288,7 +288,9 @@
 
 		metadataPromise = this.fetchSizeIndependentLightboxInfo( image.filePageTitle );
 
-		imagePromise.done( function ( thumbnail, imageElement ) {
+		imagePromise.then(
+			// done
+			function ( thumbnail, imageElement ) {
 				if ( viewer.currentIndex !== image.index ) {
 					return;
 				}
@@ -296,30 +298,38 @@
 				if ( viewer.imageDisplayedCount++ === 0 ) {
 					mw.mmv.durationLogger.stop( 'click-to-first-image' );
 
-				metadataPromise.done( function ( imageInfo ) {
+					metadataPromise.then( function ( imageInfo, repoInfo ) {
-					if ( !imageInfo || !imageInfo.anonymizedUploadDateTime ) {
+						if ( imageInfo && imageInfo.anonymizedUploadDateTime ) {
-						return;
-					}
-
 							mw.mmv.durationLogger.record( 'click-to-first-image', {
 								uploadTimestamp: imageInfo.anonymizedUploadDateTime
 							} );
+						}
+
+						return $.Deferred().resolve( imageInfo, repoInfo );
 					} );
 				}
 
 				imageElement.className = 'mw-mmv-final-image ' + image.filePageTitle.ext.toLowerCase();
 				imageElement.alt = image.alt;
 
-			$.when( metadataPromise, pluginsPromise ).done( function ( metadata ) {
+				$.when( metadataPromise, pluginsPromise ).then( function ( metadata ) {
 					$( document ).trigger( $.Event( 'mmv-metadata', { viewer: viewer, image: image, imageInfo: metadata[ 0 ] } ) );
 				} );
 
 				viewer.displayRealThumbnail( thumbnail, imageElement, imageWidths, $.now() - start );
-		} ).fail( function ( error ) {
-			viewer.ui.canvas.showError( error );
-		} );
 
-		metadataPromise.done( function ( imageInfo, repoInfo ) {
+				return $.Deferred().resolve( thumbnail, imageElement );
+			},
+			// fail
+			function ( error ) {
+				viewer.ui.canvas.showError( error );
+				return $.Deferred().reject( error );
+			}
+		);
+
+		metadataPromise.then(
+			// done
+			function ( imageInfo, repoInfo ) {
 				extraStatsDeferred.resolve( { uploadTimestamp: imageInfo.anonymizedUploadDateTime } );
 
 				if ( viewer.currentIndex !== image.index ) {
@@ -334,17 +344,22 @@
 
 				// File reuse steals a bunch of information from the DOM, so do it last
 				viewer.ui.setFileReuseData( imageInfo, repoInfo, image.caption, image.alt );
-		} ).fail( function ( error ) {
+
+				return $.Deferred().resolve( imageInfo, repoInfo );
+			},
+			// fail
+			function ( error ) {
 				extraStatsDeferred.reject();
 
-			if ( viewer.currentIndex !== image.index ) {
+				if ( viewer.currentIndex === image.index ) {
-				return;
-			}
-
 					// Set title to caption or file name if caption is not available;
 					// see setTitle() in mmv.ui.metadataPanel for extended caption fallback
 					viewer.ui.panel.showError( image.caption || image.filePageTitle.getNameText(), error );
-		} );
+				}
+
+				return $.Deferred().reject( error );
+			}
+		);
 
 		$.when( imagePromise, metadataPromise ).then( function () {
 			if ( viewer.currentIndex !== image.index ) {
@@ -507,12 +522,6 @@
 			progressBar = viewer.ui.panel.progressBar,
 			key = image.filePageTitle.getPrefixedDb() + '|' + imageWidth;
 
-		if ( imagePromise.state() !== 'pending' ) {
-			// image has already loaded (or failed to load) - do not show the progress bar
-			progressBar.hide();
-			return;
-		}
-
 		if ( !this.progressCache[ key ] ) {
 			// Animate progress bar to 5 to give a sense that something is happening, and make sure
 			// the progress bar is noticeable, even if we're sitting at 0% stuck waiting for
@@ -525,34 +534,47 @@
 		}
 
 		// FIXME would be nice to have a "filtered" promise which does not fire when the image is not visible
-		imagePromise.progress( function ( progress ) {
+		imagePromise.then(
-			// We pretend progress is always at least 5%, so progress events below 5% should be ignored
+			// done
-			// 100 will be handled by the done handler, do not mix two animations
+			function ( thumbnail, imageElement ) {
-			if ( progress < 5 || progress === 100 ) {
-				return;
-			}
-
-			viewer.progressCache[ key ] = progress;
-
-			// Touch the UI only if the user is looking at this image
-			if ( viewer.currentIndex === image.index ) {
-				progressBar.animateTo( progress );
-			}
-		} ).done( function () {
 				viewer.progressCache[ key ] = 100;
-
 				if ( viewer.currentIndex === image.index ) {
 					// Fallback in case the browser doesn't have fancy progress updates
 					progressBar.animateTo( 100 );
+
+					// Hide progress bar, we're done
+					progressBar.hide();
 				}
-		} ).fail( function () {
+
+				return $.Deferred().resolve( thumbnail, imageElement );
+			},
+			// fail
+			function ( error ) {
 				viewer.progressCache[ key ] = 100;
 
 				if ( viewer.currentIndex === image.index ) {
 					// Hide progress bar on error
 					progressBar.hide();
 				}
-		} );
+
+				return $.Deferred().reject( error );
+			},
+			// progress
+			function ( progress ) {
+				// We pretend progress is always at least 5%, so progress events below 5% should be ignored
+				// 100 will be handled by the done handler, do not mix two animations
+				if ( progress >= 5 && progress < 100 ) {
+					viewer.progressCache[ key ] = progress;
+
+					// Touch the UI only if the user is looking at this image
+					if ( viewer.currentIndex === image.index ) {
+						progressBar.animateTo( progress );
+					}
+				}
+
+				return progress;
+			}
+		);
 	};
 
 	/**
@@ -762,6 +784,7 @@
 	MMVP.fetchThumbnail = function ( fileTitle, width, sampleUrl, originalWidth, originalHeight, extraStatsDeferred ) {
 		var viewer = this,
 			guessing = false,
+			combinedDeferred = $.Deferred(),
 			thumbnailPromise,
 			imagePromise;
 
@@ -806,10 +829,13 @@
 			} );
 		}
 
-		return $.when( thumbnailPromise, imagePromise ).then( null, null, function ( thumbnailProgress, imageProgress ) {
+		// In jQuery<3, $.when used to also relay notify, but that is no longer
-			// Make progress events have a nice format.
+		// the case - but we still want to pass it along...
-			return imageProgress[ 1 ];
+		$.when( thumbnailPromise, imagePromise ).then( combinedDeferred.resolve, combinedDeferred.reject );
+		imagePromise.then( null, null, function ( arg, progress ) {
+			combinedDeferred.notify( progress );
 		} );
+		return combinedDeferred;
 	};
 
 	/**

tests/qunit/mmv/mmv.test.js

@@ -117,7 +117,9 @@
 			fakeImage = {
 				filePageTitle: new mw.Title( 'File:Stuff.jpg' ),
 				extraStatsDeferred: $.Deferred().reject()
-			};
+			},
+			// custom clock ensures progress handlers execute in correct sequence
+			clock = this.sandbox.useFakeTimers();
 
 		viewer.thumbs = [];
 		viewer.displayPlaceholderThumbnail = $.noop;
@@ -150,21 +152,27 @@
 		viewer.imageInfoProvider.get = function () { return $.Deferred().resolve( {} ); };
 		viewer.thumbnailInfoProvider.get = function () { return $.Deferred().resolve( {} ); };
 
+		// loadImage will call setupProgressBar, which will attach done, fail &
+		// progress handlers
 		viewer.loadImage( fakeImage, new Image() );
+		clock.tick( 10 );
 		assert.ok( viewer.ui.panel.progressBar.jumpTo.lastCall.calledWith( 0 ),
 			'Percentage correctly reset by loadImage' );
-
+		assert.ok( viewer.ui.panel.progressBar.animateTo.firstCall.calledWith( 5 ),
-		assert.ok( viewer.ui.panel.progressBar.animateTo.lastCall.calledWith( 5 ),
 			'Percentage correctly animated to 5 by loadImage' );
 
 		imageDeferred.notify( 'response', 45 );
-		assert.ok( viewer.ui.panel.progressBar.animateTo.lastCall.calledWith( 45 ),
+		clock.tick( 10 );
+		assert.ok( viewer.ui.panel.progressBar.animateTo.secondCall.calledWith( 45 ),
 			'Percentage correctly funneled to panel UI' );
 
 		imageDeferred.resolve( {}, {} );
-		assert.ok( viewer.ui.panel.progressBar.animateTo.lastCall.calledWith( 100 ),
+		clock.tick( 10 );
+		assert.ok( viewer.ui.panel.progressBar.animateTo.thirdCall.calledWith( 100 ),
 			'Percentage correctly funneled to panel UI' );
 
+		clock.restore();
+
 		viewer.close();
 	} );
 
@@ -181,7 +189,9 @@
 				filePageTitle: new mw.Title( 'File:Second.jpg' ),
 				extraStatsDeferred: $.Deferred().reject()
 			},
-			viewer = mw.mmv.testHelpers.getMultimediaViewer();
+			viewer = mw.mmv.testHelpers.getMultimediaViewer(),
+			// custom clock ensures progress handlers execute in correct sequence
+			clock = this.sandbox.useFakeTimers();
 
 		// animation would keep running, conflict with other tests
 		this.sandbox.stub( $.fn, 'animate' ).returnsThis();
@@ -223,60 +233,65 @@
 		// load some image
 		viewer.imageProvider.get = this.sandbox.stub().returns( firstImageDeferred );
 		viewer.loadImage( firstImage, new Image() );
-
+		clock.tick( 10 );
-		assert.ok( viewer.ui.panel.progressBar.jumpTo.lastCall.calledWith( 0 ),
+		assert.ok( viewer.ui.panel.progressBar.jumpTo.getCall( 0 ).calledWith( 0 ),
 			'Percentage correctly reset for new first image' );
-		assert.ok( viewer.ui.panel.progressBar.animateTo.lastCall.calledWith( 5 ),
+		assert.ok( viewer.ui.panel.progressBar.animateTo.getCall( 0 ).calledWith( 5 ),
 			'Percentage correctly animated to 5 for first new image' );
 
+		// progress on active image
 		firstImageDeferred.notify( 'response', 20 );
-
+		clock.tick( 10 );
-		assert.ok( viewer.ui.panel.progressBar.animateTo.lastCall.calledWith( 20 ),
+		assert.ok( viewer.ui.panel.progressBar.animateTo.getCall( 1 ).calledWith( 20 ),
 			'Percentage correctly animated when active image is loading' );
 
 		// change to another image
 		viewer.imageProvider.get = this.sandbox.stub().returns( secondImageDeferred );
 		viewer.loadImage( secondImage, new Image() );
-
+		clock.tick( 10 );
-		assert.ok( viewer.ui.panel.progressBar.jumpTo.lastCall.calledWith( 0 ),
+		assert.ok( viewer.ui.panel.progressBar.jumpTo.getCall( 1 ).calledWith( 0 ),
 			'Percentage correctly reset for second new image' );
-		assert.ok( viewer.ui.panel.progressBar.animateTo.lastCall.calledWith( 5 ),
+		assert.ok( viewer.ui.panel.progressBar.animateTo.getCall( 2 ).calledWith( 5 ),
 			'Percentage correctly animated to 5 for second new image' );
 
+		// progress on active image
 		secondImageDeferred.notify( 'response', 30 );
-
+		clock.tick( 10 );
-		assert.ok( viewer.ui.panel.progressBar.animateTo.lastCall.calledWith( 30 ),
+		assert.ok( viewer.ui.panel.progressBar.animateTo.getCall( 3 ).calledWith( 30 ),
 			'Percentage correctly animated when active image is loading' );
 
-		// this is the most convenient way of checking for new calls - just reset() and check called
+		// progress on inactive image
-		viewer.ui.panel.progressBar.animateTo.reset();
-		viewer.ui.panel.progressBar.jumpTo.reset();
-
 		firstImageDeferred.notify( 'response', 40 );
-
+		clock.tick( 10 );
-		assert.ok( !viewer.ui.panel.progressBar.animateTo.called,
+		assert.ok( viewer.ui.panel.progressBar.animateTo.callCount === 4,
 			'Percentage not animated when inactive image is loading' );
-		assert.ok( !viewer.ui.panel.progressBar.jumpTo.called,
-			'Percentage not changed when inactive image is loading' );
 
+		// progress on active image
 		secondImageDeferred.notify( 'response', 50 );
+		clock.tick( 10 );
+		assert.ok( viewer.ui.panel.progressBar.animateTo.getCall( 4 ).calledWith( 50 ),
+			'Percentage correctly ignored inactive image & only animated when active image is loading' );
 
 		// change back to first image
+		viewer.imageProvider.get = this.sandbox.stub().returns( firstImageDeferred );
 		viewer.loadImage( firstImage, new Image() );
-
+		clock.tick( 10 );
-		assert.ok( viewer.ui.panel.progressBar.jumpTo.lastCall.calledWith( 40 ),
+		assert.ok( viewer.ui.panel.progressBar.jumpTo.getCall( 2 ).calledWith( 40 ),
 			'Percentage jumps to right value when changing images' );
 
 		secondImageDeferred.resolve( {}, {} );
+		clock.tick( 10 );
 		assert.ok( !viewer.ui.panel.progressBar.hide.called,
 			'Progress bar not hidden when something finishes in the background' );
 
-		// change to second image which has finished loading
+		// change back to second image, which has finished loading
 		viewer.imageProvider.get = this.sandbox.stub().returns( secondImageDeferred );
 		viewer.loadImage( secondImage, new Image() );
-
+		clock.tick( 10 );
 		assert.ok( viewer.ui.panel.progressBar.hide.called,
-			'Progress bar not hidden when switching to finished image' );
+			'Progress bar hidden when switching to finished image' );
+
+		clock.restore();
 
 		viewer.close();
 	} );
@@ -400,7 +415,9 @@
 				filePageTitle: new mw.Title( 'File:Bar.jpg' ),
 				index: 1,
 				extraStatsDeferred: $.Deferred().reject()
-			};
+			},
+			// custom clock ensures progress handlers execute in correct sequence
+			clock = this.sandbox.useFakeTimers();
 
 		viewer.preloadFullscreenThumbnail = $.noop;
 		viewer.fetchSizeIndependentLightboxInfo = this.sandbox.stub();
@@ -435,25 +452,32 @@
 		viewer.imageProvider.get.returns( firstImageDeferred.promise() );
 		viewer.fetchSizeIndependentLightboxInfo.returns( firstLigthboxInfoDeferred.promise() );
 		viewer.loadImage( firstImage, new Image() );
+		clock.tick( 10 );
 		assert.ok( !viewer.animateMetadataDivOnce.called, 'Metadata of the first image should not be animated' );
 		assert.ok( !viewer.ui.panel.setImageInfo.called, 'Metadata of the first image should not be shown' );
 
 		viewer.imageProvider.get.returns( secondImageDeferred.promise() );
 		viewer.fetchSizeIndependentLightboxInfo.returns( secondLigthboxInfoDeferred.promise() );
 		viewer.loadImage( secondImage, new Image() );
+		clock.tick( 10 );
 
 		viewer.ui.panel.progressBar.animateTo.reset();
 		firstImageDeferred.notify( undefined, 45 );
+		clock.tick( 10 );
 		assert.ok( !viewer.ui.panel.progressBar.animateTo.reset.called, 'Progress of the first image should not be shown' );
 
 		firstImageDeferred.resolve( {}, {} );
 		firstLigthboxInfoDeferred.resolve( {} );
+		clock.tick( 10 );
 		assert.ok( !viewer.displayRealThumbnail.called, 'The first image being done loading should have no effect' );
 
 		viewer.displayRealThumbnail = this.sandbox.spy( function () { viewer.close(); } );
 		secondImageDeferred.resolve( {}, {} );
 		secondLigthboxInfoDeferred.resolve( {} );
+		clock.tick( 10 );
 		assert.ok( viewer.displayRealThumbnail.called, 'The second image being done loading should result in the image being shown' );
+
+		clock.restore();
 	} );
 
 	QUnit.test( 'Events are not trapped after the viewer is closed', 0, function ( assert ) {
@@ -558,7 +582,9 @@
 			width = 100,
 			originalWidth = 1000,
 			originalHeight = 1000,
-			image = {};
+			image = {},
+			// custom clock ensures progress handlers execute in correct sequence
+			clock = this.sandbox.useFakeTimers();
 
 		function setupStubs() {
 			guessedThumbnailInfoStub = viewer.guessedThumbnailInfoProvider.get = sandbox.stub();
@@ -574,6 +600,7 @@
 		thumbnailInfoStub.returns( $.Deferred().resolve( { url: 'apiURL' } ) );
 		imageStub.returns( $.Deferred().resolve( image ) );
 		promise = viewer.fetchThumbnail( file, width );
+		clock.tick( 10 );
 		assert.ok( !guessedThumbnailInfoStub.called, 'When we lack sample URL and original dimensions, GuessedThumbnailInfoProvider is not called' );
 		assert.ok( thumbnailInfoStub.calledOnce, 'When we lack sample URL and original dimensions, ThumbnailInfoProvider is called once' );
 		assert.ok( imageStub.calledOnce, 'When we lack sample URL and original dimensions, ImageProvider is called once' );
@@ -586,6 +613,7 @@
 		thumbnailInfoStub.returns( $.Deferred().resolve( { url: 'apiURL' } ) );
 		imageStub.returns( $.Deferred().resolve( image ) );
 		promise = viewer.fetchThumbnail( file, width, sampleURL, originalWidth, originalHeight );
+		clock.tick( 10 );
 		assert.ok( guessedThumbnailInfoStub.calledOnce, 'When the guesser bails out, GuessedThumbnailInfoProvider is called once' );
 		assert.ok( thumbnailInfoStub.calledOnce, 'When the guesser bails out, ThumbnailInfoProvider is called once' );
 		assert.ok( imageStub.calledOnce, 'When the guesser bails out, ImageProvider is called once' );
@@ -598,6 +626,7 @@
 		thumbnailInfoStub.returns( $.Deferred().resolve( { url: 'apiURL' } ) );
 		imageStub.returns( $.Deferred().resolve( image ) );
 		promise = viewer.fetchThumbnail( file, width, sampleURL, originalWidth, originalHeight );
+		clock.tick( 10 );
 		assert.ok( guessedThumbnailInfoStub.calledOnce, 'When the guesser returns an URL, GuessedThumbnailInfoProvider is called once' );
 		assert.ok( !thumbnailInfoStub.called, 'When the guesser returns an URL, ThumbnailInfoProvider is not called' );
 		assert.ok( imageStub.calledOnce, 'When the guesser returns an URL, ImageProvider is called once' );
@@ -611,6 +640,7 @@
 		imageStub.withArgs( 'guessedURL' ).returns( $.Deferred().reject() );
 		imageStub.withArgs( 'apiURL' ).returns( $.Deferred().resolve( image ) );
 		promise = viewer.fetchThumbnail( file, width, sampleURL, originalWidth, originalHeight );
+		clock.tick( 10 );
 		assert.ok( guessedThumbnailInfoStub.calledOnce, 'When the guesser returns an URL, but that returns 404, GuessedThumbnailInfoProvider is called once' );
 		assert.ok( thumbnailInfoStub.calledOnce, 'When the guesser returns an URL, but that returns 404, ThumbnailInfoProvider is called once' );
 		assert.ok( imageStub.calledTwice, 'When the guesser returns an URL, but that returns 404, ImageProvider is called twice' );
@@ -625,6 +655,7 @@
 		imageStub.withArgs( 'guessedURL' ).returns( $.Deferred().reject() );
 		imageStub.withArgs( 'apiURL' ).returns( $.Deferred().reject() );
 		promise = viewer.fetchThumbnail( file, width, sampleURL, originalWidth, originalHeight );
+		clock.tick( 10 );
 		assert.ok( guessedThumbnailInfoStub.calledOnce, 'When even the retry fails, GuessedThumbnailInfoProvider is called once' );
 		assert.ok( thumbnailInfoStub.calledOnce, 'When even the retry fails, ThumbnailInfoProvider is called once' );
 		assert.ok( imageStub.calledTwice, 'When even the retry fails, ImageProvider is called twice' );
@@ -640,11 +671,14 @@
 		thumbnailInfoStub.returns( $.Deferred().resolve( { url: 'apiURL' } ) );
 		imageStub.returns( $.Deferred().resolve( image ) );
 		promise = viewer.fetchThumbnail( file, width );
+		clock.tick( 10 );
 		assert.ok( !guessedThumbnailInfoStub.called, 'When guessing is disabled, GuessedThumbnailInfoProvider is not called' );
 		assert.ok( thumbnailInfoStub.calledOnce, 'When guessing is disabled, ThumbnailInfoProvider is called once' );
 		assert.ok( imageStub.calledOnce, 'When guessing is disabled, ImageProvider is called once' );
 		assert.ok( imageStub.calledWith( 'apiURL' ), 'When guessing is disabled, ImageProvider is called with the API url' );
 		assert.strictEqual( promise.state(), 'resolved', 'When guessing is disabled, fetchThumbnail resolves' );
+
+		clock.restore();
 	} );
 
 	QUnit.test( 'document.title', 2, function ( assert ) {
@@ -664,5 +698,4 @@
 
 		assert.strictEqual( document.title, oldDocumentTitle, 'Original title restored after viewer is closed' );
 	} );
-
 }( mediaWiki, jQuery ) );