jenkins-bot has submitted this change and it was merged. Change subject: Don't use selser if oldid is missing ......................................................................
Don't use selser if oldid is missing * So far, we were always constructing SelectiveSerializer and checking for absence of page-src to fallback to non-selser WTS. However, that is insufficient since rt-testing provides page-src to minimize dirty diffs and we don't want to be using selser there. As per https://www.mediawiki.org/wiki/Parsoid/API#v1_API_entry_points "Both it and the oldid parameter are needed for clean round-tripping of HTML retrieved earlier with" * So much for wanting to hide internal details (of selser) from the API. This does leak selser-ability via the API, but right now, there doesn't seem to be a clean way to support all the varied use cases for html2wt that provide title, oldid, and page src. The only way to disable selser right now is to NOT provide oldid. In this patch, we'll rely on that. * Clarified the serialization properties in DOMUtils.serializeDOM to more accurately reflect when DOM/orig-src needs to be fetched -- only in selser mode. * Fixed a mocha api test to reflect this new expectation. Change-Id: I30cc1ea61b915096a512466578cec565fd5f56c0 --- M api/routes.js M lib/mediawiki.DOMUtils.js M lib/mediawiki.SelectiveSerializer.js M tests/mocha/api.js 4 files changed, 18 insertions(+), 6 deletions(-) Approvals: Arlolra: Looks good to me, approved jenkins-bot: Verified diff --git a/api/routes.js b/api/routes.js index 8d921c9..8d40719 100644 --- a/api/routes.js +++ b/api/routes.js @@ -267,7 +267,13 @@ } } - var p = DU.serializeDOM(env, doc.body, parsoidConfig.useSelser) + // As per https://www.mediawiki.org/wiki/Parsoid/API#v1_API_entry_points + // "Both it and the oldid parameter are needed for + // clean round-tripping of HTML retrieved earlier with" + // So, no oldid => no selser + var hasOldId = (env.page.id && env.page.id !== '0'); + var useSelser = hasOldId && parsoidConfig.useSelser; + var p = DU.serializeDOM(env, doc.body, useSelser) .timeout(REQ_TIMEOUT) .then(function(output) { var contentType = 'text/plain;profile=mediawiki.org/specs/wikitext/1.0.0;charset=utf-8'; diff --git a/lib/mediawiki.DOMUtils.js b/lib/mediawiki.DOMUtils.js index e81c8fc..539d666 100644 --- a/lib/mediawiki.DOMUtils.js +++ b/lib/mediawiki.DOMUtils.js @@ -2551,12 +2551,12 @@ console.assert(DU.isBody(body), 'Expected a body node.'); var hasOldId = (env.page.id && env.page.id !== '0'); - var needsWt = (env.page.src === null); - var needsDOM = !(env.page.dom || env.page.domdiff); + var needsWt = useSelser && hasOldId && (env.page.src === null); + var needsOldDOM = useSelser && !(env.page.dom || env.page.domdiff); var useCache = env.conf.parsoid.parsoidCacheURI && hasOldId; var steps = []; - if (needsWt && hasOldId) { + if (needsWt) { steps.push(function() { var target = env.resolveTitle(env.normalizeTitle(env.page.name), ''); return TemplateRequest.setPageSrcInfo( @@ -2566,7 +2566,7 @@ }); }); } - if (needsDOM) { + if (needsOldDOM) { if (useCache) { steps.push(function() { return ParsoidCacheRequest.promise( diff --git a/lib/mediawiki.SelectiveSerializer.js b/lib/mediawiki.SelectiveSerializer.js index a949f7f..1030b6e 100644 --- a/lib/mediawiki.SelectiveSerializer.js +++ b/lib/mediawiki.SelectiveSerializer.js @@ -46,6 +46,7 @@ var out; var startTimers = new Map(); + if ((!this.env.page.dom && !this.env.page.domdiff) || this.env.page.src === null) { if (this.timer) { startTimers.set('html2wt.full.serialize', Date.now()); diff --git a/tests/mocha/api.js b/tests/mocha/api.js index 60de2eb..4c3eafa 100644 --- a/tests/mocha/api.js +++ b/tests/mocha/api.js @@ -443,7 +443,12 @@ // New and old html are identical, which should produce no diffs // and reuse the original wikitext. request(api) - .post('v2/' + mockHost + '/wt/') + // Need to provide an oldid so that selser mode is enabled + // Without an oldid, serialization falls back to non-selser wts. + // The oldid is used to fetch wikitext, but if wikitext is provided + // (as in this test), it is not used. So, for testing purposes, + // we can use any old random id, as long as something is present. + .post('v2/' + mockHost + '/wt/Junk_Page/1234') .send({ html: "<html><body id=\"mwAA\"><div id=\"mwBB\">Selser test</div></body></html>", original: { -- To view, visit https://gerrit.wikimedia.org/r/211146 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I30cc1ea61b915096a512466578cec565fd5f56c0 Gerrit-PatchSet: 6 Gerrit-Project: mediawiki/services/parsoid Gerrit-Branch: master Gerrit-Owner: Subramanya Sastry <[email protected]> Gerrit-Reviewer: Arlolra <[email protected]> Gerrit-Reviewer: Cscott <[email protected]> Gerrit-Reviewer: Subramanya Sastry <[email protected]> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
