Gergő Tisza has uploaded a new change for review.
https://gerrit.wikimedia.org/r/256841
Change subject: Do not set up the overlay on irrelevant hash changes
......................................................................
Do not set up the overlay on irrelevant hash changes
On not-MMV -> not-MMV hash changes the bootstrap class has set
up and immediately torn down the overlay. Besides slowing things
down, this broke TOC navigation on Chrome in an ugly way.
Add a special case to skip the overlay loading when the new hash
does not contain #/media. This is a quick-and-dirty fix; the whole
loading and hash handling could use a rewrite.
Bug: T119854
Change-Id: I5494903dfe778e533773ff142fdf756475c2df32
---
M resources/mmv/mmv.bootstrap.js
M tests/qunit/mmv/mmv.bootstrap.test.js
2 files changed, 49 insertions(+), 8 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/MultimediaViewer
refs/changes/41/256841/1
diff --git a/resources/mmv/mmv.bootstrap.js b/resources/mmv/mmv.bootstrap.js
index 2a2ef4d..cf460ac 100644
--- a/resources/mmv/mmv.bootstrap.js
+++ b/resources/mmv/mmv.bootstrap.js
@@ -79,9 +79,10 @@
/**
* Loads the mmv module asynchronously and passes the thumb data to it
+ * @param {boolean} [setupOverlay]
* @returns {jQuery.Promise}
*/
- MMVB.loadViewer = function () {
+ MMVB.loadViewer = function ( noOverlay ) {
var deferred = $.Deferred(),
bs = this,
viewer,
@@ -92,7 +93,14 @@
return deferred.reject();
}
- bs.setupOverlay();
+ // FIXME noOverlay is a qiuck hack to avoid setting up and
immediately
+ // removing the overlay on a not-MMV -> not-MMV hash change.
+ // loadViewer is called on every click and hash change and
setting up
+ // the overlay is not needed on all of those; this logic really
should
+ // not be here.
+ if ( !noOverlay ) {
+ bs.setupOverlay();
+ }
mw.loader.using( 'mmv', function () {
try {
@@ -410,7 +418,15 @@
return false;
};
-
+ /**
+ * Returns true if the hash part of the current URL is one that's owned
by MMV.
+ * @returns {boolean}
+ * @private
+ */
+ MMVB.isViewerHash = function () {
+ return window.location.hash.indexOf( '#mediaviewer/' ) === 0 ||
+ window.location.hash.indexOf( '#/media/' ) === 0;
+ }
/**
* Handles the browser location hash on pageload or hash change
@@ -421,9 +437,7 @@
// There is no point loading the mmv if it isn't loaded yet for
hash changes unrelated to the mmv
// Such as anchor links on the page
- if ( !this.viewerInitialized
- && window.location.hash.indexOf( '#mediaviewer/' ) !== 0
- && window.location.hash.indexOf( '#/media/' ) !== 0 ) {
+ if ( !this.viewerInitialized && !this.isViewerHash() ) {
return;
}
@@ -432,7 +446,7 @@
return;
}
- this.loadViewer().then( function ( viewer ) {
+ this.loadViewer( !this.isViewerHash() ).then( function ( viewer
) {
viewer.hash();
// this is an ugly temporary fix to avoid a black
screen of death when
// the page is loaded with an invalid MMV url
diff --git a/tests/qunit/mmv/mmv.bootstrap.test.js
b/tests/qunit/mmv/mmv.bootstrap.test.js
index 61a2ccf..d4e666f 100644
--- a/tests/qunit/mmv/mmv.bootstrap.test.js
+++ b/tests/qunit/mmv/mmv.bootstrap.test.js
@@ -56,7 +56,7 @@
// MediaViewer should work without it, and so should the tests.
bootstrap.ensureEventHandlersAreSetUp = $.noop;
- bootstrap.getViewer = function () { return viewer ? viewer : {
initWithThumbs: $.noop }; };
+ bootstrap.getViewer = function () { return viewer ? viewer : {
initWithThumbs: $.noop, hash: $.noop }; };
return bootstrap;
}
@@ -363,6 +363,33 @@
hashTest( 'mediaviewer', bootstrap, assert );
} );
+ QUnit.test( 'Overlay is set up on hash change', 1, function( assert ) {
+ var bootstrap;
+
+ window.location.hash = '#/media/foo';
+
+ bootstrap = createBootstrap();
+ this.sandbox.stub( bootstrap, 'setupOverlay' );
+
+ bootstrap.hash();
+
+ assert.ok( bootstrap.setupOverlay.called, 'Overlay is set up' );
+ } );
+
+ QUnit.test( 'Overlay is not set up on an irrelevant hash change', 1,
function( assert ) {
+ var bootstrap;
+
+ window.location.hash = '#foo';
+
+ bootstrap = createBootstrap();
+ this.sandbox.stub( bootstrap, 'setupOverlay' );
+ bootstrap.loadViewer();
+ bootstrap.setupOverlay.reset();
+
+ bootstrap.hash();
+
+ assert.ok( !bootstrap.setupOverlay.called, 'Overlay is not set
up' );
+ } );
QUnit.test( 'internalHashChange', 1, function ( assert ) {
var bootstrap = createBootstrap(),
--
To view, visit https://gerrit.wikimedia.org/r/256841
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5494903dfe778e533773ff142fdf756475c2df32
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/MultimediaViewer
Gerrit-Branch: master
Gerrit-Owner: Gergő Tisza <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
