jenkins-bot has submitted this change and it was merged. Change subject: Do not handle clicks if MediaViewer could not be loaded. ......................................................................
Do not handle clicks if MediaViewer could not be loaded. mmv.bootstrap will not try again to handle clicks if it failed for the first time. Change-Id: Idd55d7dc6c1388070895f8630bdcd51763a94d86 Mingle: https://wikimedia.mingle.thoughtworks.com/projects/multimedia/cards/527 Bug: 63801 --- M resources/mmv/mmv.bootstrap.js M tests/qunit/mmv/mmv.bootstrap.test.js 2 files changed, 32 insertions(+), 9 deletions(-) Approvals: MarkTraceur: Looks good to me, approved jenkins-bot: Verified diff --git a/resources/mmv/mmv.bootstrap.js b/resources/mmv/mmv.bootstrap.js index 99df70d..46c6049 100755 --- a/resources/mmv/mmv.bootstrap.js +++ b/resources/mmv/mmv.bootstrap.js @@ -42,6 +42,12 @@ /** @property {mw.mmv.HtmlUtils} htmlUtils - */ this.htmlUtils = new mw.mmv.HtmlUtils(); + /** + * This flag is set to true when we were unable to load the viewer. + * @property {boolean} + */ + this.viewerIsBroken = false; + this.thumbsReadyDeferred = $.Deferred(); this.thumbs = []; @@ -85,6 +91,7 @@ } ).fail( function( message ) { mw.log.warn( message ); bs.cleanupOverlay(); + bs.viewerIsBroken = true; mw.notify( 'Error loading MediaViewer: ' + message ); } ); }; @@ -230,7 +237,7 @@ /** * Handles a click event on a link - * @param {Object} element Clicked element + * @param {HTMLElement} element Clicked element * @param {jQuery.Event} e jQuery event object * @param {string} title File title * @param {boolean} overridePreference Whether to ignore global preferences and open @@ -250,6 +257,11 @@ return; } + // Don't load if we already tried loading and it failed + if ( this.viewerIsBroken ) { + return; + } + mw.mmv.DurationLogger.start( [ 'click-to-first-image', 'click-to-first-metadata' ] ); if ( $element.is( 'a.image' ) ) { diff --git a/tests/qunit/mmv/mmv.bootstrap.test.js b/tests/qunit/mmv/mmv.bootstrap.test.js index 734ad8a..d0f4177 100644 --- a/tests/qunit/mmv/mmv.bootstrap.test.js +++ b/tests/qunit/mmv/mmv.bootstrap.test.js @@ -64,15 +64,10 @@ } QUnit.test( 'Promise does not hang on ResourceLoader errors', 3, function ( assert ) { - var oldUsing = mw.loader.using, - bootstrap, + var bootstrap, errorMessage = 'loading failed'; - mw.loader.using = function ( module, success, error ) { - if ( $.isFunction( error ) ) { - error( new Error( errorMessage, ['mmv'] ) ); - } - }; + this.sandbox.stub( mw.loader, 'using' ).callsArgWith( 2, new Error( errorMessage, ['mmv'] ) ); bootstrap = createBootstrap(); @@ -89,10 +84,26 @@ bootstrap.loadViewer().fail( function ( message ) { assert.strictEqual( message, errorMessage, 'promise is rejected with the error message when loading fails' ); QUnit.start(); - mw.loader.using = oldUsing; } ); } ); + QUnit.test( 'Clicks are not captured once the loading fails', 4, function ( assert ) { + var event, returnValue, + bootstrap = new mw.mmv.MultimediaViewerBootstrap(); + + this.sandbox.stub( mw.loader, 'using' ).callsArgWith( 2, new Error( 'loading failed', ['mmv'] ) ); + + event = new $.Event( 'click', { button: 0, which: 1 } ); + returnValue = bootstrap.click( {}, event, 'foo' ); + assert.ok( event.isDefaultPrevented(), 'First click is caught' ); + assert.strictEqual( returnValue, false, 'First click is caught' ); + + event = new $.Event( 'click', { button: 0, which: 1 } ); + returnValue = bootstrap.click( {}, event, 'foo' ); + assert.ok( !event.isDefaultPrevented(), 'Click after loading failure is not caught' ); + assert.notStrictEqual( returnValue, false, 'Click after loading failure is not caught' ); + } ); + QUnit.test( 'Check viewer invoked when clicking on legit image links', 9, function ( assert ) { // TODO: Is <div class="gallery"><span class="image"><img/></span></div> valid ??? var div, link, link2, link3, link4, bootstrap, -- To view, visit https://gerrit.wikimedia.org/r/130272 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: Idd55d7dc6c1388070895f8630bdcd51763a94d86 Gerrit-PatchSet: 3 Gerrit-Project: mediawiki/extensions/MultimediaViewer Gerrit-Branch: master Gerrit-Owner: Gergő Tisza <gti...@wikimedia.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
