[MediaWiki-commits] [Gerrit] WTS: Fixes to wikitext escaping for wt tag pairs (headings &... - change (mediawiki...Parsoid)
jenkins-bot has submitted this change and it was merged. Change subject: WTS: Fixes to wikitext escaping for wt tag pairs (headings & links) .. WTS: Fixes to wikitext escaping for wt tag pairs (headings & links) * Previous code was accummulating all text that might get output as part of a single line and using that to determine if "=" and [" chars needing to be escaped. However, that code felt a little convoluted, especially after the rewrite of WTS to be entirely DOM-based. * Fixed the escaping to accummulate text on a line after it is output and use that to decide escaping strategies for "=", "[", "]" chars. * This now changes serialized wikitext output. HTML: =foo''a''= ==> WT: ==foo''a''== Earlier, this used to be HTML: =foo''a''= ==> WT: ==foo''a''== Similarly, escaping of "]" and "[" chars has changed. * Based on the change, parserTests.txt need updating, but updated our local copy of wt_escape.tests.txt that I often use when testing wikitext escaping. The fixes exposed some bugs in older code. 1. = was being serialized to "===" earlier which is buggy. 2. x [Foobar] x was being serialized to [[Foo| x [Foobar] x]] but the nowiki-escaping was unnecessary. 3. ]]bar [[foo]] serialization led to "]]bar" being nowiki-escaped which was unnecessary. 4. [[Foo|x [http://google.com] x]] is parsed incorrectly compared to the PHP parser that doesn't convert [http://google.com] to a link. Currently, the wikitext escaping code does not escape the link-like text in the middle, which can lead to failures since Parsoid converts the text in the middle to a link. What is the right fix here? * Fixed in a bug in 'wikilinkHandler' that was not escaping text in this (and similar) examples: ]]bar Updated wt_escaping tests with these new tests. * Parser tests results: - 2 more wt2wt tests passing - 1 more html2html test passing - 20 more selser tests passing - 6 *fewer* html2wt tests passing, but this is because of the change in expect serialized output as explained above. Once parserTests.txt is fixed, this number will go back up. * Blacklist file updated accordingly. Change-Id: Ib784416f00c1fd23d41ce0c7c7d21b675d981e86 --- M js/lib/mediawiki.DOMUtils.js M js/lib/mediawiki.WikitextSerializer.js M js/tests/parserTests-blacklist.js M js/tests/wt_escape.tests.txt 4 files changed, 150 insertions(+), 248 deletions(-) Approvals: GWicke: Looks good to me, approved jenkins-bot: Verified diff --git a/js/lib/mediawiki.DOMUtils.js b/js/lib/mediawiki.DOMUtils.js index d105fad..42011ea 100644 --- a/js/lib/mediawiki.DOMUtils.js +++ b/js/lib/mediawiki.DOMUtils.js @@ -13,6 +13,10 @@ return node.nodeType === Node.ELEMENT_NODE; }, + isText: function(node) { + return node.nodeType === Node.TEXT_NODE; + }, + isBlockNode: function(node) { return node && Util.isBlockTag(node.nodeName.toLowerCase()); }, @@ -463,9 +467,8 @@ * Is a node representing inter-element ws? */ isIEW: function (node) { - return node.nodeType === node.TEXT_NODE && - // ws-only - node.nodeValue.match(/^\s*$/); + // ws-only + return this.isText(node) && node.nodeValue.match(/^\s*$/); }, isContentNode: function(node) { diff --git a/js/lib/mediawiki.WikitextSerializer.js b/js/lib/mediawiki.WikitextSerializer.js index 19044c2..8f54e78 100644 --- a/js/lib/mediawiki.WikitextSerializer.js +++ b/js/lib/mediawiki.WikitextSerializer.js @@ -139,31 +139,27 @@ WEHP.urlParser = new PegTokenizer(); -WEHP.headingHandler = function(state, text) { - // replace heading-handler with the default handler - // only "=" at the extremities trigger escaping - - if (state.currLine && !state.currLine.seen) { - var line = state.currLine.text, - len = line ? line.length : 0; - return (line && len > 2 && (line[0] === '=') && (line[len-1] === '=')); +WEHP.headingHandler = function(headingNode, state, text, opts) { + // Only "=" at the extremities trigger escaping + if (opts.isLastChild && DU.isText(headingNode.firstChild)) { + var line = state.currLine.text; + if (line.length === 0) { + line = text; + } + return line[0] === '=' && + text && text.length > 0 && text[text.length-1] === '='; } else { return false; } }; -WEHP.liHandler = function(state, text) { - // replace li-handler with the default handler - // only bullets at the beginning of the list trigger escaping - if (state.currLine && !state.currLine.seen) { +WEHP.liHandler = function(liNode, state, text) { + // On
[MediaWiki-commits] [Gerrit] WTS: Fixes to wikitext escaping for wt tag pairs (headings &... - change (mediawiki...Parsoid)
Subramanya Sastry has uploaded a new change for review. https://gerrit.wikimedia.org/r/60300 Change subject: WTS: Fixes to wikitext escaping for wt tag pairs (headings & links) .. WTS: Fixes to wikitext escaping for wt tag pairs (headings & links) * Previous code was accumulating all text that might get output as part of a single line and using that to determine if "=" and [" chars needing to be escaped. However, that code felt a little convoluted, especially after the rewrite of WTS to be entirely DOM-based. * Fixed the escaping to accumulate text on a line after it is output and use that to decide escaping strategies for "=", "[", "]" chars. * This now changes serialized wikitext output. HTML: =foo''a''= ==> WT: ==foo''a''== Earlier, this used to be HTML: =foo''a''= ==> WT: ==foo''a''== Similarly, escaping of "]" and "[" chars has changed. * Based on the change, parserTests.txt need updating, but updated our local copy of wt_escape.tests.txt that I often use when testing wikitext escaping. The fixes exposed some bugs in older code. 1. = was being serialied to "===" earlier which is buggy. 2. x [Foobar] x was being serialized to [[Foo| x [Foobar] x]] but the nowiki escaping was unnecessary. 3. [[Foo|x [http://google.com] x]] is parsed incorrectly compared to the PHP parser that doesn't convert [http://google.com] to a link. Currently, the wikitext escaping code does not escape the link-like text in the middle, which can lead to failures since Parsoid converts the text in the middle to a link. What is the right fix here? * Parser tests results: - 2 more wt2wt tests passing - 1 more html2html test passing - 20 more selser tests passing - 6 *fewer* html2wt tests passing, but this is because of the change in expect serialized output as explained above. Once parserTests.txt is fixed, this number will go back up. Change-Id: Ib784416f00c1fd23d41ce0c7c7d21b675d981e86 --- M js/lib/mediawiki.DOMUtils.js M js/lib/mediawiki.WikitextSerializer.js M js/tests/wt_escape.tests.txt 3 files changed, 137 insertions(+), 228 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Parsoid refs/changes/00/60300/1 diff --git a/js/lib/mediawiki.DOMUtils.js b/js/lib/mediawiki.DOMUtils.js index d105fad..42011ea 100644 --- a/js/lib/mediawiki.DOMUtils.js +++ b/js/lib/mediawiki.DOMUtils.js @@ -13,6 +13,10 @@ return node.nodeType === Node.ELEMENT_NODE; }, + isText: function(node) { + return node.nodeType === Node.TEXT_NODE; + }, + isBlockNode: function(node) { return node && Util.isBlockTag(node.nodeName.toLowerCase()); }, @@ -463,9 +467,8 @@ * Is a node representing inter-element ws? */ isIEW: function (node) { - return node.nodeType === node.TEXT_NODE && - // ws-only - node.nodeValue.match(/^\s*$/); + // ws-only + return this.isText(node) && node.nodeValue.match(/^\s*$/); }, isContentNode: function(node) { diff --git a/js/lib/mediawiki.WikitextSerializer.js b/js/lib/mediawiki.WikitextSerializer.js index 19044c2..4346878 100644 --- a/js/lib/mediawiki.WikitextSerializer.js +++ b/js/lib/mediawiki.WikitextSerializer.js @@ -139,23 +139,23 @@ WEHP.urlParser = new PegTokenizer(); -WEHP.headingHandler = function(state, text) { - // replace heading-handler with the default handler - // only "=" at the extremities trigger escaping - - if (state.currLine && !state.currLine.seen) { - var line = state.currLine.text, - len = line ? line.length : 0; - return (line && len > 2 && (line[0] === '=') && (line[len-1] === '=')); +WEHP.headingHandler = function(headingNode, state, text, opts) { + // Only "=" at the extremities trigger escaping + if (opts.isLastChild && DU.isText(headingNode.firstChild)) { + var line = state.currLine.text; + if (line.length === 0) { + line = text; + } + return line[0] === '=' && + text && text.length > 0 && text[text.length-1] === '='; } else { return false; } }; -WEHP.liHandler = function(state, text) { - // replace li-handler with the default handler - // only bullets at the beginning of the list trigger escaping - if (state.currLine && !state.currLine.seen) { +WEHP.liHandler = function(liNode, state, text) { + // Only bullets at the beginning of the list trigger escaping + if (DU.isText(liNode.firstChild) && state.currLine.text === '') { return text.match(/^[#\*:;]/); } else { return false; @@ -184,14 +184,8 @@