mediawiki.page.image.pagination: Clean up, optimise and fix various bugs
authorTimo Tijhof <krinklemail@gmail.com>
Tue, 17 Jun 2014 19:16:46 +0000 (21:16 +0200)
committerBartosz Dziewoński <matma.rex@gmail.com>
Thu, 19 Jun 2014 14:12:01 +0000 (14:12 +0000)
Follows-up 427b0e15e77f39.

* 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 <tr> straight into the emptied <table>. 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

resources/src/mediawiki.page/mediawiki.page.image.pagination.js

index 4819be0..931e312 100644 (file)
@@ -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 ) );