jenkins-bot has submitted this change and it was merged. Change subject: Replace current URL generation logic with routing classes ......................................................................
Replace current URL generation logic with routing classes * deduplicates URL generating/parsing code * gets rid of spaces in URLs * fixes error for file names with / in them (in case they exist; current MediaWiki seems to disallow such names anyway) Change-Id: I5aad43f6af1b99523c597c39befcc9db1ecab83a Mingle: https://wikimedia.mingle.thoughtworks.com/projects/multimedia/cards/371 --- M MultimediaViewer.php M MultimediaViewerHooks.php M resources/mmv/mmv.EmbedFileFormatter.js M resources/mmv/mmv.bootstrap.js M resources/mmv/mmv.js M resources/mmv/ui/mmv.ui.reuse.share.js M tests/qunit/mmv/mmv.bootstrap.test.js M tests/qunit/mmv/mmv.test.js M tests/qunit/mmv/ui/mmv.ui.reuse.share.test.js 9 files changed, 38 insertions(+), 18 deletions(-) Approvals: Gilles: Looks good to me, approved jenkins-bot: Verified diff --git a/MultimediaViewer.php b/MultimediaViewer.php index 236863b..78ac77a 100644 --- a/MultimediaViewer.php +++ b/MultimediaViewer.php @@ -471,6 +471,7 @@ 'dependencies' => array( 'mmv.base', + 'mmv.routing', 'oojs', 'mmv.HtmlUtils', ), @@ -540,6 +541,7 @@ 'dependencies' => array( 'mmv.ui.reuse.tab', 'mmv.ui.reuse.utils', + 'mmv.routing', 'oojs', 'oojs-ui', ), @@ -686,6 +688,7 @@ 'mmv.model.TaskQueue', 'mmv.lightboxinterface', 'mmv.provider', + 'mmv.routing', 'jquery.fullscreen', 'jquery.hidpi', 'jquery.scrollTo', diff --git a/MultimediaViewerHooks.php b/MultimediaViewerHooks.php index cc0d104..fb5e32b 100644 --- a/MultimediaViewerHooks.php +++ b/MultimediaViewerHooks.php @@ -228,7 +228,6 @@ 'mmv.ui.reuse.share', 'mmv.ui.reuse.embed', 'mmv.ui.reuse.download', - 'mmv.routing', ), 'localBasePath' => __DIR__, 'remoteExtPath' => 'MultimediaViewer', diff --git a/resources/mmv/mmv.EmbedFileFormatter.js b/resources/mmv/mmv.EmbedFileFormatter.js index 93766e3..7297114 100644 --- a/resources/mmv/mmv.EmbedFileFormatter.js +++ b/resources/mmv/mmv.EmbedFileFormatter.js @@ -26,6 +26,11 @@ function EmbedFileFormatter() { /** @property {mw.mmv.HtmlUtils} htmlUtils - */ this.htmlUtils = new mw.mmv.HtmlUtils(); + + /** + * @property {mw.mmv.routing.Router} router - + */ + this.router = new mw.mmv.routing.Router(); } EFFP = EmbedFileFormatter.prototype; @@ -167,13 +172,12 @@ }; /** - * Generare a link which we will be using for sharing stuff. - * FIXME this should be handled by mmv.js to be DRY - * + * Generate a link which we will be using for sharing stuff. * @param {mw.mmv.model.EmbedFileInfo} info */ EFFP.getLinkUrl = function ( info ) { - return info.imageInfo.descriptionUrl + '#mediaviewer/' + info.imageInfo.title.getMainText(); + var route = new mw.mmv.routing.ThumbnailRoute( info.imageInfo.title ); + return this.router.createHashedUrl( route, info.imageInfo.descriptionUrl ); }; mw.mmv.EmbedFileFormatter = EmbedFileFormatter; diff --git a/resources/mmv/mmv.bootstrap.js b/resources/mmv/mmv.bootstrap.js index 3722b8e..42eb17f 100755 --- a/resources/mmv/mmv.bootstrap.js +++ b/resources/mmv/mmv.bootstrap.js @@ -235,7 +235,7 @@ } this.loadViewer().then( function ( viewer ) { - viewer.loadImageByTitle( title.getPrefixedText(), true ); + viewer.loadImageByTitle( title, true ); } ); e.preventDefault(); diff --git a/resources/mmv/mmv.js b/resources/mmv/mmv.js index e55288b..fb38bb5 100755 --- a/resources/mmv/mmv.js +++ b/resources/mmv/mmv.js @@ -80,9 +80,14 @@ /** * Image index on page. - * @type {number} + * @property {number} */ this.currentIndex = 0; + + /** + * @property {mw.mmv.routing.Router} router - + */ + this.router = new mw.mmv.routing.Router(); /** * UI object used to display the pictures in the page. @@ -318,7 +323,7 @@ /** * Loads an image by its title - * @param {string} title + * @param {mw.Title} title * @param {boolean} updateHash Viewer should update the location hash when true */ MMVP.loadImageByTitle = function ( title, updateHash ) { @@ -331,7 +336,7 @@ this.comingFromHashChange = !updateHash; $.each( this.thumbs, function ( idx, thumb ) { - if ( thumb.title.getPrefixedText() === title ) { + if ( thumb.title.getPrefixedText() === title.getPrefixedText() ) { viewer.loadImage( thumb.image, thumb.$thumb.clone()[ 0 ], true ); return false; } @@ -635,11 +640,10 @@ * Handles a hash change coming from the browser */ MMVP.hash = function () { - var hash = decodeURIComponent( window.location.hash ), - linkState = hash.split( '/' ); + var route = this.router.parseLocation( window.location ); - if ( linkState[0] === '#mediaviewer' ) { - this.loadImageByTitle( linkState[ 1 ] ); + if ( route instanceof mw.mmv.routing.ThumbnailRoute ) { + this.loadImageByTitle( route.fileTitle ); } else if ( this.isOpen ) { // This allows us to avoid the mmv-hash event that normally happens on close comingFromHashChange = true; @@ -654,8 +658,10 @@ }; MMVP.setHash = function() { + var route, hashFragment; if ( !this.comingFromHashChange ) { - var hashFragment = '#mediaviewer/' + this.currentImageFilename; + route = new mw.mmv.routing.ThumbnailRoute( this.currentImageFileTitle ); + hashFragment = '#' + this.router.createHash( route ); $( document ).trigger( $.Event( 'mmv-hash', { hash : hashFragment } ) ); } }; diff --git a/resources/mmv/ui/mmv.ui.reuse.share.js b/resources/mmv/ui/mmv.ui.reuse.share.js index 5e8e8b3..6df7555 100644 --- a/resources/mmv/ui/mmv.ui.reuse.share.js +++ b/resources/mmv/ui/mmv.ui.reuse.share.js @@ -28,6 +28,11 @@ function Share( $container ) { Share['super'].call( this, $container ); + /** + * @property {mw.mmv.routing.Router} router - + */ + this.router = new mw.mmv.routing.Router(); + this.init(); } oo.inheritClass( Share, mw.mmv.ui.reuse.Tab ); @@ -75,8 +80,9 @@ * @param {mw.mmv.model.Image} image */ SP.set = function ( image ) { - // FIXME this should be handled by mmv.js to be DRY - var url = image.descriptionUrl + '#mediaviewer/' + image.title.getMainText(); + var route = new mw.mmv.routing.ThumbnailRoute( image.title ), + url = this.router.createHashedUrl( route, image.descriptionUrl ); + this.pageInput.setValue( url ); this.select(); diff --git a/tests/qunit/mmv/mmv.bootstrap.test.js b/tests/qunit/mmv/mmv.bootstrap.test.js index 6d6568f..d3d6211 100644 --- a/tests/qunit/mmv/mmv.bootstrap.test.js +++ b/tests/qunit/mmv/mmv.bootstrap.test.js @@ -228,7 +228,7 @@ $link = $div.find( 'a.image' ); viewer.loadImageByTitle = function ( loadedTitle ) { - assert.strictEqual( loadedTitle, 'File:Foo.jpg', 'Titles are identical' ); + assert.strictEqual( loadedTitle.getPrefixedDb(), 'File:Foo.jpg', 'Titles are identical' ); }; // Create a new bootstrap object to trigger the DOM scan, etc. diff --git a/tests/qunit/mmv/mmv.test.js b/tests/qunit/mmv/mmv.test.js index 37c6a26..c52dc2b 100644 --- a/tests/qunit/mmv/mmv.test.js +++ b/tests/qunit/mmv/mmv.test.js @@ -78,7 +78,7 @@ $( '#qunit-fixture' ).append( '<a class="image"><img src="' + imageSrc + '"></a>' ); viewer.loadImageByTitle = function( title ) { - assert.strictEqual( title, 'File:' + imageSrc, 'The title matches' ); + assert.strictEqual( title.getPrefixedText(), 'File:' + imageSrc, 'The title matches' ); }; // Open a valid mmv hash link and check that the right image is requested. diff --git a/tests/qunit/mmv/ui/mmv.ui.reuse.share.test.js b/tests/qunit/mmv/ui/mmv.ui.reuse.share.test.js index 181291a..9558a29 100644 --- a/tests/qunit/mmv/ui/mmv.ui.reuse.share.test.js +++ b/tests/qunit/mmv/ui/mmv.ui.reuse.share.test.js @@ -36,6 +36,7 @@ image = { // fake mw.mmv.model.Image title: new mw.Title( 'File:Foobar.jpg' ), url: 'https://upload.wikimedia.org/wikipedia/commons/3/3a/Foobar.jpg', + descriptionUrl: '//commons.wikimedia.org/wiki/File:Foobar.jpg' }; assert.notStrictEqual( ! share.pageInput.getValue(), '', 'pageInput is empty.' ); @@ -58,6 +59,7 @@ image = { title: new mw.Title( 'File:Foobar.jpg' ), url: 'https://upload.wikimedia.org/wikipedia/commons/3/3a/Foobar.jpg', + descriptionUrl: '//commons.wikimedia.org/wiki/File:Foobar.jpg' }; share.set( image ); -- To view, visit https://gerrit.wikimedia.org/r/125932 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I5aad43f6af1b99523c597c39befcc9db1ecab83a Gerrit-PatchSet: 9 Gerrit-Project: mediawiki/extensions/MultimediaViewer Gerrit-Branch: master Gerrit-Owner: Gergő Tisza <gti...@wikimedia.org> Gerrit-Reviewer: Gergő Tisza <gti...@wikimedia.org> Gerrit-Reviewer: Gilles <gdu...@wikimedia.org> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
