From 2e33841b5afdec6b0fba96767f303dc59a705857 Mon Sep 17 00:00:00 2001 From: Ed Sanders Date: Wed, 6 Mar 2019 11:30:26 +0000 Subject: [PATCH] Fix section param in historical diffs Passing section=undefined resulted in no
tags being unwrapped, which broke the historical diff. Ensure 'null' is used instead for whole documents. Bug: T217752 Change-Id: Iec33e6ab83bfbd011df9dc05f4daccc26b1df8b5 --- .../ve-mw/init/ve.init.mw.ArticleTarget.js | 2 +- modules/ve-mw/init/ve.init.mw.DiffLoader.js | 19 +++++++++++-------- modules/ve-mw/init/ve.init.mw.Target.js | 2 +- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/modules/ve-mw/init/ve.init.mw.ArticleTarget.js b/modules/ve-mw/init/ve.init.mw.ArticleTarget.js index ae02a81449..37d5b705d6 100644 --- a/modules/ve-mw/init/ve.init.mw.ArticleTarget.js +++ b/modules/ve-mw/init/ve.init.mw.ArticleTarget.js @@ -1075,7 +1075,7 @@ ve.init.mw.ArticleTarget.prototype.getVisualDiffGeneratorPromise = function () { // re-fetch the HTML target.originalDmDocPromise = $.Deferred().resolve( target.constructor.static.createModelFromDom( target.doc, 'visual' ) ).promise(); } else { - target.originalDmDocPromise = mw.libs.ve.diffLoader.fetchRevision( target.revid, target.getPageName(), undefined, target.section !== null ? target.section : undefined ); + target.originalDmDocPromise = mw.libs.ve.diffLoader.fetchRevision( target.revid, target.getPageName(), target.section ); } } diff --git a/modules/ve-mw/init/ve.init.mw.DiffLoader.js b/modules/ve-mw/init/ve.init.mw.DiffLoader.js index 09fa78260a..3ce3ef436e 100644 --- a/modules/ve-mw/init/ve.init.mw.DiffLoader.js +++ b/modules/ve-mw/init/ve.init.mw.DiffLoader.js @@ -22,7 +22,7 @@ * Get a ve.dm.Document model from a Parsoid response * * @param {Object} response Parsoid response from the VisualEditor API - * @param {number} [section] Section + * @param {number|null} section Section. Null for the whole document. * @return {ve.dm.Document|null} Document, or null if an invalid response */ getModelFromResponse: function ( response, section ) { @@ -31,7 +31,7 @@ metadataIdRegExp = ve.init.platform.getMetadataIdRegExp(), data = response ? ( response.visualeditor || response.visualeditoredit ) : null; if ( data && typeof data.content === 'string' ) { - doc = targetClass.static.parseDocument( data.content, 'visual', section, true ); + doc = targetClass.static.parseDocument( data.content, 'visual', section, section !== null ); // Strip RESTBase IDs Array.prototype.forEach.call( doc.querySelectorAll( '[id^="mw"]' ), function ( element ) { if ( element.id.match( metadataIdRegExp ) ) { @@ -48,15 +48,18 @@ * * @param {number} revId Revision ID * @param {string} [pageName] Page name, defaults to wgRelevantPageName + * @param {number|null} [section=null] Section. Null for the whole document. * @param {jQuery.Promise} [parseDocumentModulePromise] Promise which resolves when Target#parseDocument is available - * @param {number} [section] Section * @return {jQuery.Promise} Promise which resolves with a document model */ - fetchRevision: function ( revId, pageName, parseDocumentModulePromise, section ) { - var cacheKey = revId + ( section !== undefined ? '/' + section : '' ); + fetchRevision: function ( revId, pageName, section, parseDocumentModulePromise ) { + var cacheKey; - parseDocumentModulePromise = parseDocumentModulePromise || $.Deferred().resolve().promise(); pageName = pageName || mw.config.get( 'wgRelevantPageName' ); + parseDocumentModulePromise = parseDocumentModulePromise || $.Deferred().resolve().promise(); + section = section !== undefined ? section : null; + + cacheKey = revId + ( section !== null ? '/' + section : '' ); revCache[ cacheKey ] = revCache[ cacheKey ] || mw.libs.ve.targetLoader.requestParsoidData( pageName, { oldId: revId, targetName: 'diff' } ).then( function ( response ) { @@ -84,8 +87,8 @@ parseDocumentModulePromise = parseDocumentModulePromise || $.Deferred().resolve().promise(); oldPageName = oldPageName || mw.config.get( 'wgRelevantPageName' ); - oldRevPromise = typeof oldIdOrPromise === 'number' ? this.fetchRevision( oldIdOrPromise, oldPageName, parseDocumentModulePromise ) : oldIdOrPromise; - newRevPromise = typeof newIdOrPromise === 'number' ? this.fetchRevision( newIdOrPromise, newPageName, parseDocumentModulePromise ) : newIdOrPromise; + oldRevPromise = typeof oldIdOrPromise === 'number' ? this.fetchRevision( oldIdOrPromise, oldPageName, null, parseDocumentModulePromise ) : oldIdOrPromise; + newRevPromise = typeof newIdOrPromise === 'number' ? this.fetchRevision( newIdOrPromise, newPageName, null, parseDocumentModulePromise ) : newIdOrPromise; return $.when( oldRevPromise, newRevPromise, parseDocumentModulePromise ).then( function ( oldDoc, newDoc ) { // TODO: Differ expects newDoc to be derived from oldDoc and contain all its store data. diff --git a/modules/ve-mw/init/ve.init.mw.Target.js b/modules/ve-mw/init/ve.init.mw.Target.js index dbcf82ec29..36a1c0a6a4 100644 --- a/modules/ve-mw/init/ve.init.mw.Target.js +++ b/modules/ve-mw/init/ve.init.mw.Target.js @@ -178,7 +178,7 @@ ve.init.mw.Target.prototype.createModelFromDom = function () { /** * @inheritdoc - * @param {number|string|null} section Section + * @param {number|string|null} section Section. Use null to unwrap all sections. * @param {boolean} [onlySection] Only return the requested section, otherwise returns the * whole document with just the requested section still wrapped (visual mode only). */