Subramanya Sastry has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/61800


Change subject: Don't ignore data-parsoid in DOMDiff
......................................................................

Don't ignore data-parsoid in DOMDiff

* In VE, sometimes wrappers get moved around without their content
  which can cause p-wrapper of deleted content to be used on
  identical undeleted content.  Since the identical undeleted content
  is marked as unchanged, selser tries to emit use dsr from its wrapper
  to emit original src, but the wrapper is not the right one and leads
  selser to emit more/less text depending on how VE manipulated the
  wrappers.

  Discovered via: /mnt/bugs/2013-05-01T09:43:14.960Z-Reverse_innovation

* Unrelated fixes: Updated debugging output in DOMDiff.

* Tested via:

node parse --html2wt --selser --domdiff domdiff.diff.html --oldtextfile wt < 
editedHtml

   which emitted duplicate text before this patch and emits correct
   text after this patch.

* parserTests results:
  - Inexplicably, this patch causes selser to reuse more original
    source fragments on parserTests.

  -  Leads to one selser test passing (which passes in wt2wt mode),
     and 3 selser tests failing (all of which fail in wt2wt mode as well).

  -  Updated blacklist.

Change-Id: I7283f649e7f32828238c1d865b83e73f1a468079
---
M js/lib/mediawiki.DOMDiff.js
M js/tests/parserTests-blacklist.js
2 files changed, 14 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Parsoid 
refs/changes/00/61800/1

diff --git a/js/lib/mediawiki.DOMDiff.js b/js/lib/mediawiki.DOMDiff.js
index 9945454..9212b8c 100644
--- a/js/lib/mediawiki.DOMDiff.js
+++ b/js/lib/mediawiki.DOMDiff.js
@@ -46,7 +46,11 @@
        // subtree actually changes.  So, ignoring this attribute in effect,
        // ignores the parser tests change.
        'data-parsoid-changed': 1,
-       'data-parsoid': 1,
+       // SSS: Don't ignore data-parsoid because in VE, sometimes wrappers get
+       // moved around without their content which occasionally leads to 
incorrect
+       // DSR being used by selser.  Hard to describe a reduced test case here.
+       // Discovered via: /mnt/bugs/2013-05-01T09:43:14.960Z-Reverse_innovation
+       // 'data-parsoid': 1,
        'data-parsoid-diff': 1,
        'about': 1
 };
@@ -151,10 +155,11 @@
 DDP.doDOMDiff = function ( baseParentNode, newParentNode ) {
        var dd = this;
 
-       function debugOut(nodeA, nodeB) {
+       function debugOut(nodeA, nodeB, laPrefix) {
+               laPrefix = laPrefix || "";
                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)));
+                       dd.debug("--> A" + laPrefix + ":" + (DU.isElt(nodeA) ? 
nodeA.outerHTML : JSON.stringify(nodeA.nodeValue)));
+                       dd.debug("--> B" + laPrefix + ":" + (DU.isElt(nodeB) ? 
nodeB.outerHTML : JSON.stringify(nodeB.nodeValue)));
                }
        }
 
@@ -181,7 +186,7 @@
                                this.debug("--lookahead in new dom--");
                                lookaheadNode = newNode.nextSibling;
                                while (lookaheadNode) {
-                                       debugOut(baseNode, lookaheadNode);
+                                       debugOut(baseNode, lookaheadNode, 
"new");
                                        if (DU.isContentNode(lookaheadNode) &&
                                                this.treeEquals(baseNode, 
lookaheadNode, true))
                                        {
@@ -205,7 +210,7 @@
                                this.debug("--lookahead in old dom--");
                                lookaheadNode = baseNode.nextSibling;
                                while (lookaheadNode) {
-                                       debugOut(lookaheadNode, newNode);
+                                       debugOut(lookaheadNode, newNode, "old");
                                        if (DU.isContentNode(lookaheadNode) &&
                                                this.treeEquals(lookaheadNode, 
newNode, true))
                                        {
diff --git a/js/tests/parserTests-blacklist.js 
b/js/tests/parserTests-blacklist.js
index 222ac5a..3e1ab47 100644
--- a/js/tests/parserTests-blacklist.js
+++ b/js/tests/parserTests-blacklist.js
@@ -1955,7 +1955,6 @@
 
 
 // Blacklist for selser
-add("selser", "Italics and bold [[3,4,3,[0,[0]]]]");
 add("selser", "Italics and bold: 2-quote opening sequence: (2,3) [2]");
 add("selser", "Italics and bold: 2-quote opening sequence: (2,3) [[[0]]]");
 add("selser", "Italics and bold: 2-quote opening sequence: (2,3) [4]");
@@ -2090,6 +2089,7 @@
 add("selser", "<nowiki> inside <pre> (bug 13238) [4,0,[0,2,0]]");
 add("selser", "<nowiki> inside <pre> (bug 13238) [1,0,[0,0]]");
 add("selser", "<nowiki> inside <pre> (bug 13238) [3,0,4,0,[1,0,3,0]]");
+add("selser", "<nowiki> inside <pre> (bug 13238) [3,0,3,0,[1,0,0]]");
 add("selser", "<nowiki> inside <pre> (bug 13238) [2,0,2,0,[4,0,0]]");
 add("selser", "<nowiki> inside <pre> (bug 13238) [4,0,[0,3,0]]");
 add("selser", "<nowiki> inside <pre> (bug 13238) [3,0,[0,3,0]]");
@@ -2098,6 +2098,7 @@
 add("selser", "<nowiki> and <pre> preference (first one wins) [[0]]");
 add("selser", "<nowiki> and <pre> preference (first one wins) 
[4,0,2,0,[4,0]]");
 add("selser", "<nowiki> and <pre> preference (first one wins) [1,0,3,0,2,0]");
+add("selser", "<nowiki> and <pre> preference (first one wins) 
[3,0,3,0,[[0]]]");
 add("selser", "<nowiki> and <pre> preference (first one wins) [3,0,[0]]");
 add("selser", "<nowiki> and <pre> preference (first one wins) [4,0,[0]]");
 add("selser", "<nowiki> and <pre> preference (first one wins) [4,0,2,0,1,0]");
@@ -2530,6 +2531,7 @@
 add("selser", "Template with targets containing wikilinks [2,0,4,0,[0]]");
 add("selser", "Template with targets containing wikilinks [2,0,1,0,[0]]");
 add("selser", "Template with targets containing wikilinks [2,0,3,0,1]");
+add("selser", "Template with targets containing wikilinks [3,0,3,0,[0]]");
 add("selser", "Template with targets containing wikilinks [4,0,1,0,1]");
 add("selser", "Template with targets containing wikilinks [1,0,1,0,4]");
 add("selser", "msgnw keyword [[0]]");

-- 
To view, visit https://gerrit.wikimedia.org/r/61800
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7283f649e7f32828238c1d865b83e73f1a468079
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

Reply via email to