Gergő Tisza has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/160413

Change subject: Make sure event handlers are set up even if onready handler is 
lost
......................................................................

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
(cherry picked from commit 4b6e44a2fcecc5bb316748a364727d20bcea7eff)
---
M resources/mmv/mmv.bootstrap.js
M tests/qunit/mmv/mmv.bootstrap.test.js
2 files changed, 24 insertions(+), 0 deletions(-)


  git pull 
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/MultimediaViewer 
refs/changes/13/160413/1

diff --git a/resources/mmv/mmv.bootstrap.js b/resources/mmv/mmv.bootstrap.js
index 9b9898f..5024dbf 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 4725a8e..a0bfed6 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' );

-- 
To view, visit https://gerrit.wikimedia.org/r/160413
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ida3b780791bc9dfec29303567d33e3aa4f44dd81
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/MultimediaViewer
Gerrit-Branch: wmf/1.24wmf21
Gerrit-Owner: Gergő Tisza <gti...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to