Arlolra has uploaded a new change for review.
https://gerrit.wikimedia.org/r/231420
Change subject: Kill off rtTestMode
......................................................................
Kill off rtTestMode
* Just let the rt-testing numbers be what they are since mostly what
we care about going forward is regressions.
Change-Id: I8a28ebdfd84d8efb0ae98361c45eae5016bd846a
---
M lib/dom.cleanup.js
M lib/dom.computeDSR.js
M lib/mediawiki.DOMPostProcessor.js
M lib/mediawiki.ParsoidConfig.js
M lib/mediawiki.Util.js
M lib/mediawiki.WikitextSerializer.js
M lib/wts.SerializerState.js
M lib/wts.TagHandlers.js
M lib/wts.normalizeDOM.js
M lib/wts.separators.js
M lib/wts.utils.js
M tests/client/client.js
M tests/roundtrip-test.js
M tests/rttest.localsettings.js
14 files changed, 24 insertions(+), 83 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/services/parsoid
refs/changes/20/231420/1
diff --git a/lib/dom.cleanup.js b/lib/dom.cleanup.js
index f75023c..8f730f0 100644
--- a/lib/dom.cleanup.js
+++ b/lib/dom.cleanup.js
@@ -9,7 +9,7 @@
var topLevelRE =
/(?:^|\s)mw:(StartTag|EndTag|Extension\/ref\/Marker|TSRMarker|Transclusion)\/?[^\s]*/;
var nonTopLevelRE =
/(?:^|\s)mw:(StartTag|EndTag|TSRMarker|Transclusion)\/?[^\s]*/;
-function stripMarkerMetas(rtTestMode, node, env, atTopLevel) {
+function stripMarkerMetas(node, env, atTopLevel) {
var metaType = node.getAttribute("typeof");
if (!metaType) {
return true;
@@ -20,7 +20,7 @@
// in which case we also have to keep it, except if it's also a
mw:extension/ref/Marker
// in which case it'll have data-mw but we have to remove the node.
var metaTestRE = atTopLevel ? topLevelRE : nonTopLevelRE;
- if ((!rtTestMode && metaType === "mw:Placeholder/StrippedTag")
+ if (metaType === "mw:Placeholder/StrippedTag"
|| (metaTestRE.test(metaType) &&
(Object.keys(DU.getDataMw(node) || {}).length === 0 ||
metaType.match(/mw:Extension\/ref\/Marker/)))) {
diff --git a/lib/dom.computeDSR.js b/lib/dom.computeDSR.js
index 5229ffe..e3a6359 100644
--- a/lib/dom.computeDSR.js
+++ b/lib/dom.computeDSR.js
@@ -213,7 +213,6 @@
// the child dom are indeed identical. Alternatively, we could
// explicitly code this check before everything and bypass this.
var cs = ce;
- var rtTestMode = env.conf.parsoid.rtTestMode;
for (var i = numChildren - 1; i >= 0; i--) {
var isMarkerTag = false;
@@ -228,18 +227,16 @@
// be around to miss in the filling gap. So, absorb its width
into
// the DSR of its previous sibling. Currently, this fix is
only for
// B and I tags where the fix is clear-cut and obvious.
- if (!rtTestMode) {
- var next = child.nextSibling;
- if (next && DU.isElt(next)) {
- var ndp = DU.getDataParsoid(next);
- if (ndp.src &&
-
/(?:^|\s)mw:Placeholder\/StrippedTag(?=$|\s)/.test(next.getAttribute("typeof")))
{
- if (Consts.WTQuoteTags.has(ndp.name) &&
-
Consts.WTQuoteTags.has(child.nodeName)) {
- correction = ndp.src.length;
- ce += correction;
- dsrCorrection = correction;
- }
+ var next = child.nextSibling;
+ if (next && DU.isElt(next)) {
+ var ndp = DU.getDataParsoid(next);
+ if (ndp.src &&
+
/(?:^|\s)mw:Placeholder\/StrippedTag(?=$|\s)/.test(next.getAttribute("typeof")))
{
+ if (Consts.WTQuoteTags.has(ndp.name) &&
+ Consts.WTQuoteTags.has(child.nodeName))
{
+ correction = ndp.src.length;
+ ce += correction;
+ dsrCorrection = correction;
}
}
}
@@ -277,7 +274,7 @@
//
// Currently, this fix is only for
// B and I tags where the fix is clear-cut and obvious.
- if (!rtTestMode && ce !== null && dp.autoInsertedEnd &&
DU.isQuoteElt(child)) {
+ if (ce !== null && dp.autoInsertedEnd &&
DU.isQuoteElt(child)) {
correction = (3 + child.nodeName.length);
if (correction === dsrCorrection) {
ce -= correction;
diff --git a/lib/mediawiki.DOMPostProcessor.js
b/lib/mediawiki.DOMPostProcessor.js
index ebaf5d8..45812a7 100644
--- a/lib/mediawiki.DOMPostProcessor.js
+++ b/lib/mediawiki.DOMPostProcessor.js
@@ -146,8 +146,7 @@
var tableFixer = new TableFixups.TableFixups(env);
// 1. Strip marker metas -- removes left over marker metas (ex: metas
// nested in expanded tpl/extension output).
- domVisitor.addHandler('meta',
- CleanUp.stripMarkerMetas.bind(null,
env.conf.parsoid.rtTestMode));
+ domVisitor.addHandler('meta', CleanUp.stripMarkerMetas);
// 2. Deal with <li>-hack and move trailing categories in <li>s out of
the list
domVisitor.addHandler('li', liFixups.handleLIHack);
domVisitor.addHandler('li', liFixups.migrateTrailingCategories);
diff --git a/lib/mediawiki.ParsoidConfig.js b/lib/mediawiki.ParsoidConfig.js
index e8a150c..91260c4 100644
--- a/lib/mediawiki.ParsoidConfig.js
+++ b/lib/mediawiki.ParsoidConfig.js
@@ -199,11 +199,6 @@
ParsoidConfig.prototype.fetchConfig = true;
/**
- * @property {boolean} rtTestMode
- */
-ParsoidConfig.prototype.rtTestMode = false;
-
-/**
* @property {number} Parsoid DOM format version
* See https://bugzilla.wikimedia.org/show_bug.cgi?id=52937
*/
diff --git a/lib/mediawiki.Util.js b/lib/mediawiki.Util.js
index 6b662e9..7db5088 100644
--- a/lib/mediawiki.Util.js
+++ b/lib/mediawiki.Util.js
@@ -190,7 +190,6 @@
[
'fetchConfig',
'fetchTemplates',
- 'rtTestMode',
'addHTMLTemplateParameters',
].forEach(function(c) {
if (opts[c] !== undefined) {
@@ -320,11 +319,6 @@
'apiURL': {
description: 'http path to remote API, e.g.
http://en.wikipedia.org/w/api.php',
'default': null,
- },
- 'rtTestMode': {
- description: 'Test in rt test mode (changes
some parse & serialization strategies)',
- 'boolean': true,
- 'default': false,
},
'dp': {
description: 'Output data-parsoid JSON',
diff --git a/lib/mediawiki.WikitextSerializer.js
b/lib/mediawiki.WikitextSerializer.js
index b052fe7..54866d0 100644
--- a/lib/mediawiki.WikitextSerializer.js
+++ b/lib/mediawiki.WikitextSerializer.js
@@ -52,11 +52,6 @@
this.options = options;
this.env = options.env;
- // Set rtTestMode if not already set.
- if (this.options.rtTestMode === undefined) {
- this.options.rtTestMode = this.env.conf.parsoid.rtTestMode;
- }
-
// WT escaping handlers
this.wteHandlers = new WTEModule.WikitextEscapeHandlers(this.env, this);
@@ -1333,7 +1328,6 @@
// Init state
state.selserMode = selserMode || false;
- state.rtTestMode = state.rtTestMode && !state.selserMode; // always
false in selser mode
// Normalize the DOM
(new Normalizer(state)).normalizeDOM(body);
diff --git a/lib/wts.SerializerState.js b/lib/wts.SerializerState.js
index ec592b3..b6c3f4b 100644
--- a/lib/wts.SerializerState.js
+++ b/lib/wts.SerializerState.js
@@ -11,12 +11,6 @@
/* *********************************************************************
* Here is what the state attributes mean:
*
- * rtTestMode
- * Are we currently running round-trip tests? If yes, then we know
- * there won't be any edits and we more aggressively try to use original
- * source and source flags during serialization since this is a test of
- * Parsoid's efficacy in preserving information.
- *
* sep
* Separator information:
* - constraints: min/max number of newlines
@@ -78,7 +72,6 @@
* ********************************************************************* */
var initialState = {
- rtTestMode: true,
sep: {},
onSOL: true,
escapeText: false,
diff --git a/lib/wts.TagHandlers.js b/lib/wts.TagHandlers.js
index d809937..5cd457a 100644
--- a/lib/wts.TagHandlers.js
+++ b/lib/wts.TagHandlers.js
@@ -400,8 +400,7 @@
// they parse back identically.
//
// Capture the end tag src to see if it is
actually going
- // to be emitted (not always true if marked as
autoInsertedEnd
- // and running in rtTestMode)
+ // to be emitted.
var endTagSrc = '';
var captureEndTagSrcCB = function(src, _) {
endTagSrc = src;
@@ -1053,7 +1052,7 @@
return !/^data-parsoid/.test(kv.k) &&
!(kv.k === 'id' &&
/^mw[\w-]{2,}$/.test(kv.v));
});
- if (!state.rtTestMode && dp.misnested && dp.stx
!== 'html' && !kvs.length) {
+ if (dp.misnested && dp.stx !== 'html' &&
!kvs.length) {
// Discard span wrappers added to flag
misnested content.
// Warn since selser should have reused
source.
state.env.log('warning',
diff --git a/lib/wts.normalizeDOM.js b/lib/wts.normalizeDOM.js
index dba11bb..92be673 100644
--- a/lib/wts.normalizeDOM.js
+++ b/lib/wts.normalizeDOM.js
@@ -18,7 +18,6 @@
function Normalizer(state) {
this.env = state.env;
this.inSelserMode = state.selserMode;
- this.inRtTestMode = state.rtTestMode;
this.inInsertedContent = false;
}
@@ -184,18 +183,10 @@
Normalizer.prototype.stripIfEmpty = function(node) {
var next = DU.nextNonDeletedSibling(node);
var dp = DU.getDataParsoid(node);
- var strict = this.inRtTestMode;
- var autoInserted = dp.autoInsertedStart || dp.autoInsertedEnd;
-
- // In rtTestMode, let's reduce noise by requiring the node to be fully
- // empty (ie. exclude whitespace text) and not having auto-inserted
tags.
- var strippable = !(this.inRtTestMode && autoInserted) &&
- DU.nodeEssentiallyEmpty(node, strict) &&
+ var strippable = DU.nodeEssentiallyEmpty(node, false) &&
// Ex: "<a..>..</a><b></b>bar"
// From [[Foo]]<b/>bar usage found on some dewiki pages.
- // FIXME: Should this always than just in rt-test mode
- !(this.inRtTestMode && dp.stx === 'html');
-
+ dp.stx !== 'html';
if (strippable) {
// Update diff markers (before the deletion)
this.addDiffMarks(node, 'deleted', true);
@@ -211,8 +202,7 @@
var next = DU.nextNonDeletedSibling(node);
var last = DU.lastNonDeletedChildNode(node);
var endsInSpace = DU.isText(last) && last.nodeValue.match(/\s+$/);
- // Conditional on rtTestMode to reduce the noise in testing.
- if (!this.inRtTestMode && endsInSpace) {
+ if (endsInSpace) {
last.nodeValue = last.nodeValue.substring(0, endsInSpace.index);
// Try to be a little smarter and drop the spaces if possible.
if (!/^\s+/.test(next.nodeValue)) {
diff --git a/lib/wts.separators.js b/lib/wts.separators.js
index 3f706f6..c124fb8 100644
--- a/lib/wts.separators.js
+++ b/lib/wts.separators.js
@@ -557,12 +557,11 @@
* In other scenarios, DSR values on "adjacent" nodes in the edited DOM
* may not reflect deleted content between them.
*
---------------------------------------------------------------------- */
- var origSepUsable = node && prevNode && node !== prevNode &&
- src && (state.rtTestMode ||
+ var origSepUsable = node && prevNode && node !== prevNode && src &&
(state.selserMode && !state.inModifiedContent &&
!DU.nextToDeletedBlockNodeInWT(state.env, prevNode,
true) &&
!DU.nextToDeletedBlockNodeInWT(state.env, node, false)
&&
- DU.origSrcValidInEditedContext(state.env, node)));
+ DU.origSrcValidInEditedContext(state.env, node));
if (origSepUsable) {
if (!DU.isElt(prevNode)) {
// Check if this is the last child of a zero-width
element, and use
@@ -581,9 +580,7 @@
DU.getDataParsoid(prevNode.previousSibling).dsr &&
// Don't extrapolate if the string was
potentially changed
// or we didn't diff (selser disabled)
- (state.rtTestMode || // no changes in
rt testing
- // diffed and no change here
- (state.selserMode &&
!DU.directChildrenChanged(node.parentNode, this.env)))
+ (state.selserMode &&
!DU.directChildrenChanged(node.parentNode, this.env))
) {
var endDsr =
DU.getDataParsoid(prevNode.previousSibling).dsr[1];
var correction;
diff --git a/lib/wts.utils.js b/lib/wts.utils.js
index 41eee3a..834d3f4 100644
--- a/lib/wts.utils.js
+++ b/lib/wts.utils.js
@@ -22,12 +22,7 @@
* not marked with autoInsertedStart
*/
emitStartTag: function(src, node, state, cb) {
- if (!state.rtTestMode) {
- cb(src, node);
- } else if (!DU.getDataParsoid(node).autoInsertedStart) {
- cb(src, node);
- }
- // else: drop content
+ cb(src, node);
},
/**
@@ -35,12 +30,7 @@
* not marked with autoInsertedStart
*/
emitEndTag: function(src, node, state, cb) {
- if (!state.rtTestMode) {
- cb(src, node);
- } else if (!DU.getDataParsoid(node).autoInsertedEnd) {
- cb(src, node);
- }
- // else: drop content
+ cb(src, node);
},
};
diff --git a/tests/client/client.js b/tests/client/client.js
index fe5a4c3..70d49b7 100755
--- a/tests/client/client.js
+++ b/tests/client/client.js
@@ -75,7 +75,6 @@
rtTest.runTests(test.title, {
setup: config.setup,
prefix: test.prefix,
- rtTestMode: true,
parsoidURL: parsoidURL,
}, rtTest.xmlFormat).nodify(function(err, results) {
var callback = null;
diff --git a/tests/roundtrip-test.js b/tests/roundtrip-test.js
index b932ac5..eadba98 100755
--- a/tests/roundtrip-test.js
+++ b/tests/roundtrip-test.js
@@ -677,9 +677,6 @@
parsoidURL: {
description: 'The URL for the Parsoid API',
},
- }, {
- // defaults for standard options
- rtTestMode: true, // suppress noise by default
});
var opts = yargs.usage(
diff --git a/tests/rttest.localsettings.js b/tests/rttest.localsettings.js
index 7c5f587..6416bd5 100644
--- a/tests/rttest.localsettings.js
+++ b/tests/rttest.localsettings.js
@@ -55,9 +55,6 @@
// changing api.php for load.php.
// parsoidConfig.modulesLoadURI = true;
- // Set rtTestMode to true for round-trip testing
- parsoidConfig.rtTestMode = true;
-
// Set to true to enable Performance timing
parsoidConfig.useDefaultPerformanceTimer = false;
// Peformance timing options for testing
--
To view, visit https://gerrit.wikimedia.org/r/231420
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8a28ebdfd84d8efb0ae98361c45eae5016bd846a
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/services/parsoid
Gerrit-Branch: master
Gerrit-Owner: Arlolra <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
