Subramanya Sastry has uploaded a new change for review.
https://gerrit.wikimedia.org/r/59421
Change subject: Fixes to DOMDiff and Selser code to better handle modified
content.
......................................................................
Fixes to DOMDiff and Selser code to better handle modified content.
* Deleted some dead code from DOMUtils.
* Used node.data loading to eliminate repeated parses of
data-parsoid-diff attribute.
* Improved dom-diff marking to distinguish between a modified
wrapper vs. a modified subtree.
Ex: modification of attributes of a node vs. modification of
the wrapper itself.
* So, dom-diff has two kinds of markers: markers that apply to
just the node vs. markers that apply to the entire subtree rooted
at a node. 'modified' and 'inserted' markers belong to the
second category -- this kind eliminates the need to litter all
nodes of a sub-tree with the same diff marker. If a node is
marked modified/inserted, WTS-selser treats the the entire
sub-tree as modified.
TODO: Possibly redo the marker naming scheme to better
reflect this difference.
* Since 'modified' and 'inserted' markers are now handled more
accurately both by dom-diff and the serializer, 7 more selser
tests are now passing.
Change-Id: I9895f079d607abcebee0ccee710f9a350411935e
---
M js/lib/mediawiki.DOMDiff.js
M js/lib/mediawiki.DOMUtils.js
M js/lib/mediawiki.WikitextSerializer.js
3 files changed, 44 insertions(+), 58 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Parsoid
refs/changes/21/59421/1
diff --git a/js/lib/mediawiki.DOMDiff.js b/js/lib/mediawiki.DOMDiff.js
index 28449ce..4be7ff9 100644
--- a/js/lib/mediawiki.DOMDiff.js
+++ b/js/lib/mediawiki.DOMDiff.js
@@ -10,12 +10,9 @@
*/
function DOMDiff ( env ) {
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
+ this.debugging = env.conf.parsoid.debug ||
+ (env.conf.parsoid.traceFlags &&
env.conf.parsoid.traceFlags.indexOf('selser') !== -1);
+ this.debug = this.debugging ? 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.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
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 the 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,7 +282,6 @@
// 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);
} else if (node.nodeType !== node.TEXT_NODE && node.nodeType
!== node.COMMENT_NODE) {
diff --git a/js/lib/mediawiki.DOMUtils.js b/js/lib/mediawiki.DOMUtils.js
index beabf9c..b23a677 100644
--- a/js/lib/mediawiki.DOMUtils.js
+++ b/js/lib/mediawiki.DOMUtils.js
@@ -419,12 +419,26 @@
);
},
- hasCurrentDiffMark: function(node, env) {
+ currentDiffMark: function(node, env) {
if (!node || !this.isElt(node)) {
return false;
}
- var dpd = this.getJSONAttribute(node, 'data-parsoid-diff',
null);
- return dpd !== null && dpd.id === env.page.id;
+ if ( !( node.data && node.data["parsoid-diff"] ) ) {
+ this.loadDataAttrib(node, "parsoid-diff");
+ }
+ var dpd = node.data["parsoid-diff"];
+ return dpd !== {} && dpd.id === env.page.id ? dpd : null;
+ },
+
+ hasCurrentDiffMark: function(node, env) {
+ return this.currentDiffMark(node, env) !== null;
+ },
+
+ hasInsertedOrModifiedDiffMark: function(node, env) {
+ var diffMark = this.currentDiffMark(node, env);
+ return diffMark &&
+ (diffMark.diff.indexOf('modified') >= 0 ||
+ diffMark.diff.indexOf('inserted') >= 0);
},
setDiffMark: function(node, env, change) {
@@ -452,40 +466,6 @@
return node.nodeType === node.TEXT_NODE &&
// ws-only
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;
},
/**
diff --git a/js/lib/mediawiki.WikitextSerializer.js
b/js/lib/mediawiki.WikitextSerializer.js
index 21320f5..82f4cf4 100644
--- a/js/lib/mediawiki.WikitextSerializer.js
+++ b/js/lib/mediawiki.WikitextSerializer.js
@@ -3015,7 +3015,7 @@
}
var handled = false;
- if (state.selserMode) {
+ if (state.selserMode && !state.inModifiedContent) {
// To serialize from source, we need 2 things
of the node:
// -- it should not have a diff marker
// -- it should have valid, usable DSR
@@ -3073,6 +3073,9 @@
if ( !handled ) {
state.prevNodeUnmodified =
state.currNodeUnmodified;
state.currNodeUnmodified = false;
+ if (state.selserMode &&
DU.hasInsertedOrModifiedDiffMark(node, this.env)) {
+ state.inModifiedContent = true;
+ }
// console.warn("USED NEW");
if ( domHandler && domHandler.handle ) {
// DOM-based serialization
@@ -3089,6 +3092,7 @@
// Used to be token-based serialization
console.error('No dom handler found
for', node.outerHTML);
}
+ state.inModifiedContent = false;
}
// Update end separator constraints
--
To view, visit https://gerrit.wikimedia.org/r/59421
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9895f079d607abcebee0ccee710f9a350411935e
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
