Subramanya Sastry has uploaded a new change for review. https://gerrit.wikimedia.org/r/59859
Change subject: Code cleanup + additional debugging stmts in DOMDiff.js ...................................................................... Code cleanup + additional debugging stmts in DOMDiff.js * Ported over cleanup and debugging code from the abandoned patch https://gerrit.wikimedia.org/r/#/c/59448/ * No change in any test results. Change-Id: I1a6efc6beb036ea2f8fd2e04b79677433404efc6 --- M js/lib/mediawiki.DOMDiff.js M js/lib/mediawiki.DOMUtils.js M js/lib/mediawiki.WikitextSerializer.js 3 files changed, 38 insertions(+), 64 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Parsoid refs/changes/59/59859/1 diff --git a/js/lib/mediawiki.DOMDiff.js b/js/lib/mediawiki.DOMDiff.js index cdec7c0..50e37c5 100644 --- a/js/lib/mediawiki.DOMDiff.js +++ b/js/lib/mediawiki.DOMDiff.js @@ -161,8 +161,13 @@ * Assume typical CSS white-space, so ignore ws diffs in non-pre content. */ DDP.doDOMDiff = function ( baseParentNode, newParentNode ) { - function canBeIgnored(node) { - return !DU.isElt(node) || DU.isIEW(node); + var dd = this; + + function debugOut(nodeA, nodeB) { + if (dd.debugging) { + dd.debug("--> A: " + (DU.isElt(nodeA) ? nodeA.outerHTML : JSON.stringify(nodeA.nodeValue))); + dd.debug("--> B: " + (DU.isElt(nodeB) ? nodeB.outerHTML : JSON.stringify(nodeB.nodeValue))); + } } // Perform a relaxed version of the recursive treeEquals algorithm that @@ -172,14 +177,11 @@ newNode = newParentNode.firstChild, lookaheadNode = null, foundDiffOverall = false; - while ( baseNode && newNode ) { - if (this.debugging) { - 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 + while ( baseNode && newNode ) { + debugOut(baseNode, newNode); if ( ! this.treeEquals(baseNode, newNode, false) ) { + this.debug("-- not equal --"); var origNode = newNode, foundDiff = false; @@ -187,16 +189,18 @@ // in the DOM. // look-ahead in *new* DOM to detect insertions - if (!canBeIgnored(baseNode)) { + if (DU.isContentNode(baseNode)) { this.debug("--lookahead in new dom--"); lookaheadNode = newNode.nextSibling; while (lookaheadNode) { - if (!canBeIgnored(lookaheadNode) && + debugOut(baseNode, lookaheadNode); + if (DU.isContentNode(lookaheadNode) && this.treeEquals(baseNode, lookaheadNode, true)) { // mark skipped-over nodes as inserted var markNode = newNode; - while(markNode !== lookaheadNode) { + while (markNode !== lookaheadNode) { + this.debug("--found diff: inserted--"); this.markNode(markNode, 'inserted'); markNode = markNode.nextSibling; } @@ -209,13 +213,15 @@ } // look-ahead in *base* DOM to detect deletions - if (!foundDiff && !canBeIgnored(newNode)) { + if (!foundDiff && DU.isContentNode(newNode)) { this.debug("--lookahead in old dom--"); lookaheadNode = baseNode.nextSibling; while (lookaheadNode) { - if (!canBeIgnored(lookaheadNode) && + debugOut(lookaheadNode, newNode); + if (DU.isContentNode(lookaheadNode) && this.treeEquals(lookaheadNode, newNode, true)) { + this.debug("--found diff: deleted--"); // TODO: treat skipped-over nodes as deleted // insertModificationMarker //console.log('inserting deletion mark before ' + newNode.outerHTML); @@ -242,7 +248,7 @@ } foundDiffOverall = true; - } else if(!DU.isTplElementNode(this.env, newNode)) { + } else if (!DU.isTplElementNode(this.env, newNode)) { // Recursively diff subtrees if not template-like content var subtreeDiffers = this.doDOMDiff(baseNode, newNode); if (subtreeDiffers) { @@ -258,6 +264,7 @@ // mark extra new nodes as modified while (newNode) { + this.debug("--found trailing new node: inserted--"); this.markNode(newNode, 'inserted'); foundDiffOverall = true; newNode = newNode.nextSibling; @@ -266,6 +273,7 @@ // If there are extra base nodes, something was deleted. Mark the parent as // having lost children for now. if (baseNode) { + this.debug("--found trailing base nodes: deleted--"); this.markNode(newParentNode, 'deleted-child'); foundDiffOverall = true; } diff --git a/js/lib/mediawiki.DOMUtils.js b/js/lib/mediawiki.DOMUtils.js index 054a19c..d105fad 100644 --- a/js/lib/mediawiki.DOMUtils.js +++ b/js/lib/mediawiki.DOMUtils.js @@ -468,10 +468,10 @@ node.nodeValue.match(/^\s*$/); }, - isNonContentNode: function(node) { - return node.nodeType === node.COMMENT_NODE || - this.isIEW(node) || - this.isMarkerMeta(node, "mw:DiffMarker"); + isContentNode: function(node) { + return node.nodeType !== node.COMMENT_NODE && + !this.isIEW(node) && + !this.isMarkerMeta(node, "mw:DiffMarker"); }, /** @@ -480,7 +480,7 @@ */ getFirstNonSepChildNode: function(node) { var child = node.firstChild; - while (child && this.isNonContentNode(child)) { + while (child && !this.isContentNode(child)) { child = child.nextSibling; } return child; @@ -488,7 +488,7 @@ previousNonSepSibling: function (node) { var prev = node.previousSibling; - while (prev && this.isNonContentNode(prev)) { + while (prev && !this.isContentNode(prev)) { prev = prev.previousSibling; } return prev; @@ -496,7 +496,7 @@ nextNonSepSibling: function (node) { var next = node.nextSibling; - while (next && this.isNonContentNode(next)) { + while (next && !this.isContentNode(next)) { next = next.nextSibling; } return next; diff --git a/js/lib/mediawiki.WikitextSerializer.js b/js/lib/mediawiki.WikitextSerializer.js index 82f4cf4..fbfa2aa 100644 --- a/js/lib/mediawiki.WikitextSerializer.js +++ b/js/lib/mediawiki.WikitextSerializer.js @@ -44,40 +44,6 @@ typeof(dsr[1]) === 'number' && dsr[1] >= 0; } -function isNonContentNode(node) { - return node.nodeType === node.COMMENT_NODE || - DU.isIEW(node) || - DU.isMarkerMeta(node, "mw:DiffMarker"); -} - -/** - * Get the first child element or non-IEW text node, ignoring - * whitespace-only text nodes and comments. - */ -function getFirstNonSepChildNode(node) { - var child = node.firstChild; - while (child && isNonContentNode(child)) { - child = child.nextSibling; - } - return child; -} - -function previousNonSepSibling(node) { - var prev = node.previousSibling; - while (prev && isNonContentNode(prev)) { - prev = prev.previousSibling; - } - return prev; -} - -function nextNonSepSibling(node) { - var next = node.nextSibling; - while (next && isNonContentNode(next)) { - next = next.nextSibling; - } - return next; -} - /** * Emit the start tag source when not round-trip testing, or when the node is * not marked with autoInsertedStart @@ -1351,7 +1317,7 @@ } function wtListEOL(node, otherNode) { - var nextSibling = nextNonSepSibling(node); + var nextSibling = DU.nextNonSepSibling(node); //console.log(nextSibling && nextSibling.nodeName); if (!DU.isElt(otherNode)) { return {min:0, max:2}; @@ -1382,13 +1348,13 @@ return { handle: function (node, state, cb) { - var firstChildElt = getFirstNonSepChildNode(node); + var firstChildElt = DU.getFirstNonSepChildNode(node); // Skip builder-inserted wrappers // Ex: <ul><s auto-inserted-start-and-end-><li>..</li><li>..</li></s>...</ul> // output from: <s>\n*a\n*b\n*c</s> while (firstChildElt && isBuilderInsertedElt(firstChildElt)) { - firstChildElt = getFirstNonSepChildNode(firstChildElt); + firstChildElt = DU.getFirstNonSepChildNode(firstChildElt); } if (!firstChildElt || ! (firstChildElt.nodeName in firstChildNames)) { @@ -1418,7 +1384,7 @@ li: { handle: function (node, state, cb) { - var firstChildElement = getFirstNonSepChildNode(node); + var firstChildElement = DU.getFirstNonSepChildNode(node); if (!DU.isList(firstChildElement)) { cb(state.serializer._getListBullets(node), node); } @@ -1447,7 +1413,7 @@ dt: { handle: function (node, state, cb) { - var firstChildElement = getFirstNonSepChildNode(node); + var firstChildElement = DU.getFirstNonSepChildNode(node); if (!DU.isList(firstChildElement)) { cb(state.serializer._getListBullets(node), node); } @@ -1474,7 +1440,7 @@ dd: { handle: function (node, state, cb) { - var firstChildElement = getFirstNonSepChildNode(node); + var firstChildElement = DU.getFirstNonSepChildNode(node); if (!DU.isList(firstChildElement)) { // XXX: handle stx: row if (node.data.parsoid.stx === 'row') { @@ -1666,7 +1632,7 @@ // <div>foo</div> a // b ((nodeName === '#text' && - otherNode === previousNonSepSibling(node) && + otherNode === DU.previousNonSepSibling(node) && // FIXME HACK: Avoid forcing two newlines if the // first line is a text node that ends up on the // same line as a block @@ -2894,11 +2860,11 @@ }; WSP._getPrevSeparatorElement = function (node, state) { - return previousNonSepSibling(node) || node.parentNode; + return DU.previousNonSepSibling(node) || node.parentNode; }; WSP._getNextSeparatorElement = function (node) { - return nextNonSepSibling(node) || node.parentNode; + return DU.nextNonSepSibling(node) || node.parentNode; }; // Gather a text line for the wikitext escaper's benefit -- To view, visit https://gerrit.wikimedia.org/r/59859 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I1a6efc6beb036ea2f8fd2e04b79677433404efc6 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
