wikimedia/mediawiki-extensions-MultimediaViewer
1
0
Fork 0
mirror of https://gerrit.wikimedia.org/r/mediawiki/extensions/MultimediaViewer synced 2024-09-29 05:07:36 +00:00
Code Issues Packages Projects Releases Wiki Activity

Do not set up the overlay on irrelevant hash changes

Browse source 
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
This commit is contained in:
Tisza Gergő 2015-12-03 21:35:30 +00:00
parent 216d62b2e7
commit 6dddb2f7db
2 changed files with 51 additions and 10 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

30
resources/mmv/mmv.bootstrap.js
View file

@ -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 ( setupOverlay ) {
var deferred = $.Deferred(),
bs = this,
viewer,
@ -92,7 +93,14 @@
return deferred.reject();
}
bs.setupOverlay();
// FIXME setupOverlay is a quick 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 ( setupOverlay ) {
bs.setupOverlay();
}
mw.loader.using( 'mmv', function () {
try {
@ -373,7 +381,7 @@
this.ensureEventHandlersAreSetUp();
return this.loadViewer().then( function ( viewer ) {
return this.loadViewer( true ).then( function ( viewer ) {
viewer.loadImageByTitle( title, true );
} );
};
@ -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

31
tests/qunit/mmv/mmv.bootstrap.test.js
View file

@ -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;
}
@ -106,7 +106,7 @@
QUnit.stop();
bootstrap.loadViewer().fail( function ( message ) {
bootstrap.loadViewer( true ).fail( function ( message ) {
assert.strictEqual( message, errorMessage, 'promise is rejected with the error message when loading fails' );
QUnit.start();
} );
@ -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(),