From 93514ba101ac5c037f5df52288b76a87d6eb3ce0 Mon Sep 17 00:00:00 2001 From: Mark Jaquith Date: Thu, 18 Jul 2013 16:35:19 +0000 Subject: [PATCH] Revisions: Better error handling. * Shows an error message if the current diff can't be loaded. * For bulk pre-loading, catches errors, and cuts subsequent requests in half, until eventually giving up. * Some CSS fixes related to this, and the loading spinner. * `wp.revisions.loadAll()` now returns a promise representing whether or not all revisions could be loaded. Fixes #24758. git-svn-id: http://core.svn.wordpress.org/trunk@24732 1a063a9b-81f0-0310-95a4-ce76da25c4cd --- wp-admin/css/wp-admin.css | 28 ++++++++++++++++++---- wp-admin/js/revisions.js | 49 ++++++++++++++++++++++++++++++--------- wp-admin/revision.php | 3 ++- 3 files changed, 63 insertions(+), 17 deletions(-) diff --git a/wp-admin/css/wp-admin.css b/wp-admin/css/wp-admin.css index 8cb0e3bfc8..6b415f725f 100644 --- a/wp-admin/css/wp-admin.css +++ b/wp-admin/css/wp-admin.css @@ -3554,16 +3554,24 @@ body.revision-php { height: 140px; } -.revisions .loading-indicator { +.revisions .diff-error { position: absolute; text-align: center; - vertical-align: middle; - opacity: 0; margin: 0 auto; width: 100%; - height: 32px; + display: none; +} + +.revisions.diff-error .diff-error { + display: block; +} + +.revisions .loading-indicator { + position: absolute; + vertical-align: middle; + opacity: 0; + width: 100%; top: 3em; - background: #fff url(../images/wpspin_light-2x.gif) no-repeat center top; -webkit-transition: opacity 0.5s; -moz-transition: opacity 0.5s; -ms-transition: opacity 0.5s; @@ -3571,6 +3579,12 @@ body.revision-php { transition: opacity 0.5s; } +.revisions .loading-indicator span.spinner { + display: block; + margin: 0 auto; + float: none; +} + .revisions.loading .loading-indicator { opacity: 1; } @@ -3587,6 +3601,10 @@ body.revision-php { opacity: 0.5; } +.revisions.diff-error .diff { + visibility: hidden; +} + .revisions-meta { margin-top: 15px; } diff --git a/wp-admin/js/revisions.js b/wp-admin/js/revisions.js index b20240eeec..d72186fd31 100644 --- a/wp-admin/js/revisions.js +++ b/wp-admin/js/revisions.js @@ -233,7 +233,9 @@ window.wp = window.wp || {}; request.done( _.bind( function() { deferred.resolveWith( context, [ this.get( id ) ] ); - }, this ) ); + }, this ) ).fail( _.bind( function() { + deferred.reject(); + }) ); } return deferred.promise(); @@ -256,13 +258,22 @@ window.wp = window.wp || {}; diffs = _.first( this.getClosestUnloaded( allRevisionIds, centerId ), num ); if ( _.size( diffs ) > 0 ) { this.load( diffs ).done( function() { - deferred.resolve(); - self._loadAll( allRevisionIds, centerId, num ); + self._loadAll( allRevisionIds, centerId, num ).done( function() { + deferred.resolve(); + }); + }).fail( function() { + if ( 1 === num ) { // Already tried 1. This just isn't working. Give up. + deferred.reject(); + } else { // Request fewer diffs this time + self._loadAll( allRevisionIds, centerId, Math.ceil( num / 2 ) ).done( function() { + deferred.resolve(); + }); + } }); - return deferred.promise(); } else { - return deferred.reject().promise(); + deferred.resolve(); } + return deferred; }, load: function( comparisons ) { @@ -314,12 +325,14 @@ window.wp = window.wp || {}; revisions.model.FrameState = Backbone.Model.extend({ defaults: { loading: false, + error: false, compareTwoMode: false }, initialize: function( attributes, options ) { var properties = {}; + _.bindAll( this, 'receiveDiff' ); this._debouncedEnsureDiff = _.debounce( this._ensureDiff, 200 ); this.revisions = options.revisions; @@ -351,6 +364,7 @@ window.wp = window.wp || {}; }, updateLoadingStatus: function() { + this.set( 'error', false ); this.set( 'loading', ! this.diff() ); }, @@ -399,7 +413,7 @@ window.wp = window.wp || {}; // If we already have the diff, then immediately trigger the update. if ( diff ) { - this.trigger( 'update:diff', diff ); + this.receiveDiff( diff ); return $.Deferred().resolve().promise(); // Otherwise, fetch the diff. } else { @@ -418,12 +432,20 @@ window.wp = window.wp || {}; this.updateDiff(); }, + receiveDiff: function( diff ) { + // Did we actually get a diff? + if ( _.isUndefined( diff ) || _.isUndefined( diff.id ) ) { + this.set({ + loading: false, + error: true + }); + } else if ( this._diffId === diff.id ) { // Make sure the current diff didn't change + this.trigger( 'update:diff', diff ); + } + }, + _ensureDiff: function() { - return this.diffs.ensure( this._diffId, this ).done( function( diff ) { - // Make sure the current diff didn't change while the request was in flight. - if ( this._diffId === diff.id ) - this.trigger( 'update:diff', diff ); - }); + return this.diffs.ensure( this._diffId, this ).always( this.receiveDiff ); } }); @@ -443,6 +465,7 @@ window.wp = window.wp || {}; this.listenTo( this.model, 'update:diff', this.renderDiff ); this.listenTo( this.model, 'change:compareTwoMode', this.updateCompareTwoMode ); this.listenTo( this.model, 'change:loading', this.updateLoadingStatus ); + this.listenTo( this.model, 'change:error', this.updateErrorStatus ); this.views.set( '.revisions-control-frame', new revisions.view.Controls({ model: this.model @@ -470,6 +493,10 @@ window.wp = window.wp || {}; this.$el.toggleClass( 'loading', this.model.get('loading') ); }, + updateErrorStatus: function() { + this.$el.toggleClass( 'diff-error', this.model.get('error') ); + }, + updateCompareTwoMode: function() { this.$el.toggleClass( 'comparing-two-revisions', this.model.get('compareTwoMode') ); } diff --git a/wp-admin/revision.php b/wp-admin/revision.php index 3d4d36906a..d87ab4e420 100644 --- a/wp-admin/revision.php +++ b/wp-admin/revision.php @@ -186,7 +186,8 @@ require_once( './admin-header.php' );