From 8065fdf2b9498544a8506655064f86639a42799a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartosz=20Dziewo=C5=84ski?= Date: Sat, 15 Feb 2020 03:50:16 +0100 Subject: [PATCH] Clean up code related to token and bad token handling Things I noticed while writing I37f8e89b6d92c419d1b6569891612256342f8139, but which felt too messy to include in that commit. * Use promise chaining * Update documentation * Remove redundant code * Split a method that now handles two different errors * Grumble about localisation messages Change-Id: I81e28a03af4f6c3452679ef6bbcaa89bb1235122 --- .../init/targets/ve.init.mw.ArticleTarget.js | 59 ++++++++------ .../ve-mw/init/targets/ve.init.mw.Target.js | 80 +++++++------------ .../preinit/ve.init.mw.ArticleTargetSaver.js | 10 +-- 3 files changed, 69 insertions(+), 80 deletions(-) diff --git a/modules/ve-mw/init/targets/ve.init.mw.ArticleTarget.js b/modules/ve-mw/init/targets/ve.init.mw.ArticleTarget.js index 3929f88c5d..54be7def4c 100644 --- a/modules/ve-mw/init/targets/ve.init.mw.ArticleTarget.js +++ b/modules/ve-mw/init/targets/ve.init.mw.ArticleTarget.js @@ -784,33 +784,48 @@ ve.init.mw.ArticleTarget.prototype.saveErrorHookAborted = function ( data ) { }; /** - * Handle assert error indicating another user is logged in, and token fetch errors. + * Handle assert error indicating another user is logged in. * * @param {string|null} username Name of newly logged-in user, or null if anonymous - * @param {boolean} [error=false] Whether this is a token fetch error - * @fires saveErrorBadToken * @fires saveErrorNewUser */ -ve.init.mw.ArticleTarget.prototype.saveErrorBadTokenOrNewUser = function ( username, error ) { - var $msg = $( document.createTextNode( mw.msg( 'visualeditor-savedialog-error-badtoken' ) + ' ' ) ); +ve.init.mw.ArticleTarget.prototype.saveErrorNewUser = function ( username ) { + var $msg; + + this.emit( 'saveErrorNewUser' ); + + // TODO: Improve this message, concatenating it this way is a bad practice. + // This should read more like 'session_fail_preview' in MediaWiki core + // (but with the caveat that we know already whether you're logged in or not). + $msg = $( document.createTextNode( mw.msg( 'visualeditor-savedialog-error-badtoken' ) + ' ' ) ).add( + mw.message( + username === null ? + 'visualeditor-savedialog-identify-anon' : + 'visualeditor-savedialog-identify-user', + username + ).parseDom() + ); - if ( error ) { - this.emit( 'saveErrorBadToken', false ); - $msg = $msg.add( document.createTextNode( mw.msg( 'visualeditor-savedialog-identify-trylogin' ) ) ); - } else { - this.emit( 'saveErrorNewUser' ); - $msg = $msg.add( - mw.message( - username === null ? - 'visualeditor-savedialog-identify-anon' : - 'visualeditor-savedialog-identify-user', - username - ).parseDom() - ); - } this.showSaveError( $msg ); }; +/** + * Handle token fetch errors. + * + * @fires saveErrorBadToken + */ +ve.init.mw.ArticleTarget.prototype.saveErrorBadToken = function () { + this.emit( 'saveErrorBadToken', false ); + + // TODO: Improve this message, concatenating it this way is a bad practice. + // Also, it's not always true that you're "no longer logged in". + // This should read more like 'session_fail_preview' in MediaWiki core. + this.showSaveError( + mw.msg( 'visualeditor-savedialog-error-badtoken' ) + ' ' + + mw.msg( 'visualeditor-savedialog-identify-trylogin' ) + ); +}; + /** * Handle unknown save error * @@ -1264,10 +1279,8 @@ ve.init.mw.ArticleTarget.prototype.clearPreparedCacheKey = function () { * HTML directly if there is no cache key present or pending, or if the request for the cache key * fails, or if using the cache key fails with a badcachekey error. * - * If extraData.token is set, this function will use mw.Api#post and let the caller handle badtoken - * errors. If extraData.token is not set, this function will use mw.Api#postWithToken which retries - * automatically when encountering a badtoken error. If you do not want the automatic retry behavior - * and want to control badtoken retries, you have to set extradata.token. + * This function will use mw.Api#postWithToken to retry automatically when encountering a 'badtoken' + * error. * * @param {HTMLDocument|string} doc Document to submit or string in source mode * @param {Object} extraData POST parameters to send. Do not include 'html', 'cachekey' or 'format'. diff --git a/modules/ve-mw/init/targets/ve.init.mw.Target.js b/modules/ve-mw/init/targets/ve.init.mw.Target.js index fc55c3fd38..4a8eb14bee 100644 --- a/modules/ve-mw/init/targets/ve.init.mw.Target.js +++ b/modules/ve-mw/init/targets/ve.init.mw.Target.js @@ -461,51 +461,35 @@ ve.init.mw.Target.prototype.teardown = function () { * @return {jQuery.Promise} Promise resolved with new username, or null if anonymous */ ve.init.mw.Target.prototype.refreshUser = function ( doc ) { - var api = this.getContentApi( doc ), - deferred = ve.createDeferred(); - api.get( { + return this.getContentApi( doc ).get( { action: 'query', meta: 'userinfo' - } ) - .done( function ( data ) { - var - userInfo = data.query && data.query.userinfo, - isAnon = mw.user.isAnon(); + } ).then( function ( data ) { + var userInfo = data.query && data.query.userinfo; - if ( userInfo ) { - if ( - ( isAnon && userInfo.anon !== undefined ) || - // Comparing id instead of name to protect against possible - // normalisation and against case where the user got renamed. - mw.config.get( 'wgUserId' ) === userInfo.id - ) { - // New session is the same user still - deferred.resolve( mw.user.getName() ); - } else { - // The now current session is a different user - if ( userInfo.anon !== undefined ) { - // New session is an anonymous user - mw.config.set( { - // wgUserId is unset for anonymous users, not set to null - wgUserId: undefined, - // wgUserName is explicitly set to null for anonymous users, - // functions like mw.user.isAnon rely on this. - wgUserName: null - } ); - } else { - // New session is a different user - mw.config.set( { wgUserId: userInfo.id, wgUserName: userInfo.name } ); - } - deferred.resolve( mw.user.getName() ); - } - } else { - deferred.reject(); - } - } ) - .fail( function () { - deferred.reject(); - } ); - return deferred.promise(); + if ( !userInfo ) { + return ve.createDeferred().reject(); + } + + if ( userInfo.anon !== undefined ) { + // New session is an anonymous user + mw.config.set( { + // wgUserId is unset for anonymous users, not set to null + wgUserId: undefined, + // wgUserName is explicitly set to null for anonymous users, + // functions like mw.user.isAnon rely on this. + wgUserName: null + } ); + } else { + // New session is a logged in user + mw.config.set( { + wgUserId: userInfo.id, + wgUserName: userInfo.name + } ); + } + + return mw.user.getName(); + } ); }; /** @@ -516,7 +500,7 @@ ve.init.mw.Target.prototype.refreshUser = function ( doc ) { * @return {jQuery.Promise} Abortable promise which resolves with a wikitext string */ ve.init.mw.Target.prototype.getWikitextFragment = function ( doc, useRevision ) { - var promise, xhr, + var xhr, params = { action: 'visualeditoredit', paction: 'serialize', @@ -539,18 +523,12 @@ ve.init.mw.Target.prototype.getWikitextFragment = function ( doc, useRevision ) { contentType: 'multipart/form-data' } ); - promise = xhr.then( function ( response ) { + return xhr.then( function ( response ) { if ( response.visualeditoredit ) { return response.visualeditoredit.content; } return ve.createDeferred().reject(); - } ); - - promise.abort = function () { - xhr.abort(); - }; - - return promise; + } ).promise( { abort: xhr.abort } ); }; /** diff --git a/modules/ve-mw/preinit/ve.init.mw.ArticleTargetSaver.js b/modules/ve-mw/preinit/ve.init.mw.ArticleTargetSaver.js index 147fca9505..d4d35073d3 100644 --- a/modules/ve-mw/preinit/ve.init.mw.ArticleTargetSaver.js +++ b/modules/ve-mw/preinit/ve.init.mw.ArticleTargetSaver.js @@ -195,7 +195,8 @@ }, /** - * Post content to the API. + * Post content to the API, using mw.Api#postWithToken to retry automatically when encountering + * a 'badtoken' error. * * By default uses action=visualeditoredit, paction=save. * @@ -230,11 +231,8 @@ data ); - if ( data.token ) { - request = api.post( data, { contentType: 'multipart/form-data' } ); - } else { - request = api.postWithToken( 'csrf', data, { contentType: 'multipart/form-data' } ); - } + request = api.postWithToken( 'csrf', data, { contentType: 'multipart/form-data' } ); + return request.then( function ( response, jqxhr ) { var eventData, fullEventName, error,