From 3be5195ed2c365a4d78c128351c5afe5079cb76b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Tisza?= Date: Mon, 28 Apr 2014 23:20:57 +0000 Subject: [PATCH] Ensure click playback happens at the right time The code to replay clicks and clean up the handler was called after processing each thumbnail, instead of just once at the end. This might have caused many subtle issues such as clicks on any but the first image not replaying correctly; more problematically, of there were no MediaViewer-compatible thumbs, the handler was never cleaned up and the clicks were never replayed. Besides fixing that, this commit also adds a try..finally wrapper so that the click replaying is not broken even if there is an error in the thumb processing code. This might or might not be a good idea. The internets say that try..finally without a catch causes errors in IE8, but only if it is not wrapped in another try..catch, so we are probably fine. (Adding a catch which just rethrows the error would be an alternative, but it messes up stack traces.) Change-Id: I2f645762103274c92c15a0d4b595d18d93b08415 Mingle: https://wikimedia.mingle.thoughtworks.com/projects/multimedia/cards/497 Bug: 64345 --- resources/mmv/mmv.bootstrap.js | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/resources/mmv/mmv.bootstrap.js b/resources/mmv/mmv.bootstrap.js index 007617fe4..9c405467e 100755 --- a/resources/mmv/mmv.bootstrap.js +++ b/resources/mmv/mmv.bootstrap.js @@ -125,9 +125,18 @@ MMVB.processThumbs = function () { var bs = this; - this.$thumbs.each( function ( i, thumb ) { - bs.processThumb( thumb ); - } ); + // if this breaks in IE8, see https://github.com/ebryn/backburner.js/pull/50 + // but it probably won't since there is a catch further up the chain + try { + this.$thumbs.each( function ( i, thumb ) { + bs.processThumb( thumb ); + } ); + } finally { + this.thumbsReadyDeferred.resolve(); + // now that we have set up our real click handler we can we can remove the temporary + // handler added in mmv.head.js which just replays clicks to the real handler + $( document ).off( 'click.mmv-head' ); + } }; /** @@ -207,11 +216,6 @@ $link.add( $enlarge ).click( function ( e ) { return bs.click( this, e, title, alwaysOpen ); } ); - // now that we have set up our real click handler we can we can remove the temporary - // handler added in mmv.head.js which just replays clicks to the real handler - $( document ).off( 'click.mmv-head' ); - - this.thumbsReadyDeferred.resolve(); }; /**