From 4b6e44a2fcecc5bb316748a364727d20bcea7eff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Tisza?= Date: Fri, 12 Sep 2014 17:13:42 +0000 Subject: [PATCH] Make sure event handlers are set up even if onready handler is lost Due to a jQuery bug, errors in local code (gadgets, user scripts) can cause onready handlers to not be executed. For MMV this causes catastrophic failure, with a black screen of death on exit. This change makes sure that the setup code necessary for Media Viewer to work is executed at latest when MV is invoked, even if some onready handlers were skipped. Opening MediaViewer via a hash-URL will still not work if the onready handler fails, but that's hard to avoid and it is not a catastrophic failure anymore. This change can be reverted when bug 70772 gets fixed. Bug: 70756 Change-Id: Ida3b780791bc9dfec29303567d33e3aa4f44dd81 Mingle: https://wikimedia.mingle.thoughtworks.com/projects/multimedia/cards/891 --- resources/mmv/mmv.bootstrap.js | 18 ++++++++++++++++++ tests/qunit/mmv/mmv.bootstrap.test.js | 6 ++++++ 2 files changed, 24 insertions(+) diff --git a/resources/mmv/mmv.bootstrap.js b/resources/mmv/mmv.bootstrap.js index 9b9898f4b..5024dbfe3 100644 --- a/resources/mmv/mmv.bootstrap.js +++ b/resources/mmv/mmv.bootstrap.js @@ -307,6 +307,8 @@ mw.mmv.actionLogger.log( 'enlarge' ); } + this.ensureEventHandlersAreSetUp(); + this.loadViewer().then( function ( viewer ) { viewer.loadImageByTitle( title, true ); } ); @@ -395,6 +397,9 @@ MMVB.setupEventHandlers = function () { var self = this; + /** @property {boolean} eventHandlersHaveBeenSetUp tracks domready event handler state */ + this.eventHandlersHaveBeenSetUp = true; + $( window ).on( this.browserHistory && this.browserHistory.pushState ? 'popstate.mmvb' : 'hashchange', function () { self.hash(); } ); @@ -415,6 +420,19 @@ MMVB.cleanupEventHandlers = function () { $( window ).off( 'hashchange popstate.mmvb' ); $( document ).off( 'mmv-hash' ); + this.eventHandlersHaveBeenSetUp = false; + }; + + /** + * Makes sure event handlers are set up properly via MultimediaViewerBootstrap.setupEventHandlers(). + * Called before loading the main mmv module. At this point, event handers for MultimediaViewerBootstrap + * should have been set up, but due to bug 70756 it cannot be guaranteed. + */ + MMVB.ensureEventHandlersAreSetUp = function () { + if ( !this.eventHandlersHaveBeenSetUp ) { + this.setupEventHandlers(); + this.eventHandlersHaveBeenSetUp = true; + } }; /** diff --git a/tests/qunit/mmv/mmv.bootstrap.test.js b/tests/qunit/mmv/mmv.bootstrap.test.js index 4725a8e98..a0bfed63a 100644 --- a/tests/qunit/mmv/mmv.bootstrap.test.js +++ b/tests/qunit/mmv/mmv.bootstrap.test.js @@ -38,6 +38,11 @@ function createBootstrap( viewer ) { var bootstrap = new mw.mmv.MultimediaViewerBootstrap(); + + // MultimediaViewerBootstrap.ensureEventHandlersAreSetUp() is a weird workaround for gadget bugs. + // MediaViewer should work without it, and so should the tests. + bootstrap.ensureEventHandlersAreSetUp = $.noop; + bootstrap.getViewer = function() { return viewer ? viewer : { initWithThumbs : $.noop }; }; return bootstrap; @@ -101,6 +106,7 @@ this.sandbox.stub( mw.loader, 'using' ) .callsArgWith( 2, new Error( 'loading failed', ['mmv'] ) ) .withArgs( 'mediawiki.notification' ).returns( $.Deferred().reject() ); // needed for mw.notify + bootstrap.ensureEventHandlersAreSetUp = $.noop; event = new $.Event( 'click', { button: 0, which: 1 } ); returnValue = bootstrap.click( {}, event, 'foo' );