From b0f4b4c94ec9b25c06b41ad0c771ea683a623685 Mon Sep 17 00:00:00 2001 From: Ed Sanders Date: Mon, 4 Nov 2019 14:54:53 +0000 Subject: [PATCH] Move more code to ArticleTargetSaver * Add a postWikitext method and split out postContent from postHtml * Move saveSuccess handling into postContent promise * Connect promise directly to saveComplete instead * Pass whole response.visualeditoredit object, instead of splitting into variadic arguments for saveComplete. * [DEPRECATION] Make serialize return the postHtml promise and deprecate passing a callback. Change-Id: I905737515578000b2b87214c92e8b9fe9e82f6b7 --- .../init/targets/ve.init.mw.ArticleTarget.js | 272 ++++++------------ .../ve.init.mw.DesktopArticleTarget.js | 87 +++--- .../targets/ve.init.mw.MobileArticleTarget.js | 8 +- .../init/ve.init.mw.ArticleTargetEvents.js | 15 +- .../preinit/ve.init.mw.ArticleTargetSaver.js | 149 +++++++--- .../ve-mw/ui/dialogs/ve.ui.MWSaveDialog.js | 9 +- 6 files changed, 260 insertions(+), 280 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 e26edf4908..65e90261d5 100644 --- a/modules/ve-mw/init/targets/ve.init.mw.ArticleTarget.js +++ b/modules/ve-mw/init/targets/ve.init.mw.ArticleTarget.js @@ -94,16 +94,7 @@ OO.inheritClass( ve.init.mw.ArticleTarget, ve.init.mw.Target ); /** * @event save - * @param {string} html Rendered page HTML from server - * @param {string} categoriesHtml Rendered categories HTML from server - * @param {number} newid New revision id, undefined if unchanged - * @param {boolean} isRedirect Whether this page is a redirect or not - * @param {string} displayTitle What HTML to show as the page title - * @param {Object} lastModified Object containing user-formatted date - * and time strings, or undefined if we made no change. - * @param {string} contentSub HTML to show as the content subtitle - * @param {Array} modules The modules to be loaded on the page - * @param {Object} jsconfigvars The mw.config values needed on the page + * @param {Object} data Save data from the API, see ve.init.mw.ArticleTarget#saveComplete * Fired immediately after a save is successfully completed */ @@ -442,7 +433,7 @@ ve.init.mw.ArticleTarget.prototype.parseMetadata = function ( response ) { this.retriedRevIdConflict = true; // TODO this retries both requests, in RESTbase mode we should only retry // the request that gave us the lower revid - this.loading = false; + this.loading = null; // HACK: Load with explicit revid to hopefully prevent this from happening again this.requestedRevId = Math.max( docRevId, this.revid ); this.load(); @@ -517,7 +508,7 @@ ve.init.mw.ArticleTarget.prototype.documentReady = function () { } ) ); - this.loading = false; + this.loading = null; this.edited = this.fromEditedState; // Parent method @@ -637,84 +628,51 @@ ve.init.mw.ArticleTarget.prototype.restoreAccessKeys = function () { * @fires loadError */ ve.init.mw.ArticleTarget.prototype.loadFail = function () { - this.loading = false; + this.loading = null; this.emit( 'loadError' ); }; -/** - * Handle a successful save request. - * - * This method is called within the context of a target instance. - * - * @param {HTMLDocument} doc HTML document we tried to save - * @param {Object} saveData Options that were used - * @param {Object} response Response data - * @param {string} status Text status message - */ -ve.init.mw.ArticleTarget.prototype.saveSuccess = function ( doc, saveData, response ) { - var data = response.visualeditoredit; - this.saving = false; - if ( !data ) { - this.saveFail( doc, saveData, false, null, 'Invalid response from server', response ); - } else if ( data.result !== 'success' ) { - // Note, this could be any of db failure, hookabort, badtoken or even a captcha - this.saveFail( doc, saveData, false, null, 'Save failure', response ); - } else if ( typeof data.content !== 'string' ) { - this.saveFail( doc, saveData, false, null, 'Invalid HTML content in response from server', response ); - } else { - this.saveComplete( - data.content, - data.categorieshtml, - data.newrevid, - data.isRedirect, - data.displayTitleHtml, - data.lastModified, - data.contentSub, - data.modules, - data.jsconfigvars - ); - } -}; - /** * Handle successful DOM save event. * - * @param {string} html Rendered page HTML from server - * @param {string} categoriesHtml Rendered categories HTML from server - * @param {number} newid New revision id, undefined if unchanged - * @param {boolean} isRedirect Whether this page is a redirect or not - * @param {string} displayTitle What HTML to show as the page title - * @param {Object} lastModified Object containing user-formatted date + * @param {Object} data Save data from the API + * @param {string} data.content Rendered page HTML from server + * @param {string} data.categorieshtml Rendered categories HTML from server + * @param {number} data.newrevid New revision id, undefined if unchanged + * @param {boolean} data.isRedirect Whether this page is a redirect or not + * @param {string} data.displayTitleHtml What HTML to show as the page title + * @param {Object} data.lastModified Object containing user-formatted date * and time strings, or undefined if we made no change. - * @param {string} contentSub HTML to show as the content subtitle - * @param {Array} modules The modules to be loaded on the page - * @param {Object} jsconfigvars The mw.config values needed on the page + * @param {string} data.contentSub HTML to show as the content subtitle + * @param {Array} data.modules The modules to be loaded on the page + * @param {Object} data.jsconfigvars The mw.config values needed on the page * @fires save */ -ve.init.mw.ArticleTarget.prototype.saveComplete = function () { +ve.init.mw.ArticleTarget.prototype.saveComplete = function ( data ) { this.editSummaryValue = null; this.initialEditSummary = null; this.saveDeferred.resolve(); - this.emit.apply( this, [ 'save' ].concat( Array.prototype.slice.call( arguments ) ) ); + this.emit( 'save', data ); }; /** * Handle an unsuccessful save request. * + * TODO: This code should be mostly moved to ArticleTargetSaver, + * in particular the badtoken error handling. + * * @param {HTMLDocument} doc HTML document we tried to save * @param {Object} saveData Options that were used * @param {boolean} wasRetry Whether this was a retry after a 'badtoken' error - * @param {Object} jqXHR - * @param {string} status Text status message - * @param {Object|null} data API response data + * @param {string} code Error code + * @param {Object|null} data Full API response data, or XHR error details */ -ve.init.mw.ArticleTarget.prototype.saveFail = function ( doc, saveData, wasRetry, jqXHR, status, data ) { +ve.init.mw.ArticleTarget.prototype.saveFail = function ( doc, saveData, wasRetry, code, data ) { var name, handler, i, error, saveErrorHandlerFactory = ve.init.mw.saveErrorHandlerFactory, target = this; - this.saving = false; this.pageDeletedWarning = false; // Handle empty response @@ -918,61 +876,13 @@ ve.init.mw.ArticleTarget.prototype.editConflict = function () { this.saveDialog.swapPanel( 'conflict' ); }; -/** - * Handle a successful serialize request. - * - * This method is called within the context of a target instance. - * - * @static - * @param {Object} response API response data - * @param {string} status Text status message - * @fires serializeComplete - */ -ve.init.mw.ArticleTarget.prototype.serializeSuccess = function ( response ) { - var data = response.visualeditoredit; - this.serializing = false; - if ( !data && !response.error ) { - this.serializeFail( null, 'Invalid response from server', null ); - } else if ( response.error ) { - this.serializeFail( - null, 'Unsuccessful request: ' + response.error.info, null - ); - } else if ( data.result === 'error' ) { - this.serializeFail( null, 'Server error', null ); - } else if ( typeof data.content !== 'string' ) { - this.serializeFail( - null, 'No Wikitext content in response from server', null - ); - } else { - if ( typeof this.serializeCallback === 'function' ) { - this.serializeCallback( data.content ); - this.emit( 'serializeComplete' ); - delete this.serializeCallback; - } - } -}; - -/** - * Handle an unsuccessful serialize request. - * - * This method is called within the context of a target instance. - * - * @param {jqXHR|null} jqXHR - * @param {string} status Text status message - * @param {Mixed|null} error HTTP status text - * @fires serializeError - */ -ve.init.mw.ArticleTarget.prototype.serializeFail = function () { - this.serializing = false; - this.emit( 'serializeError' ); -}; - /** * Handle clicks on the review button in the save dialog. * * @fires saveReview */ ve.init.mw.ArticleTarget.prototype.onSaveDialogReview = function () { + var target = this; if ( !this.saveDialog.hasDiff ) { this.emit( 'saveReview' ); this.saveDialog.pushPending(); @@ -980,7 +890,9 @@ ve.init.mw.ArticleTarget.prototype.onSaveDialogReview = function () { // Has no callback, handled via target.showChangesDiff this.showChanges( this.getDocToSave() ); } else { - this.serialize( this.getDocToSave(), this.onSaveDialogReviewComplete.bind( this ) ); + this.serialize( this.getDocToSave() ).then( function ( data ) { + target.onSaveDialogReviewComplete( data.content ); + } ); } } else { this.saveDialog.swapPanel( 'review' ); @@ -1049,7 +961,7 @@ ve.init.mw.ArticleTarget.prototype.bindSaveDialogClearDiff = function () { /** * Handle completed serialize request for diff views for new page creations. * - * @param {string} wikitext + * @param {string} wikitext Wikitext */ ve.init.mw.ArticleTarget.prototype.onSaveDialogReviewComplete = function ( wikitext ) { this.bindSaveDialogClearDiff(); @@ -1122,16 +1034,16 @@ ve.init.mw.ArticleTarget.prototype.getVisualDiffGeneratorPromise = function () { * Handle clicks on the resolve conflict button in the conflict dialog. */ ve.init.mw.ArticleTarget.prototype.onSaveDialogResolveConflict = function () { - var fields = { wpSave: 1 }; + var fields = { wpSave: 1 }, + target = this; if ( this.getSurface().getMode() === 'source' && this.section !== null ) { fields.section = this.section; } // Get Wikitext from the DOM, and set up a submit call when it's done - this.serialize( - this.getDocToSave(), - this.submitWithSaveFields.bind( this, fields ) - ); + this.serialize( this.getDocToSave() ).then( function ( data ) { + target.submitWithSaveFields( fields, data.content ); + } ); }; /** @@ -1162,7 +1074,7 @@ ve.init.mw.ArticleTarget.prototype.onSaveDialogClose = function () { * * @param {jQuery.Promise} [dataPromise] Promise for pending request, if any * @return {jQuery.Promise} Data promise -*/ + */ ve.init.mw.ArticleTarget.prototype.load = function ( dataPromise ) { // Prevent duplicate requests if ( this.loading ) { @@ -1192,8 +1104,8 @@ ve.init.mw.ArticleTarget.prototype.load = function ( dataPromise ) { ve.init.mw.ArticleTarget.prototype.clearState = function () { this.restoreAccessKeys(); this.clearPreparedCacheKey(); - this.loading = false; - this.saving = false; + this.loading = null; + this.saving = null; this.clearDiff(); this.serializing = false; this.submitting = false; @@ -1380,14 +1292,12 @@ ve.init.mw.ArticleTarget.prototype.clearPreparedCacheKey = function () { * @return {jQuery.Promise} Promise which resolves/rejects when saving is complete/fails */ ve.init.mw.ArticleTarget.prototype.tryWithPreparedCacheKey = function ( doc, extraData, eventName ) { - var data, htmlOrCacheKeyPromise, api, + var data, htmlOrCacheKeyPromise, target = this; if ( this.getSurface().getMode() === 'source' ) { - data = ve.extendObject( {}, extraData, { - wikitext: doc, - format: 'json' - } ); + data = ve.copy( extraData ); + if ( this.section !== null ) { data.section = this.section; } @@ -1395,11 +1305,12 @@ ve.init.mw.ArticleTarget.prototype.tryWithPreparedCacheKey = function ( doc, ext data.sectiontitle = this.sectionTitle.getValue(); data.summary = undefined; } - api = this.getContentApi(); - if ( data.token ) { - return api.post( data, { contentType: 'multipart/form-data' } ); - } - return api.postWithToken( 'csrf', data, { contentType: 'multipart/form-data' } ); + + return mw.libs.ve.targetSaver.postWikitext( + doc, + data, + { api: target.getContentApi() } + ); } // getPreparedCacheKey resolves with { cacheKey: ..., html: ... } or rejects. @@ -1557,21 +1468,18 @@ ve.init.mw.ArticleTarget.prototype.getSaveOptions = function () { * - {boolean} minor Edit is a minor edit * - {boolean} watch Watch the page * @param {boolean} [isRetry=false] Whether this is a retry after a 'badtoken' error - * @return {boolean} Saving has been started -*/ + * @return {jQuery.Promise} Save promise, see mw.libs.ve.targetSaver.postHtml + */ ve.init.mw.ArticleTarget.prototype.save = function ( doc, options, isRetry ) { - var data; + var data, promise, + target = this; + // Prevent duplicate requests if ( this.saving ) { - return false; + return this.saving; } data = ve.extendObject( {}, options, { - action: 'visualeditoredit', - paction: 'save', - errorformat: 'html', - errorlang: mw.config.get( 'wgUserLanguage' ), - errorsuselocal: 1, page: this.getPageName(), oldid: this.revid, basetimestamp: this.baseTimeStamp, @@ -1581,11 +1489,14 @@ ve.init.mw.ArticleTarget.prototype.save = function ( doc, options, isRetry ) { token: this.editToken } ); - this.saving = this.tryWithPreparedCacheKey( doc, data, 'save' ) - .done( this.saveSuccess.bind( this, doc, data ) ) - .fail( this.saveFail.bind( this, doc, data, !!isRetry ) ); + promise = this.saving = this.tryWithPreparedCacheKey( doc, data, 'save' ) + .done( this.saveComplete.bind( this ) ) + .fail( this.saveFail.bind( this, doc, data, !!isRetry ) ) + .always( function () { + target.saving = null; + } ); - return true; + return promise; }; /** @@ -1623,32 +1534,21 @@ ve.init.mw.ArticleTarget.prototype.clearDiff = function () { * @return {jQuery.Promise} Promise which resolves with the wikitext diff, or rejects with an error * @fires showChanges * @fires showChangesError -*/ + */ ve.init.mw.ArticleTarget.prototype.getWikitextDiffPromise = function ( doc ) { var target = this; if ( !this.wikitextDiffPromise ) { this.wikitextDiffPromise = this.tryWithPreparedCacheKey( doc, { - action: 'visualeditoredit', paction: 'diff', page: this.getPageName(), oldid: this.revid, etag: this.etag - }, 'diff' ).then( function ( response ) { - var data = response.visualeditoredit; - if ( !data && !response.error ) { - return ve.createDeferred().reject( 'Invalid response from server' ).promise(); - } else if ( response.error ) { - return ve.createDeferred().reject( response.error.info ).promise(); - } else if ( data.result === 'nochanges' ) { + }, 'diff' ).then( function ( data ) { + if ( data.result === 'nochanges' ) { target.emit( 'noChanges' ); return null; - } else if ( data.result !== 'success' ) { - return ve.createDeferred().reject( 'Failed request: ' + data.result ).promise(); - } else if ( typeof data.diff !== 'string' ) { - return ve.createDeferred().reject( 'Invalid HTML content in response from server' ).promise(); - } else { - return data.diff; } + return data.diff; } ); this.wikitextDiffPromise .done( this.emit.bind( this, 'showChanges' ) ) @@ -1668,7 +1568,7 @@ ve.init.mw.ArticleTarget.prototype.getWikitextDiffPromise = function ( doc ) { * @param {Object} fields Other form fields to add (e.g. wpSummary, wpWatchthis, etc.). To actually * save the wikitext, add { wpSave: 1 }. To go to the diff view, add { wpDiff: 1 }. * @return {boolean} Submitting has been started -*/ + */ ve.init.mw.ArticleTarget.prototype.submit = function ( wikitext, fields ) { var key, $form, params; @@ -1707,33 +1607,42 @@ ve.init.mw.ArticleTarget.prototype.submit = function ( wikitext, fields ) { * * This method performs an asynchronous action and uses a callback function to handle the result. * - * target.serialize( - * doc, - * function ( wikitext ) { - * // Do something with the loaded DOM - * } - * ); + * target.serialize( doc ).then( function ( data ) { + * // Do something with data.content (wikitext) + * } ); * * @param {HTMLDocument} doc Document to serialize - * @param {Function} callback Function to call when complete, accepts error and wikitext arguments - * @return {boolean} Serializing has been started -*/ + * @param {Function} [callback] Optional callback to run after. + * Deprecated in favor of using the returned promise. + * @return {jQuery.Promise} Serialize promise, see mw.libs.ve.targetSaver.postHtml + */ ve.init.mw.ArticleTarget.prototype.serialize = function ( doc, callback ) { + var promise, + target = this; // Prevent duplicate requests if ( this.serializing ) { - return false; + return this.serializing; } - this.serializeCallback = callback; - this.serializing = this.tryWithPreparedCacheKey( doc, { - action: 'visualeditoredit', + promise = this.serializing = this.tryWithPreparedCacheKey( doc, { paction: 'serialize', page: this.getPageName(), oldid: this.revid, etag: this.etag }, 'serialize' ) - .done( this.serializeSuccess.bind( this ) ) - .fail( this.serializeFail.bind( this ) ); - return true; + .done( this.emit.bind( this, 'serializeComplete' ) ) + .fail( this.emit.bind( this, 'serializeError' ) ) + .always( function () { + target.serializing = null; + } ); + + if ( callback ) { + OO.ui.warnDeprecation( 'Passing a callback to ve.init.mw.ArticleTarget#serialize is deprecated. Use the returned promise instead.' ); + promise.then( function ( data ) { + callback.call( target, data.content ); + } ); + } + + return promise; }; /** @@ -2284,10 +2193,8 @@ ve.init.mw.ArticleTarget.prototype.switchToWikitextEditor = function ( modified */ ve.init.mw.ArticleTarget.prototype.getWikitextDataPromiseForDoc = function ( modified ) { var target = this; - this.serialize( this.getDocToSave() ); - return this.serializing.then( function ( response ) { + return this.serialize( this.getDocToSave() ).then( function ( data ) { // HACK - add parameters the API doesn't provide for a VE->WT switch - var data = response.visualeditoredit; data.etag = target.etag; data.fromEditedState = modified; data.notices = target.remoteNotices; @@ -2297,7 +2204,8 @@ ve.init.mw.ArticleTarget.prototype.getWikitextDataPromiseForDoc = function ( mod data.oldid = target.revid; data.canEdit = target.canEdit; data.checkboxesDef = target.checkboxesDef; - return response; + // Wrap up like a response object as that is what dataPromise is expected to be + return { visualeditoredit: data }; } ); }; diff --git a/modules/ve-mw/init/targets/ve.init.mw.DesktopArticleTarget.js b/modules/ve-mw/init/targets/ve.init.mw.DesktopArticleTarget.js index cb8b04e2ce..b202cce8a6 100644 --- a/modules/ve-mw/init/targets/ve.init.mw.DesktopArticleTarget.js +++ b/modules/ve-mw/init/targets/ve.init.mw.DesktopArticleTarget.js @@ -685,15 +685,7 @@ ve.init.mw.DesktopArticleTarget.prototype.loadFail = function ( code, errorDetai // Don't show an error if the load was manually aborted // The response.status check here is to catch aborts triggered by navigation away from the page - if ( - errorDetails && - Object.prototype.hasOwnProperty.call( errorDetails, 'error' ) && - Object.prototype.hasOwnProperty.call( errorDetails.error, 'info' ) - ) { - errorInfo = errorDetails.error.info; - } else { - errorInfo = errorDetails; - } + errorInfo = ve.getProp( errorDetails, 'error', 'info' ) || errorDetails; if ( !errorDetails || errorDetails.statusText !== 'abort' ) { if ( code === 'http' || code === 'error' ) { @@ -923,9 +915,7 @@ ve.init.mw.DesktopArticleTarget.prototype.onViewTabClick = function ( e ) { /** * @inheritdoc */ -ve.init.mw.DesktopArticleTarget.prototype.saveComplete = function ( - html, categoriesHtml, newid, isRedirect, displayTitle, lastModified, contentSub, modules, jsconfigvars -) { +ve.init.mw.DesktopArticleTarget.prototype.saveComplete = function ( data ) { var newUrlParams, watchChecked, watch, target = this; @@ -937,9 +927,9 @@ ve.init.mw.DesktopArticleTarget.prototype.saveComplete = function ( this.teardown().then( function () { // This is a page creation or restoration, refresh the page - newUrlParams = newid === undefined ? {} : { venotify: target.restoring ? 'restored' : 'created' }; + newUrlParams = data.newrevid === undefined ? {} : { venotify: target.restoring ? 'restored' : 'created' }; - if ( isRedirect ) { + if ( data.isRedirect ) { newUrlParams.redirect = 'no'; } location.href = target.viewUri.extend( newUrlParams ); @@ -966,38 +956,38 @@ ve.init.mw.DesktopArticleTarget.prototype.saveComplete = function ( // we don't want to go back into oldid mode anyway this.requestedRevId = undefined; - if ( newid !== undefined ) { + if ( data.newrevid !== undefined ) { mw.config.set( { - wgCurRevisionId: newid, - wgRevisionId: newid + wgCurRevisionId: data.newrevid, + wgRevisionId: data.newrevid } ); - this.revid = newid; - this.currentRevisionId = newid; + this.revid = data.newrevid; + this.currentRevisionId = data.newrevid; } // Update module JS config values and notify ResourceLoader of any new // modules needed to be added to the page - mw.config.set( jsconfigvars ); + mw.config.set( data.jsconfigvars ); // Also load postEdit in case it's needed, below. - mw.loader.load( modules.concat( [ 'mediawiki.action.view.postEdit' ] ) ); + mw.loader.load( data.modules.concat( [ 'mediawiki.action.view.postEdit' ] ) ); mw.config.set( { - wgIsRedirect: !!isRedirect + wgIsRedirect: !!data.isRedirect } ); this.saveDialog.reset(); this.replacePageContent( - html, - categoriesHtml, - displayTitle, - lastModified, - contentSub + data.content, + data.categorieshtml, + data.displayTitleHtml, + data.lastModified, + data.contentSub ); - if ( newid !== undefined ) { + if ( data.newrevid !== undefined ) { $( '#t-permalink a, #coll-download-as-rl a' ).each( function () { var uri = new mw.Uri( $( this ).attr( 'href' ) ); - uri.query.oldid = newid; + uri.query.oldid = data.newrevid; $( this ).attr( 'href', uri.toString() ); } ); } @@ -1005,7 +995,7 @@ ve.init.mw.DesktopArticleTarget.prototype.saveComplete = function ( // Tear down the target now that we're done saving // Not passing trackMechanism because this isn't an abort action this.tryTeardown( true ); - if ( newid !== undefined ) { + if ( data.newrevid !== undefined ) { mw.hook( 'postEdit' ).fire( { // The following messages are used here: // * postedit-confirmation-published @@ -1019,17 +1009,25 @@ ve.init.mw.DesktopArticleTarget.prototype.saveComplete = function ( /** * @inheritdoc */ -ve.init.mw.DesktopArticleTarget.prototype.serializeFail = function ( jqXHR, status ) { +ve.init.mw.DesktopArticleTarget.prototype.serialize = function () { // Parent method - ve.init.mw.DesktopArticleTarget.super.prototype.serializeFail.apply( this, arguments ); + var promise = ve.init.mw.DesktopArticleTarget.super.prototype.serialize.apply( this, arguments ), + target = this; - OO.ui.alert( ve.msg( 'visualeditor-serializeerror', status ) ); + return promise.fail( function ( error, response ) { + OO.ui.alert( + $( ve.htmlMsg( + 'visualeditor-serializeerror', + $( '' ).append( target.extractErrorMessages( response ) )[ 0 ] + ) ) + ); - // It's possible to get here while the save dialog has never been opened (if the user uses - // the switch to source mode option) - if ( this.saveDialog ) { - this.saveDialog.popPending(); - } + // It's possible to get here while the save dialog has never been opened (if the user uses + // the switch to source mode option) + if ( target.saveDialog ) { + target.saveDialog.popPending(); + } + } ); }; /** @@ -1566,14 +1564,11 @@ ve.init.mw.DesktopArticleTarget.prototype.switchToFallbackWikitextEditor = funct location.href = uri.toString(); } ); } else { - this.serialize( - this.getDocToSave(), - function ( wikitext ) { - ve.track( 'activity.editor-switch', { action: 'source-desktop' } ); - ve.track( 'mwedit.abort', { type: 'switchwith', mechanism: 'navigate', mode: 'visual' } ); - target.submitWithSaveFields( { wpDiff: true, wpAutoSummary: '' }, wikitext ); - } - ); + this.serialize( this.getDocToSave() ).then( function ( data ) { + ve.track( 'activity.editor-switch', { action: 'source-desktop' } ); + ve.track( 'mwedit.abort', { type: 'switchwith', mechanism: 'navigate', mode: 'visual' } ); + target.submitWithSaveFields( { wpDiff: true, wpAutoSummary: '' }, data.content ); + } ); } }; diff --git a/modules/ve-mw/init/targets/ve.init.mw.MobileArticleTarget.js b/modules/ve-mw/init/targets/ve.init.mw.MobileArticleTarget.js index c09c3987d5..9ef0135656 100644 --- a/modules/ve-mw/init/targets/ve.init.mw.MobileArticleTarget.js +++ b/modules/ve-mw/init/targets/ve.init.mw.MobileArticleTarget.js @@ -426,22 +426,22 @@ ve.init.mw.MobileArticleTarget.prototype.showSaveDialog = function () { /** * @inheritdoc */ -ve.init.mw.MobileArticleTarget.prototype.saveComplete = function ( html, categoriesHtml, newRevId ) { +ve.init.mw.MobileArticleTarget.prototype.saveComplete = function ( data ) { // TODO: parsing this is expensive just for the section details. We should // change MobileFrontend+this to behave like desktop does and just rerender // the page with the provided HTML (T219420). - var fragment = this.getSectionFragmentFromPage( $.parseHTML( html ) ); + var fragment = this.getSectionFragmentFromPage( $.parseHTML( data.content ) ); // Parent method ve.init.mw.MobileArticleTarget.super.prototype.saveComplete.apply( this, arguments ); this.overlay.sectionId = fragment; - this.overlay.onSaveComplete( newRevId ); + this.overlay.onSaveComplete( data.newrevid ); }; /** * @inheritdoc */ -ve.init.mw.MobileArticleTarget.prototype.saveFail = function ( doc, saveData, wasRetry, jqXHR, status, data ) { +ve.init.mw.MobileArticleTarget.prototype.saveFail = function ( doc, saveData, wasRetry, code, data ) { // parent method ve.init.mw.MobileArticleTarget.super.prototype.saveFail.apply( this, arguments ); diff --git a/modules/ve-mw/init/ve.init.mw.ArticleTargetEvents.js b/modules/ve-mw/init/ve.init.mw.ArticleTargetEvents.js index 1936dfcf25..dc7a295fad 100644 --- a/modules/ve-mw/init/ve.init.mw.ArticleTargetEvents.js +++ b/modules/ve-mw/init/ve.init.mw.ArticleTargetEvents.js @@ -118,24 +118,15 @@ ve.init.mw.ArticleTargetEvents.prototype.onSaveInitiated = function () { /** * Track when the save is complete * - * @param {string} content Rendered page HTML from server - * @param {string} categoriesHtml Rendered categories HTML from server - * @param {number} newRevId New revision id, undefined if unchanged - * @param {boolean} isRedirect Whether this page is a redirect or not - * @param {string} displayTitle What HTML to show as the page title - * @param {Object} lastModified Object containing user-formatted date - * and time strings, or undefined if we made no change. - * @param {string} contentSub HTML to show as the content subtitle - * @param {Array} modules The modules to be loaded on the page - * @param {Object} jsconfigvars The mw.config values needed on the page + * @param {Object} data Save data from the API, see ve.init.mw.ArticleTarget#saveComplete */ -ve.init.mw.ArticleTargetEvents.prototype.onSaveComplete = function ( content, categoriesHtml, newRevId ) { +ve.init.mw.ArticleTargetEvents.prototype.onSaveComplete = function ( data ) { this.trackTiming( 'performance.user.saveComplete', { duration: ve.now() - this.timings.saveInitiated } ); this.timings.saveRetries = 0; this.track( 'mwedit.saveSuccess', { timing: ve.now() - this.timings.saveInitiated + ( this.timings.serializeForCache || 0 ), // eslint-disable-next-line camelcase - revision_id: newRevId + revision_id: data.newrevid } ); }; diff --git a/modules/ve-mw/preinit/ve.init.mw.ArticleTargetSaver.js b/modules/ve-mw/preinit/ve.init.mw.ArticleTargetSaver.js index c66b684d13..a6f9cb2d85 100644 --- a/modules/ve-mw/preinit/ve.init.mw.ArticleTargetSaver.js +++ b/modules/ve-mw/preinit/ve.init.mw.ArticleTargetSaver.js @@ -131,6 +131,24 @@ } ); }, + /** + * Post wikitext to the API. + * + * By default uses action=visualeditoredit, paction=save. + * + * @param {string} wikitext Wikitext to post. Deflating is optional but recommended. + * @param {Object} [extraData] Extra data to send to the API + * @param {Object} [options] Options + * @param {mw.Api} [options.api] Api to use + * @param {Function} [options.now] Function returning current time in milliseconds for tracking, e.g. ve.now + * @param {Function} [options.track] Tracking function + * @param {string} [options.eventName] Event name for tracking + * @return {jQuery.Promise} Promise which resolves with API save data, or rejects with error details + */ + postWikitext: function ( wikitext, extraData, options ) { + return this.postContent( $.extend( { wikitext: wikitext }, extraData ), options ); + }, + /** * Post HTML to the API. * @@ -141,15 +159,54 @@ * @param {string} [cacheKey] Optional cache key of HTML stashed on server. * @param {Object} [extraData] Extra data to send to the API * @param {Object} [options] Options + * @return {jQuery.Promise} Promise which resolves with API save data, or rejects with error details + */ + postHtml: function ( html, cacheKey, extraData, options ) { + var data, + saver = this; + + if ( cacheKey ) { + data = $.extend( { cachekey: cacheKey }, extraData ); + } else { + data = $.extend( { html: html }, extraData ); + } + return this.postContent( data, options ).then( + null, + function ( code, response ) { + // This cache key is evidently bad, clear it + if ( options.onCacheKeyFail ) { + options.onCacheKeyFail(); + } + if ( code === 'badcachekey' ) { + // If the cache key failed, try again without the cache key + return saver.postHtml( + html, + null, + extraData, + options + ); + } + // Failed for some other reason - let caller handle it. + return $.Deferred().reject( code, response ).promise(); + } + ); + }, + + /** + * Post content to the API. + * + * By default uses action=visualeditoredit, paction=save. + * + * @param {string} data Content data + * @param {Object} [options] Options * @param {mw.Api} [options.api] Api to use * @param {Function} [options.now] Function returning current time in milliseconds for tracking, e.g. ve.now * @param {Function} [options.track] Tracking function * @param {string} [options.eventName] Event name for tracking - * @return {jQuery.Promise} Promise which resolves if the post was successful + * @return {jQuery.Promise} Promise which resolves with API save data, or rejects with error details */ - postHtml: function ( html, cacheKey, extraData, options ) { - var request, fullEventName, api, data, start, - saver = this; + postContent: function ( data, options ) { + var request, api, start; options = options || {}; api = options.api || new mw.Api(); @@ -158,17 +215,15 @@ start = options.now(); } - if ( cacheKey ) { - data = $.extend( {}, extraData, { cachekey: cacheKey } ); - } else { - data = $.extend( {}, extraData, { html: html } ); - } data = $.extend( { action: 'visualeditoredit', paction: 'save', format: 'json', - formatversion: 2 + formatversion: 2, + errorformat: 'html', + errorlang: mw.config.get( 'wgUserLanguage' ), + errorsuselocal: true }, data ); @@ -180,7 +235,9 @@ } return request.then( function ( response, jqxhr ) { - var eventData; + var eventData, fullEventName, error, + data = response.visualeditoredit; + // Log data about the request if eventName was set if ( options.track && options.eventName ) { eventData = { @@ -188,44 +245,68 @@ duration: options.now() - start }; fullEventName = 'performance.system.' + options.eventName + - ( cacheKey ? '.withCacheKey' : '.withoutCacheKey' ); + ( data.cachekey ? '.withCacheKey' : '.withoutCacheKey' ); options.track( fullEventName, eventData ); } - return jqxhr; + + // TODO: i18n + if ( !data ) { + error = { + code: 'invalidresponse', + html: 'Invalid response from server' + }; + } else if ( data.result !== 'success' ) { + // Note, this could be any of db failure, hookabort, badtoken or even a captcha + error = { + code: 'failure', + html: 'Save failure: ' + mw.html.escape( data.result ) + }; + } else { + // paction specific errors + switch ( data.paction ) { + case 'save': + case 'serialize': + if ( typeof data.content !== 'string' ) { + error = { + code: 'invalidcontent', + html: 'Invalid content in response from server' + }; + } + break; + case 'diff': + if ( typeof data.diff !== 'string' ) { + error = { + code: 'invalidcontent', + html: 'Invalid content in response from server' + }; + } + break; + } + } + + if ( error ) { + // Use the same format as API errors + return $.Deferred().reject( error.code, { errors: [ error ] } ).promise(); + } + return data; }, - function ( errorName, errorObject ) { - var eventData, - responseText = OO.getProp( errorObject, 'xhr', 'responseText' ); + function ( code, response ) { + var eventData, fullEventName, + responseText = OO.getProp( response, 'xhr', 'responseText' ); if ( responseText && options.track && options.eventName ) { eventData = { bytes: require( 'mediawiki.String' ).byteLength( responseText ), duration: options.now() - start }; - if ( errorName === 'badcachekey' ) { + if ( code === 'badcachekey' ) { fullEventName = 'performance.system.' + options.eventName + '.badCacheKey'; } else { fullEventName = 'performance.system.' + options.eventName + '.withoutCacheKey'; } options.track( fullEventName, eventData ); } - // This cache key is evidently bad, clear it - if ( options.onCacheKeyFail ) { - options.onCacheKeyFail(); - } - if ( errorName === 'badcachekey' ) { - // If the cache key failed, try again without the cache key - return saver.postHtml( - html, - null, - extraData, - options - ); - } else { - // Failed for some other reason - let caller handle it. - // FIXME Can't just `return this` because all callers are broken. - return $.Deferred().reject( null, errorName, errorObject ).promise(); - } + return $.Deferred().reject( code, response ).promise(); } ); } diff --git a/modules/ve-mw/ui/dialogs/ve.ui.MWSaveDialog.js b/modules/ve-mw/ui/dialogs/ve.ui.MWSaveDialog.js index 726307345d..e29c2416f4 100644 --- a/modules/ve-mw/ui/dialogs/ve.ui.MWSaveDialog.js +++ b/modules/ve-mw/ui/dialogs/ve.ui.MWSaveDialog.js @@ -173,8 +173,13 @@ ve.ui.MWSaveDialog.prototype.setDiffAndReview = function ( wikitextDiffPromise, $( '
' ).addClass( 've-ui-mwSaveDialog-no-changes' ).text( ve.msg( 'visualeditor-diff-no-changes' ) ) ); } - }, function ( error ) { - dialog.$reviewWikitextDiff.empty().append( error ); + }, function ( code, errorObject ) { + dialog.$reviewWikitextDiff.empty().append( + new OO.ui.MessageWidget( { + type: 'error', + label: ve.init.target.extractErrorMessages( errorObject ) + } ).$element + ); } ).always( function () { dialog.updateSize(); } );