From c955ffce294d9e68a3b76ba07ffc6536c3a97583 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Tue, 17 Jun 2014 21:16:46 +0200 Subject: [PATCH] mediawiki.page.image.pagination: Clean up, optimise and fix various bugs Follows-up 427b0e1, 5e77f39. * Remove comment stating the obvious (initialize variable). * Unset 'xhr' variable in the success handler. This allows it to be freed from memory, and prevents us from uselessly trying to abort an already finished request. * Rename 'xhr' to 'jqXhr' as it is not an xhr. * Use the Promise of jQuery.ajax instead of the long-deprecated callback option 'success'. * Rephrase comment stating that jQuery.load is used (which is not true). * Don't re-query 'table.multipageimage' multiple times. Instead query it from the document once and cache it. * Remove useless 'window.history' check. This is not a new global in HTML5. The 'history' global, infamous from history.go() and history.back(), has been a part of the web for a very long time. We can safely depend on them, just as we depend on 'location', 'navigator' and 'document'. * Add an identifier to the history pop-state to prevent an exception when dealing with the state object of another application. Lots of gadgets use the History API these days (as well as extensions like VisualEditor and MultimediaViewer). * Don't store the location.href inside the pop state data. This is not needed because it is already stored by the browser natively. A history event contains a url, page title and custom data stored as JSON. There is no need to store the url again in the custom data. * Despite the width/height transferring from the old content to the spinner container, the scroll position still jumped because it wasn't accounting for margin, padding and border. Use outerHeight() instead of height(). * Avoid using the DOM to store and retrieve information. Use application state instead of a DOM query to know whether we have a spinner already. This increases performance but also avoids false positives where (unlikely) other similar elements and/or spinners from a different script are on the page. * Properly replace the entire table contents() instead of just dropping the straight into the emptied . This way the script makes no assumptions about whether there is a caption/thead/tbody or multiple rows etc. Plus it saves another recursive DOM query (the one for "tr"). And fixes a potential bug if there are nested tables (since it was doing a recursive query). Change-Id: I8b64a0860b73a5dcd8051b5e7a1fcb65107228a6 --- .../mediawiki.page.image.pagination.js | 117 +++++++++--------- 1 file changed, 58 insertions(+), 59 deletions(-) diff --git a/resources/src/mediawiki.page/mediawiki.page.image.pagination.js b/resources/src/mediawiki.page/mediawiki.page.image.pagination.js index 4819be0ca3..931e312d34 100644 --- a/resources/src/mediawiki.page/mediawiki.page.image.pagination.js +++ b/resources/src/mediawiki.page/mediawiki.page.image.pagination.js @@ -1,93 +1,92 @@ /*! - * Change multi-page image navigation so that the current page display can be changed - * without a page reload. Currently, the only image formats that can be multi-page images are - * PDF and DjVu files + * Implement AJAX navigation for multi-page images so the user may browse without a full page reload. */ ( function ( mw, $ ) { + var jqXhr, $multipageimage, $spinner; - // Initialize ajax request variable - var xhr; - - // Use jQuery's load function to specifically select and replace table.multipageimage's child - // tr with the new page's table.multipageimage's tr element. - // table.multipageimage always has only one row. - function loadPage( page, hist ) { - if ( xhr ) { - // Abort previous requests to prevent backlog created by - // repeatedly pressing back/forwards buttons - xhr.abort(); + /* Fetch the next page and use jQuery to swap the table.multipageimage contents. + * @param {string} url + * @param {boolean} [hist=false] Whether this is a load triggered by history navigation (if + * true, this function won't push a new history state, for the browser did so already). + */ + function loadPage( url, hist ) { + var $tr; + if ( jqXhr ) { + // Prevent race conditions and piling up pending requests + jqXhr.abort(); + jqXhr = undefined; } - var $multipageimage = $( 'table.multipageimage' ), - $spinner; - // Add a new spinner if one doesn't already exist - if ( !$multipageimage.find( '.mw-spinner' ).length ) { - + if ( !$spinner ) { + $tr = $multipageimage.find( 'tr' ); $spinner = $.createSpinner( { size: 'large', type: 'block' } ) - // Set the spinner's dimensions equal to the table's dimensions so that - // the current scroll position is not lost after the table is emptied prior to - // its contents being updated + // Copy the old content dimensions equal so that the current scroll position is not + // lost between emptying the table is and receiving the new contents. .css( { - height: $multipageimage.find( 'tr' ).height(), - width: $multipageimage.find( 'tr' ).width() + height: $tr.outerHeight(), + width: $tr.outerWidth() } ); $multipageimage.empty().append( $spinner ); } - xhr = $.ajax( { - url: page, - success: function ( data ) { - // Load the page - $multipageimage.empty().append( $( data ).find( 'table.multipageimage tr' ) ); - // Fire hook because the page's content has changed - mw.hook( 'wikipage.content' ).fire( $multipageimage ); - // Set up the new page for pagination - ajaxifyPageNavigation(); - // Add new page of image to history. To preserve the back-forwards chain in the browser, - // if the user gets here via the back/forward button, don't update the history. - if ( window.history && history.pushState && !hist ) { - history.pushState( { url: page }, document.title, page ); - } + // @todo Don't fetch the entire page. Ideally we'd only fetch the content portion or the data + // (thumbnail urls) and update the interface manually. + jqXhr = $.ajax( url ).done( function ( data ) { + jqXhr = $spinner = undefined; + + // Replace table contents + $multipageimage.empty().append( $( data ).find( 'table.multipageimage' ).contents() ); + + bindPageNavigation( $multipageimage ); + + // Fire hook because the page's content has changed + mw.hook( 'wikipage.content' ).fire( $multipageimage ); + + // Update browser history and address bar. But not if we came here from a history + // event, in which case the url is already updated by the browser. + if ( history.pushState && !hist ) { + history.pushState( { tag: 'mw-pagination' }, document.title, url ); } } ); } - function ajaxifyPageNavigation() { - // Intercept the default action of the links in the thumbnail navigation - $( '.multipageimagenavbox' ).one( 'click', 'a', function ( e ) { + function bindPageNavigation( $container ) { + $container.find( '.multipageimagenavbox' ).one( 'click', 'a', function ( e ) { loadPage( this.href ); e.preventDefault(); } ); - // Prevent the submission of the page select form and instead call loadPage - $( 'form[name="pageselector"]' ).one( 'change submit', function ( e ) { + $container.find( 'form[name="pageselector"]' ).one( 'change submit', function ( e ) { loadPage( this.action + '?' + $( this ).serialize() ); e.preventDefault(); } ); } - $( document ).ready( function () { - // The presence of table.multipageimage signifies that this file is a multi-page image - if ( mw.config.get( 'wgNamespaceNumber' ) === 6 && $( 'table.multipageimage' ).length !== 0 ) { - ajaxifyPageNavigation(); + $( function () { + if ( mw.config.get( 'wgNamespaceNumber' ) !== 6 ) { + return; + } + $multipageimage = $( 'table.multipageimage' ); + if ( !$multipageimage.length ) { + return; + } - // Set up history.pushState (if available), so that when the user browses to a new page of - // the same file, the browser's history is updated. If the user clicks the back/forward button - // in the midst of navigating a file's pages, load the page inline. - if ( window.history && history.pushState && history.replaceState ) { - history.replaceState( { url: window.location.href }, '' ); - $( window ).on( 'popstate', function ( e ) { - var state = e.originalEvent.state; - if ( state ) { - loadPage( state.url, true ); - } - } ); - } + bindPageNavigation( $multipageimage ); + + // Update the url using the History API (if available) + if ( history.pushState && history.replaceState ) { + history.replaceState( { tag: 'mw-pagination' }, '' ); + $( window ).on( 'popstate', function ( e ) { + var state = e.originalEvent.state; + if ( state && state.tag === 'mw-pagination' ) { + loadPage( location.href, true ); + } + } ); } } ); }( mediaWiki, jQuery ) ); -- 2.20.1