Skip to content

Commit

Permalink
Dont wrap IEW with spans when they are in fosterable posns in tables
Browse files Browse the repository at this point in the history
* On the following wikitext snippet, Parsoid was adding span wrappers
  around IEW emitted by the {{WDL}} tpl. These would then get fostered
  out and introduce dirty diffs by emitting <span></span> on RTing.
----
{|
|5th {{WDL|7|2|2|3}} || 5 || 7 || 6
|}
----

* This patch makes fixes by not span-wrapping IEW in fosterable posns
  in tables.

* Also fixed getAboutSiblings to continue collecting siblings across
  IEW gaps when in fosterable position.

* Fixes parsing of above snippet.

* No change in parser test results (TODO: need to come up with a good
  test case for this to prevent regressions).

Change-Id: Idaf069866510f63921334c272982ec6e38297bea
  • Loading branch information
subbuss committed Jul 2, 2013
1 parent 613287b commit 65bd7ae
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 28 deletions.
20 changes: 13 additions & 7 deletions js/lib/mediawiki.DOMPostProcessor.js
Expand Up @@ -1329,13 +1329,19 @@ function encapsulateTemplates( doc, env, tplRanges, tplArrays) {
var next = n.nextSibling;

if ( n.nodeType === Node.TEXT_NODE || n.nodeType === Node.COMMENT_NODE ) {
span = doc.createElement( 'span' );
span.setAttribute( 'about', about );
// attach the new span to the DOM
n.parentNode.insertBefore( span, n );
// move the text node into the span
span.appendChild( n );
n = span;
// Dont add span-wrappers in fosterable positions
//
// NOTE: there cannot be any non-IEW text in fosterable position
// since the HTML tree builder would already have fostered it out.
if (!DU.isFosterablePosition(n)) {
span = doc.createElement( 'span' );
span.setAttribute( 'about', about );
// attach the new span to the DOM
n.parentNode.insertBefore( span, n );
// move the text node into the span
span.appendChild( n );
n = span;
}
} else if (n.nodeType === Node.ELEMENT_NODE) {
n.setAttribute( 'about', about );
}
Expand Down
38 changes: 23 additions & 15 deletions js/lib/mediawiki.DOMUtils.js
Expand Up @@ -672,6 +672,22 @@ var DOMUtils = {
}
},

getAboutSiblings: function(node, about) {
var nodes = [node];

node = node.nextSibling;
while (node && (
this.isElt(node) && node.getAttribute('about') === about ||
this.isFosterablePosition(node) && !this.isElt(node) && this.isIEW(node)
))
{
nodes.push(node);
node = node.nextSibling;
}

return nodes;
},

/**
* Extract transclusion and extension expansions from a DOM, and return
* them in a structure like this:
Expand All @@ -697,6 +713,8 @@ var DOMUtils = {
* }
*/
extractExpansions: function (doc) {
var DU = this;

var node = doc.body,
expansion,
expansions = {
Expand All @@ -705,17 +723,6 @@ var DOMUtils = {
files: {}
};

function getAboutSiblings(node, about) {
var nodes = [node];
node = node.nextSibling;
while (node && node.nodeType === node.ELEMENT_NODE &&
node.getAttribute('about') === about)
{
nodes.push(node);
node = node.nextSibling;
}
return nodes;
}

function doExtractExpansions (node) {
var nodes, expAccum,
Expand All @@ -731,8 +738,8 @@ var DOMUtils = {
.test(typeOf) && about) ||
/\b(?:mw:Image(?:\b|\/))/.test(typeOf))
{
DOMUtils.loadDataParsoid(node);
nodes = getAboutSiblings(node, about);
DU.loadDataParsoid(node);
nodes = DU.getAboutSiblings(node, about);
var key;
if (/\bmw:Transclusion\b/.test(typeOf)) {
expAccum = expansions.transclusions;
Expand Down Expand Up @@ -817,6 +824,7 @@ var DOMUtils = {
* mainly for transclusion and extension processing.
*/
getWrapperTokens: function ( nodes ) {
var DU = this;
function makeWrapperForNode ( node ) {
var workNode;
if (node.nodeType === node.ELEMENT_NODE && node.childNodes.length) {
Expand All @@ -837,7 +845,7 @@ var DOMUtils = {
}
var res = [];
// Now convert our node to tokens
DOMUtils.convertDOMtoTokens(res, workNode);
DU.convertDOMtoTokens(res, workNode);
return res;
}

Expand All @@ -846,7 +854,7 @@ var DOMUtils = {
// broken things up already when building the DOM for the first time.
//var hasBlockElement = false;
//for (var i = 0; i < nodes.length; i++) {
// if (DOMUtils.hasBlockElementDescendant(nodes[i])) {
// if (DU.hasBlockElementDescendant(nodes[i])) {
// hasBlockElement = true;
// break;
// }
Expand Down
7 changes: 1 addition & 6 deletions js/lib/mediawiki.WikitextSerializer.js
Expand Up @@ -2890,12 +2890,7 @@ WSP.skipOverEncapsulatedContent = function(node) {
return node;
}

node = node.nextSibling;
while (node && DU.isElt(node) && about === node.getAttribute('about')) {
node = node.nextSibling;
}

return node;
return DU.getAboutSiblings(node, about).last().nextSibling;
};

/**
Expand Down

0 comments on commit 65bd7ae

Please sign in to comment.