Subramanya Sastry has uploaded a new change for review.
https://gerrit.wikimedia.org/r/77357
Change subject: (Bug 51678) Fixed bug in dom-diff algorithm
......................................................................
(Bug 51678) Fixed bug in dom-diff algorithm
* The dom-diff algorithm was occasionally recursing into
template transclusion HTML. Since transclusion HTML can never
be directly modified by editors and template updates are
transmitted via the data-mw object in the wrapper, it is not
necessary to compare transclusion content. It can in fact
introduce bugs as in the itwp examples in the bug report because
DOM-diff might insert deleted-content-markers and break the
continuity in template about markers (which is important to
skip over transclusion content during serialization correctly).
This bug also only surfaces because of the peculiarities of how
updated template HTML is modified in VE (see bug report for
more details).
* Tested this with HTML (old and edited) dumped from VE via
Chrome's console. Verified bug without patch and a fix with
the patch.
Currently, we don't have direct dom-diff tests, only indirect
tests via selser.
Even if we did have, it would be hard to write a test for this
bug fix becuase it depends on template content HTML getting
deleted on edit (see bug report for details).
But, it would be possible with a jasmine testing framework which
we could consider resurrecting.
To cut a long story short: no change in parsertest results,
and no new tests added.
Change-Id: I886161d981a0c79b8300d88c5281514b4248cff2
---
M js/lib/mediawiki.DOMDiff.js
M js/lib/mediawiki.DOMUtils.js
M js/lib/mediawiki.WikitextSerializer.js
3 files changed, 34 insertions(+), 16 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Parsoid
refs/changes/57/77357/1
diff --git a/js/lib/mediawiki.DOMDiff.js b/js/lib/mediawiki.DOMDiff.js
index 5d4814e..39a90c4 100644
--- a/js/lib/mediawiki.DOMDiff.js
+++ b/js/lib/mediawiki.DOMDiff.js
@@ -231,31 +231,48 @@
if (origNode.nodeName === baseNode.nodeName) {
// Identical wrapper-type, but modified.
// Mark as modified, and recurse.
+ this.debug("--found diff:
modified-wrapper--");
this.markNode(origNode,
'modified-wrapper');
- this.doDOMDiff(baseNode, origNode);
+ if (!DU.isTplElementNode(this.env,
baseNode) && !DU.isTplElementNode(this.env, origNode)) {
+ // Dont recurse into
template-like-content
+ this.doDOMDiff(baseNode,
origNode);
+ }
} else {
// Mark the sub-tree as modified since
// we have two entirely different nodes
here
+ this.debug("--found diff: modified--");
this.markNode(origNode, 'modified');
}
}
// Record the fact that direct children changed in the
parent node
+ this.debug("--found diff: modified-children--");
this.markNode(newParentNode, 'modified-children');
foundDiffOverall = true;
- } else if (!DU.isTplElementNode(this.env, newNode)) {
+ } else if (!DU.isTplElementNode(this.env, baseNode) &&
!DU.isTplElementNode(this.env, newNode)) {
+ this.debug("--equal: recursing--");
// Recursively diff subtrees if not template-like
content
var subtreeDiffers = this.doDOMDiff(baseNode, newNode);
if (subtreeDiffers) {
+ this.debug("--found diff: subtree-changed--");
this.markNode(newNode, 'subtree-changed');
}
foundDiffOverall = subtreeDiffers || foundDiffOverall;
}
- // And move on to the next pair
- baseNode = baseNode.nextSibling;
- newNode = newNode.nextSibling;
+ // And move on to the next pair (skipping over template HTML)
+ if (DU.isTplElementNode(this.env, baseNode)) {
+ baseNode = DU.skipOverEncapsulatedContent(baseNode);
+ } else {
+ baseNode = baseNode.nextSibling;
+ }
+
+ if (DU.isTplElementNode(this.env, newNode)) {
+ newNode = DU.skipOverEncapsulatedContent(newNode);
+ } else {
+ newNode = newNode.nextSibling;
+ }
}
// mark extra new nodes as modified
diff --git a/js/lib/mediawiki.DOMUtils.js b/js/lib/mediawiki.DOMUtils.js
index 0dd729e..e701a04 100644
--- a/js/lib/mediawiki.DOMUtils.js
+++ b/js/lib/mediawiki.DOMUtils.js
@@ -4,6 +4,7 @@
* General DOM utilities
*/
+require('./core-upgrade.js');
var Util = require('./mediawiki.Util.js').Util,
Node = require('./mediawiki.wikitext.constants.js').Node,
pd = require('./mediawiki.parser.defines.js');
@@ -681,6 +682,15 @@
return nodes;
},
+ skipOverEncapsulatedContent: function(node) {
+ var about = node.getAttribute('about');
+ if (!about) {
+ return node;
+ }
+
+ return this.getAboutSiblings(node, about).last().nextSibling;
+ },
+
/**
* Extract transclusion and extension expansions from a DOM, and return
* them in a structure like this:
diff --git a/js/lib/mediawiki.WikitextSerializer.js
b/js/lib/mediawiki.WikitextSerializer.js
index bcf736b..651752e 100644
--- a/js/lib/mediawiki.WikitextSerializer.js
+++ b/js/lib/mediawiki.WikitextSerializer.js
@@ -3109,15 +3109,6 @@
return srcParts.join('');
};
-WSP.skipOverEncapsulatedContent = function(node) {
- var about = node.getAttribute('about');
- if (!about) {
- return node;
- }
-
- return DU.getAboutSiblings(node, about).last().nextSibling;
-};
-
/**
* Get a DOM-based handler for an element node
*/
@@ -3168,7 +3159,7 @@
if (src !== undefined) {
self.emitWikitext(src, state, cb, node);
- return
self.skipOverEncapsulatedContent(node);
+ return
DU.skipOverEncapsulatedContent(node);
} else {
console.error("ERROR: No handler for: "
+ node.outerHTML);
console.error("Serializing as HTML.");
@@ -3944,7 +3935,7 @@
// Skip over encapsulated content since
it has already been serialized
var typeOf = node.getAttribute(
'typeof' ) || '';
if
(/\bmw:(?:Transclusion\b|Param\b|Extension\/[^\s]+)/.test(typeOf)) {
- nextNode =
this.skipOverEncapsulatedContent(node);
+ nextNode =
DU.skipOverEncapsulatedContent(node);
}
} else if (DU.onlySubtreeChanged(node,
this.env) &&
hasValidTagWidths(dp.dsr) &&
--
To view, visit https://gerrit.wikimedia.org/r/77357
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I886161d981a0c79b8300d88c5281514b4248cff2
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Parsoid
Gerrit-Branch: master
Gerrit-Owner: Subramanya Sastry <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
