Subramanya Sastry has uploaded a new change for review.
https://gerrit.wikimedia.org/r/70866
Change subject: Experimental: Improved selser handling for subtree-changed nodes
......................................................................
Experimental: Improved selser handling for subtree-changed nodes
* So far, if a node was marked 'subtree-changed', in addition
to descending down the tree to look for which specific child was
modified, the serializer used regular WTS for the node itself.
So for example, if a table cell is modified, the table itself
is marked 'subtree-changed' and goes through normal serialization
which leads to the table-wrapper (and all intervening nodes)
getting normalized.
* The current patch recognizes the 'subtree-changed' diff marker
and if the dsr values are valid, it enables serialization of the
opening and closing tags from original source. The separators
are still normalized currently, but that can be a future
enhancement.
* Since handlers are responsible for handling a node's children,
right now, the wrapperUnmodified flag is passed into all handlers
and the handlers decide what they want to do with the flag.
In this patch, table/td/th/tr handlers and the html-handler
exploit this information, and in later patches, other handlers
could potentially be enhanced as well.
* 9 selser tests are being flagged as failures. However, this is
an artefact of how we test for success/failure of selser tests.
Since we compare selser output with wt2wt output, if the wt2wt
output itself is incorrect (because of normalization), selser
failure/success flagging is not trustworthy. As it turns out,
in this instance, wt2wt is at fault and the 9 selser tests are
actually an improved output.
This test framework limitation will be addressed separately.
Change-Id: Idc7c2dbf12f0146b57edd6d053005189ac25f753
---
M js/lib/mediawiki.DOMUtils.js
M js/lib/mediawiki.WikitextSerializer.js
M js/tests/parserTests-blacklist.js
3 files changed, 74 insertions(+), 44 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Parsoid
refs/changes/66/70866/1
diff --git a/js/lib/mediawiki.DOMUtils.js b/js/lib/mediawiki.DOMUtils.js
index 0b747b5..7cdc2c2 100644
--- a/js/lib/mediawiki.DOMUtils.js
+++ b/js/lib/mediawiki.DOMUtils.js
@@ -486,17 +486,6 @@
return tokBuf;
},
- /**
- * Helper function to check for a change marker in data-ve-changed
structure
- */
- isModificationChangeMarker: function( dvec ) {
- return dvec && (
- dvec['new'] || dvec.attributes ||
- dvec.content || dvec.annotations ||
- dvec.childrenRemoved || dvec.rebuilt
- );
- },
-
currentDiffMark: function(node, env) {
if (!node || !this.isElt(node)) {
return false;
@@ -512,6 +501,11 @@
return this.currentDiffMark(node, env) !== null;
},
+ onlySubtreeChanged: function(node, env) {
+ var dmark = this.currentDiffMark(node, env);
+ return dmark && dmark.diff.length === 1 && dmark.diff[0] ===
'subtree-changed';
+ },
+
hasInsertedOrModifiedDiffMark: function(node, env) {
var diffMark = this.currentDiffMark(node, env);
return diffMark &&
diff --git a/js/lib/mediawiki.WikitextSerializer.js
b/js/lib/mediawiki.WikitextSerializer.js
index e405f63..c934166 100644
--- a/js/lib/mediawiki.WikitextSerializer.js
+++ b/js/lib/mediawiki.WikitextSerializer.js
@@ -45,6 +45,12 @@
typeof(dsr[1]) === 'number' && dsr[1] >= 0;
}
+function hasValidTagWidths(dsr) {
+ return dsr &&
+ typeof(dsr[2]) === 'number' && dsr[2] >= 0 &&
+ typeof(dsr[3]) === 'number' && dsr[3] >= 0;
+}
+
/**
* Emit the start tag source when not round-trip testing, or when the node is
* not marked with autoInsertedStart
@@ -832,14 +838,20 @@
this.handleImage( node, state, cb );
};
-WSP._serializeTableTag = function ( symbol, endSymbol, state, token ) {
- var sAttribs = this._serializeAttributes(state, token);
- if (sAttribs.length > 0) {
- // IMPORTANT: 'endSymbol !== null' NOT 'endSymbol' since the ''
string
- // is a falsy value and we want to treat it as a truthy value.
- return symbol + ' ' + sAttribs + (endSymbol !== null ?
endSymbol : ' |');
+WSP._serializeTableTag = function ( symbol, endSymbol, state, node,
wrapperUnmodified ) {
+ if (wrapperUnmodified) {
+ var dsr = node.data.parsoid.dsr;
+ return state.getOrigSrc(dsr[0], dsr[0]+dsr[2]);
} else {
- return symbol + (endSymbol || '');
+ var token = DU.mkTagTk(node);
+ var sAttribs = this._serializeAttributes(state, token);
+ if (sAttribs.length > 0) {
+ // IMPORTANT: 'endSymbol !== null' NOT 'endSymbol'
since the '' string
+ // is a falsy value and we want to treat it as a truthy
value.
+ return symbol + ' ' + sAttribs + (endSymbol !== null ?
endSymbol : ' |');
+ } else {
+ return symbol + (endSymbol || '');
+ }
}
};
@@ -856,7 +868,13 @@
}
};
-WSP._serializeHTMLTag = function ( state, token ) {
+WSP._serializeHTMLTag = function ( state, node, wrapperUnmodified ) {
+ if (wrapperUnmodified) {
+ var dsr = node.data.parsoid.dsr;
+ return state.getOrigSrc(dsr[0], dsr[0]+dsr[2]);
+ }
+
+ var token = DU.mkTagTk(node);
var da = token.dataAttribs;
if ( token.name === 'pre' ) {
// html-syntax pre is very similar to nowiki
@@ -881,7 +899,13 @@
}
};
-WSP._serializeHTMLEndTag = function ( state, token ) {
+WSP._serializeHTMLEndTag = function ( state, node, wrapperUnmodified ) {
+ if (wrapperUnmodified) {
+ var dsr = node.data.parsoid.dsr;
+ return state.getOrigSrc(dsr[1]-dsr[3], dsr[1]);
+ }
+
+ var token = DU.mkEndTagTk(node);
if ( token.name === 'pre' ) {
state.inHTMLPre = false;
}
@@ -1960,9 +1984,9 @@
// XXX: handle options
table: {
- handle: function (node, state, cb) {
+ handle: function (node, state, cb, wrapperUnmodified) {
var wt = node.data.parsoid.startTagSrc || "{|";
- cb(state.serializer._serializeTableTag(wt, '', state,
DU.mkTagTk(node)), node);
+ cb(state.serializer._serializeTableTag(wt, '', state,
node, wrapperUnmodified), node);
state.serializeChildren(node, cb);
emitEndTag(node.data.parsoid.endTagSrc || "|}", node,
state, cb);
},
@@ -1987,13 +2011,13 @@
}
},
tr: {
- handle: function (node, state, cb) {
+ handle: function (node, state, cb, wrapperUnmodified) {
// If the token has 'startTagSrc' set, it means that
the tr was present
// in the source wikitext and we emit it -- if not, we
ignore it.
var dp = node.data.parsoid;
if (node.previousSibling || dp.startTagSrc) {
var res =
state.serializer._serializeTableTag(dp.startTagSrc || "|-", '', state,
- DU.mkTagTk(node) );
+ node, wrapperUnmodified
);
emitStartTag(res, node, state, cb);
}
state.serializeChildren(node, cb);
@@ -2013,14 +2037,14 @@
}
},
th: {
- handle: function (node, state, cb) {
+ handle: function (node, state, cb, wrapperUnmodified) {
var dp = node.data.parsoid, res;
if ( dp.stx_v === 'row' ) {
res =
state.serializer._serializeTableTag(dp.startTagSrc || "!!",
- dp.attrSepSrc || null,
state, DU.mkTagTk(node));
+ dp.attrSepSrc || null,
state, node, wrapperUnmodified);
} else {
res =
state.serializer._serializeTableTag(dp.startTagSrc || "!", dp.attrSepSrc ||
null,
- state, DU.mkTagTk(node));
+ state, node, wrapperUnmodified);
}
emitStartTag(res, node, state, cb);
state.serializeChildren(node, cb,
state.serializer.wteHandlers.thHandler);
@@ -2038,16 +2062,16 @@
}
},
td: {
- handle: function (node, state, cb) {
+ handle: function (node, state, cb, wrapperUnmodified) {
var dp = node.data.parsoid, res;
if ( dp.stx_v === 'row' ) {
res =
state.serializer._serializeTableTag(dp.startTagSrc || "||",
- dp.attrSepSrc || null, state,
DU.mkTagTk(node));
+ dp.attrSepSrc || null, state,
node, wrapperUnmodified);
} else {
// If the HTML for the first td is not enclosed
in a tr-tag,
// we start a new line. If not, tr will have
taken care of it.
res =
state.serializer._serializeTableTag(dp.startTagSrc || "|",
- dp.attrSepSrc || null, state,
DU.mkTagTk(node));
+ dp.attrSepSrc || null, state,
node, wrapperUnmodified);
}
// FIXME: bad state hack!
@@ -2072,11 +2096,11 @@
}
},
caption: {
- handle: function (node, state, cb) {
+ handle: function (node, state, cb, wrapperUnmodified) {
var dp = node.data.parsoid;
// Serialize the tag itself
var res = state.serializer._serializeTableTag(
- dp.startTagSrc || "|+", null, state,
DU.mkTagTk(node));
+ dp.startTagSrc || "|+", null, state,
node, wrapperUnmodified);
emitStartTag(res, node, state, cb);
state.serializeChildren(node, cb);
},
@@ -2612,7 +2636,7 @@
}
};
-WSP._htmlElementHandler = function (node, state, cb) {
+WSP._htmlElementHandler = function (node, state, cb, wrapperUnmodified) {
// Wikitext supports the following list syntax:
//
// * <li class="a"> hello world
@@ -2621,7 +2645,7 @@
// specially reconstruct the above from a single <li> tag.
this._handleLIHackIfApplicable(node, cb);
- emitStartTag(this._serializeHTMLTag(state, DU.mkTagTk(node)),
+ emitStartTag(this._serializeHTMLTag(state, node, wrapperUnmodified),
node, state, cb);
if (node.childNodes.length) {
var inPHPBlock = state.inPHPBlock;
@@ -2631,7 +2655,7 @@
state.serializeChildren(node, cb);
state.inPHPBlock = inPHPBlock;
}
- emitEndTag(this._serializeHTMLEndTag(state, DU.mkEndTagTk(node)),
+ emitEndTag(this._serializeHTMLEndTag(state, node, wrapperUnmodified),
node, state, cb);
};
@@ -2841,7 +2865,6 @@
}
if ( dp.src !== undefined ) {
- // Source-based template/extension round-tripping for now
//console.log(node.parentNode.outerHTML);
if (/^mw:Placeholder(\/\w*)?$/.test(typeOf) ||
(typeOf === "mw:Nowiki" && node.textContent ===
dp.src )) {
@@ -3535,12 +3558,13 @@
node, domHandler);
}
- var handled = false;
+ var handled = false, wrapperUnmodified = false;
// WTS should not be in a subtree with a modification
flag that applies
// to every node of a subtree (rather than an
indication that some node
// in the subtree is modified).
- if (state.selserMode && !state.inModifiedContent) {
+ if (state.selserMode && !state.inModifiedContent &&
+ dp && isValidDSR(dp.dsr) && dp.dsr[1] >
dp.dsr[0]) {
// To serialize from source, we need 3 things
of the node:
// -- it should not have a diff marker
// -- it should have valid, usable DSR
@@ -3556,9 +3580,7 @@
//
// TO BE DONE
//
- if (dp && isValidDSR(dp.dsr) &&
- (dp.dsr[1] > dp.dsr[0]) &&
- !DU.hasCurrentDiffMark(node, this.env))
{
+ if (!DU.hasCurrentDiffMark(node, this.env)) {
// Strip leading/trailing separators
*ONLY IF* the previous/following
// node will go through non-selser
serialization.
var src = state.getOrigSrc(dp.dsr[0],
dp.dsr[1]),
@@ -3596,6 +3618,11 @@
if
(/\bmw:(?:Transclusion\b|Param\b|Extension\/[^\s]+)/.test(typeOf)) {
nextNode =
this.skipOverEncapsulatedContent(node);
}
+ } else if (DU.onlySubtreeChanged(node,
this.env) &&
+ hasValidTagWidths(dp.dsr) &&
+ !dp.autoInsertedStart &&
!dp.autoInsertedEnd)
+ {
+ wrapperUnmodified = true;
}
}
@@ -3609,7 +3636,7 @@
if ( domHandler && domHandler.handle ) {
// DOM-based serialization
try {
- nextNode =
domHandler.handle(node, state, cb);
+ nextNode =
domHandler.handle(node, state, cb, wrapperUnmodified);
} catch(e) {
console.error(e.stack ||
e.toString());
console.error(node.nodeName,
domHandler);
@@ -3639,8 +3666,8 @@
prev = this._getPrevSeparatorElement(node,
state);
if (prev) {
this.updateSeparatorConstraints(state,
- prev,
this._getDOMHandler(prev, state, cb),
- node, {});
+ prev,
this._getDOMHandler(prev, state, cb),
+ node, {});
}
// regular serialization
this._serializeTextNode(node, state, cb );
diff --git a/js/tests/parserTests-blacklist.js
b/js/tests/parserTests-blacklist.js
index 6e43e5c..7218878 100644
--- a/js/tests/parserTests-blacklist.js
+++ b/js/tests/parserTests-blacklist.js
@@ -2715,12 +2715,16 @@
add("selser", "Allow +/- in 2nd and later cells in a row, in 1st cell when
td-attrs are present, or in 1st cell when there is a space between \"|\" and
+/- [[2,[[3,[2],4,3],2,[4,2,0,0,2,3,4,2],0,2,0]]]");
add("selser", "Table rowspan [[3,2]]");
add("selser", "Table rowspan [[0,1]]");
+add("selser", "Table rowspan [[2,[[2,0,3,0,4],0,[0,0,0,3],2]]]");
add("selser", "Table rowspan [2]");
add("selser", "Table rowspan [1]");
add("selser", "Table rowspan [[0,[1,0,2,0]]]");
+add("selser", "Table rowspan [[3,4]]");
add("selser", "Table rowspan [[2,2]]");
add("selser", "Table rowspan [[0,[1,0,2,2]]]");
add("selser", "Table rowspan [[3,[1,0,[0,2,0,4],2]]]");
+add("selser", "Table rowspan [[0,[[4,0,4,0,1],0,1,2]]]");
+add("selser", "Table rowspan [[0,4]]");
add("selser", "Table rowspan [[4,1]]");
add("selser", "Table rowspan [[0,[2,0,4,0]]]");
add("selser", "Table rowspan [[2,1]]");
@@ -2728,14 +2732,19 @@
add("selser", "Nested table [[0,[1,4]]]");
add("selser", "Nested table [[0,2]]");
add("selser", "Nested table [2]");
+add("selser", "Nested table [[2,[4,2]]]");
add("selser", "Nested table [[4,1]]");
add("selser", "Nested table [1]");
add("selser", "Nested table [[3,2]]");
add("selser", "Nested table [[2,1]]");
+add("selser", "Nested table [[3,[3,0]]]");
add("selser", "Nested table [[0,[[[4,[4]],2,1,3,[3]],2]]]");
add("selser", "Nested table [[3,[1,4]]]");
+add("selser", "Nested table [[0,4]]");
+add("selser", "Nested table [[0,[[1,2,[3,4],0,4],2]]]");
add("selser", "Nested table [[0,[1,2]]]");
add("selser", "Nested table [[0,1]]");
+add("selser", "Nested table [[0,[[2,0,[4,[0,[0,0,[0,3],2]]],0,0],4]]]");
add("selser", "Invalid attributes in table cell (bug 1830) [1]");
add("selser", "Invalid attributes in table cell (bug 1830) [[4,1]]");
add("selser", "Invalid attributes in table cell (bug 1830) [2]");
--
To view, visit https://gerrit.wikimedia.org/r/70866
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Idc7c2dbf12f0146b57edd6d053005189ac25f753
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
