jenkins-bot has submitted this change and it was merged.
Change subject: Fix crashers when dp isn't present for an id
......................................................................
Fix crashers when dp isn't present for an id
* In a48ca3e98dc66ae4e7456d0fad78392c8a223553, we assumed that dp would
always be available for a child of body with an id. However, template
siblings can still have manually added ids (present in the wikitext),
even though only the first about sibling has a dp. The algorithm only
accounted for the absence of ids and was thrown off by their
presence.
* Now we explicitly test that the id doesn't come from an about sibling
so that all templated content is in one section.
Change-Id: I3aacedb7ba4e288d560c3bb52aff02ffdb9e184d
---
M lib/XMLSerializer.js
M lib/mediawiki.DOMUtils.js
M tests/mocha/xmlserializer.js
3 files changed, 23 insertions(+), 19 deletions(-)
Approvals:
Subramanya Sastry: Looks good to me, approved
jenkins-bot: Verified
diff --git a/lib/XMLSerializer.js b/lib/XMLSerializer.js
index da33174..1f631a9 100644
--- a/lib/XMLSerializer.js
+++ b/lib/XMLSerializer.js
@@ -163,16 +163,14 @@
}
}
-// These two are from DU but there's a cyclic dependency.
-function isElt(node) {
- return node.nodeType === ELEMENT_NODE;
-}
-function isBody(node) {
- return isElt(node) && node.nodeName === "BODY";
-}
-
+var DU;
var accumOffsets = function(out, bit, node, flag) {
- if (isBody(node)) {
+ // Circular ref
+ if (!DU) {
+ DU = require('./mediawiki.DOMUtils.js').DOMUtils;
+ }
+
+ if (DU.isBody(node)) {
out.str += bit;
if (flag === 'start') {
out.start = out.str.length;
@@ -180,7 +178,7 @@
out.start = null;
out.uid = null;
}
- } else if (!isElt(node) || out.start === null ||
!isBody(node.parentNode)) {
+ } else if (!DU.isElt(node) || out.start === null ||
!DU.isBody(node.parentNode)) {
// In case you're wondering, out.start may never be set if body
// isn't a child of the node passed to serializeToString, or if
it
// is the node itself but options.innerXML is true.
@@ -189,9 +187,13 @@
out.offsets[out.uid].html[1] += bit.length;
}
} else {
- // Template siblings don't have ids,
+ var newUid = node.getAttribute('id');
+ // Template siblings don't have generated ids (but may have an
id),
// so associate them with preceding content.
- out.uid = node.getAttribute('id') || out.uid;
+ if (newUid && (!DU.isTplOrExtToplevelNode(node) ||
+ DU.isFirstEncapsulationWrapperNode(node))) {
+ out.uid = newUid;
+ }
console.assert(out.uid !== null);
if (!out.offsets.hasOwnProperty(out.uid)) {
var dt = out.str.length - out.start;
diff --git a/lib/mediawiki.DOMUtils.js b/lib/mediawiki.DOMUtils.js
index 360a6e3..117d6ec 100644
--- a/lib/mediawiki.DOMUtils.js
+++ b/lib/mediawiki.DOMUtils.js
@@ -81,7 +81,7 @@
},
isBody: function(node) {
- return node.nodeName === "BODY";
+ return DU.isElt(node) && node.nodeName === "BODY";
},
/**
@@ -2619,9 +2619,10 @@
out.type = dpScriptElt.getAttribute('type');
// Add the wt offsets.
Object.keys(out.offsets).forEach(function(key) {
- var dsr = out.dp.ids[key].dsr;
- if (Util.isValidDSR(dsr)) {
- out.offsets[key].wt = dsr.slice(0, 2);
+ var dp = out.dp.ids[key];
+ console.assert(dp);
+ if (Util.isValidDSR(dp.dsr)) {
+ out.offsets[key].wt = dp.dsr.slice(0, 2);
}
});
out.dp.sectionOffsets = out.offsets;
diff --git a/tests/mocha/xmlserializer.js b/tests/mocha/xmlserializer.js
index 22186f6..e623beb 100644
--- a/tests/mocha/xmlserializer.js
+++ b/tests/mocha/xmlserializer.js
@@ -28,7 +28,7 @@
it('should handle templates properly while capturing offsets',
function() {
var html = '<html><head><title>hi</title><body>' +
'<p about="#mwt1" typeof="mw:Transclusion"
id="mwAQ">a</p>' +
- '<p about="#mwt1">b</p>' +
+ '<p about="#mwt1" id="justhappenstobehere">b</p>' +
'<p id="mwAg">c</p>' +
'</body></html>';
var doc = domino.createDocument(html);
@@ -41,7 +41,8 @@
ret.should.have.property('offsets');
ret.offsets.should.have.property('mwAQ');
ret.offsets.should.have.property('mwAg');
- ret.offsets.mwAQ.html.should.eql([0, 79]);
- ret.offsets.mwAg.html.should.eql([79, 97]);
+ ret.offsets.should.not.have.property('justhappenstobehere');
+ ret.offsets.mwAQ.html.should.eql([0, 104]);
+ ret.offsets.mwAg.html.should.eql([104, 122]);
});
});
--
To view, visit https://gerrit.wikimedia.org/r/211921
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I3aacedb7ba4e288d560c3bb52aff02ffdb9e184d
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/services/parsoid
Gerrit-Branch: master
Gerrit-Owner: Arlolra <abrea...@wikimedia.org>
Gerrit-Reviewer: Cscott <canan...@wikimedia.org>
Gerrit-Reviewer: GWicke <gwi...@wikimedia.org>
Gerrit-Reviewer: Subramanya Sastry <ssas...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
