Subramanya Sastry has uploaded a new change for review.
https://gerrit.wikimedia.org/r/59361
Change subject: First set of fixes to DOMDiff.
......................................................................
First set of fixes to DOMDiff.
* DOMDiff was not setting modification/insertion markers
correctly.
* Removed some dead code from DOMUtils.
* This does not affect any selser parser-test results since
those parser tests are not complex.
Change-Id: I900483ea426147ea9a2a13a4bfea840e95d181c5
---
M js/lib/mediawiki.DOMDiff.js
M js/lib/mediawiki.DOMUtils.js
2 files changed, 29 insertions(+), 52 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Parsoid
refs/changes/61/59361/1
diff --git a/js/lib/mediawiki.DOMDiff.js b/js/lib/mediawiki.DOMDiff.js
index 28449ce..27be3eb 100644
--- a/js/lib/mediawiki.DOMDiff.js
+++ b/js/lib/mediawiki.DOMDiff.js
@@ -12,10 +12,7 @@
this.env = env;
this.debug = env.conf.parsoid.debug ||
(env.conf.parsoid.traceFlags &&
env.conf.parsoid.traceFlags.indexOf('selser') !== -1) ?
- console.error : function(){};
- this.currentId = 0;
- this.startPos = 0; // start offset of the current unmodified chunk
- this.curPos = 0; // end offset of the last processed node
+ console.log : function(){};
}
var DDP = DOMDiff.prototype;
@@ -28,12 +25,13 @@
// work on a cloned copy of the passed-in node
var workNode = node.cloneNode(true);
- // First do a quick check on the nodes themselves
+ // SSS FIXME: Is this required?
+ //
+ // First do a quick check on the top-level nodes themselves
if (!this.treeEquals(this.env.page.dom, workNode, false)) {
this.markNode(workNode, 'modified');
return { isEmpty: false, dom: workNode };
}
-
// The root nodes are equal, call recursive differ
var foundChange = this.doDOMDiff(this.env.page.dom, workNode);
@@ -173,8 +171,10 @@
lookaheadNode = null,
foundDiffOverall = false;
while ( baseNode && newNode ) {
- // console.warn("--> A: " + (DU.isElt(baseNode) ?
baseNode.outerHTML : JSON.stringify(baseNode.nodeValue)));
- // console.warn("--> B: " + (DU.isElt(newNode) ?
newNode.outerHTML : JSON.stringify(newNode.nodeValue)));
+ if (this.debug) {
+ console.warn("--> A: " + (DU.isElt(baseNode) ?
baseNode.outerHTML : JSON.stringify(baseNode.nodeValue)));
+ console.warn("--> B: " + (DU.isElt(newNode) ?
newNode.outerHTML : JSON.stringify(newNode.nodeValue)));
+ }
// Quick shallow equality check first
if ( ! this.treeEquals(baseNode, newNode, false) ) {
@@ -186,7 +186,7 @@
// look-ahead in *new* DOM to detect insertions
if (!canBeIgnored(baseNode)) {
- // console.warn("--lookahead in new dom--");
+ this.debug("--lookahead in new dom--");
lookaheadNode = newNode.nextSibling;
while (lookaheadNode) {
if (!canBeIgnored(lookaheadNode) &&
@@ -208,7 +208,7 @@
// look-ahead in *base* DOM to detect deletions
if (!foundDiff && !canBeIgnored(newNode)) {
- // console.warn("--lookahead in old dom--");
+ this.debug("--lookahead in old dom--");
lookaheadNode = baseNode.nextSibling;
while (lookaheadNode) {
if (!canBeIgnored(lookaheadNode) &&
@@ -226,14 +226,19 @@
}
}
- // If we never found a diff through lookaheads,
reconsider
- // the original node, mark it modified, and recurse
into subtrees.
if (!foundDiff) {
- this.markNode(origNode, 'modified');
- this.doDOMDiff(baseNode, origNode);
+ if (origNode.nodeName === baseNode.nodeName) {
+ // Identical wrapper-type, but modified.
+ // Mark as modified, and recurse.
+ this.markNode(origNode,
'modified-wrapper');
+ this.doDOMDiff(baseNode, origNode);
+ } else {
+ // Mark entire sub-tree as modified
since
+ // we have two entirely different nodes
here
+ this.markNode(origNode, 'modified');
+ }
}
- // nothing found, mark new node as modified / differing
foundDiffOverall = true;
} else if(!DU.isTplElementNode(this.env, newNode)) {
// Recursively diff subtrees if not template-like
content
@@ -267,8 +272,6 @@
};
-
-
/******************************************************
* Helpers
*****************************************************/
@@ -279,9 +282,17 @@
// insert a meta tag marking the place where content used to be
DU.prependTypedMeta(node, 'mw:DiffMarker');
} else {
- // modification/insertion marker
if (node.nodeType === node.ELEMENT_NODE) {
DU.setDiffMark(node, this.env, change);
+ if (change === 'inserted' || change === 'modified') {
+ // Recursively mark the entire subtree with
+ // the 'inserted/modified' marker.
+ var n = node.firstChild;
+ while (n) {
+ this.markNode(n, change);
+ n = n.nextSibling;
+ }
+ }
} else if (node.nodeType !== node.TEXT_NODE && node.nodeType
!== node.COMMENT_NODE) {
console.error('ERROR: Unhandled node type ' +
node.nodeType + ' in markNode!');
console.trace();
diff --git a/js/lib/mediawiki.DOMUtils.js b/js/lib/mediawiki.DOMUtils.js
index beabf9c..8c9206f 100644
--- a/js/lib/mediawiki.DOMUtils.js
+++ b/js/lib/mediawiki.DOMUtils.js
@@ -454,40 +454,6 @@
node.nodeValue.match(/^\s*$/);
},
- isNonContentNode: function(node) {
- return node.nodeType === node.COMMENT_NODE ||
- this.isIEW(node) ||
- this.isMarkerMeta(node, "mw:DiffMarker");
- },
-
- /**
- * Get the first child element or non-IEW text node, ignoring
- * whitespace-only text nodes and comments.
- */
- getFirstNonSepChildNode: function(node) {
- var child = node.firstChild;
- while (child && this.isNonContentNode(child)) {
- child = child.nextSibling;
- }
- return child;
- },
-
- previousNonSepSibling: function (node) {
- var prev = node.previousSibling;
- while (prev && this.isNonContentNode(prev)) {
- prev = prev.previousSibling;
- }
- return prev;
- },
-
- nextNonSepSibling: function (node) {
- var next = node.nextSibling;
- while (next && this.isNonContentNode(next)) {
- next = next.nextSibling;
- }
- return next;
- },
-
/**
* Are all children of this node text nodes?
*/
--
To view, visit https://gerrit.wikimedia.org/r/59361
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I900483ea426147ea9a2a13a4bfea840e95d181c5
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
