Subramanya Sastry has uploaded a new change for review.
https://gerrit.wikimedia.org/r/89227
Change subject: Bug fix in aboutId assignment during DOM Fragment unpacking
......................................................................
Bug fix in aboutId assignment during DOM Fragment unpacking
* Page native dom-fragment content that is part of an encapsulation
wrapper (say, transclusions) will have to assigned the about id
of the dom-fragment wrapper.
* Added code to recognize when a node is an encapsulation wrapper
node -- also renamed older badly named isEncapsulatedElt helper
to isFirstEncapsulationWrapperNode.
* As far as foreign content goes, <figure> captions that get reused
can have multiple independent encapsulated objects whose about ids
will all have to be renamed. This requires an about-id map to
correctly reassign about ids. Added this documentation in the
code itself.
* No change in parser tests.
TODO: We need a parser test with extensions in them.
{{echo|<div>foo</div> <math>1+2</math> <div>bar</div>}} would
not RT properly before this patch because the inner <math> node
did not have the about-id as the surrounding <div>s. This patch
fixes this.
This should also fix a lot of the <timeline> regressions seen
in latest RT testing.
Change-Id: I595e9f46d8ce789a05e4b7dd8b58e443e4c02f98
---
M js/lib/dom.cleanup.js
M js/lib/dom.minimizeTags.js
M js/lib/dom.minimizeWTQuoteTags.js
M js/lib/dom.t.TableFixups.js
M js/lib/dom.t.unpackDOMFragments.js
M js/lib/ext.core.TemplateHandler.js
M js/lib/mediawiki.DOMUtils.js
M js/lib/mediawiki.WikitextSerializer.js
8 files changed, 75 insertions(+), 24 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Parsoid
refs/changes/27/89227/1
diff --git a/js/lib/dom.cleanup.js b/js/lib/dom.cleanup.js
index b51ec2d..e023b31 100644
--- a/js/lib/dom.cleanup.js
+++ b/js/lib/dom.cleanup.js
@@ -74,7 +74,7 @@
//
// But, do not zero it out if the node has template
encapsulation
// information. That will be disastrous (see bug
52638, 52488).
- if (dp.fostered && dp.dsr &&
!DU.isEncapsulatedElt(node)) {
+ if (dp.fostered && dp.dsr &&
!DU.isFirstEncapsulationWrapperNode(node)) {
dp.dsr[0] = dp.dsr[1];
}
}
diff --git a/js/lib/dom.minimizeTags.js b/js/lib/dom.minimizeTags.js
index 2d61054..ca9963f 100644
--- a/js/lib/dom.minimizeTags.js
+++ b/js/lib/dom.minimizeTags.js
@@ -55,7 +55,7 @@
var P = [];
for (var i = 0; i < n; i++) {
var s = children[i];
- if (DU.isElt(s) && DU.isEncapsulatedElt(s)) {
+ if (DU.isFirstEncapsulationWrapperNode(s)) {
// Dont descend into template generated content
P.push({path: [], orig_parent: node, children:
[s]});
} else if (DU.isElt(s)) {
@@ -100,7 +100,7 @@
// Do not cross into templates and block tags
while (DU.isElt(node) &&
!DU.isBlockTag(node.nodeName) &&
- !DU.isEncapsulatedElt(node))
+ !DU.isFirstEncapsulationWrapperNode(node))
{
path.push(node);
children = node.childNodes;
diff --git a/js/lib/dom.minimizeWTQuoteTags.js
b/js/lib/dom.minimizeWTQuoteTags.js
index f35f37a..a69a05f 100644
--- a/js/lib/dom.minimizeWTQuoteTags.js
+++ b/js/lib/dom.minimizeWTQuoteTags.js
@@ -61,7 +61,7 @@
* ==> <i>A<b>XY</b>Z</i>
*/
function minimizeTags(node, rewriteablePair, recurse) {
- if (DU.isEncapsulatedElt(node) || !node.firstChild) {
+ if (DU.isFirstEncapsulationWrapperNode(node) || !node.firstChild) {
return;
}
@@ -86,7 +86,10 @@
// If 'a' and 'b' make a rewriteable tag-pair and neither of
them
// is an encapsulated element, we are good to go!
- if (rewriteablePair(a, b) && !DU.isEncapsulatedElt(a) &&
!DU.isEncapsulatedElt(b)) {
+ if (rewriteablePair(a, b) &&
+ !DU.isFirstEncapsulationWrapperNode(a) &&
+ !DU.isFirstEncapsulationWrapperNode(b))
+ {
if (mergable(a, b)) {
a = merge(a, b);
// the new a's children have new siblings. so
let's look
diff --git a/js/lib/dom.t.TableFixups.js b/js/lib/dom.t.TableFixups.js
index ff08009..22b2021 100644
--- a/js/lib/dom.t.TableFixups.js
+++ b/js/lib/dom.t.TableFixups.js
@@ -49,7 +49,7 @@
!DU.isLiteralHTMLNode(nextNode) &&
DU.nodeEssentiallyEmpty(node) &&
(// FIXME: will not be set for nested templates
- DU.isEncapsulatedElt(nextNode) ||
+ DU.isFirstEncapsulationWrapperNode(nextNode) ||
// Hacky work-around for nested templates
/^{{.*?}}$/.test(nextNode.data.parsoid.src))
)
@@ -92,7 +92,7 @@
TableFixups.prototype.hasOnlyOneTransclusionChild = function (node) {
var n = 0;
node.childNodes.forEach(function(n) {
- if(DU.isElt(n) && DU.isEncapsulatedElt(n)) {
+ if (DU.isFirstEncapsulationWrapperNode(n)) {
n++;
}
if (n > 1) {
diff --git a/js/lib/dom.t.unpackDOMFragments.js
b/js/lib/dom.t.unpackDOMFragments.js
index 89b1810..a3aed49 100644
--- a/js/lib/dom.t.unpackDOMFragments.js
+++ b/js/lib/dom.t.unpackDOMFragments.js
@@ -139,28 +139,40 @@
}
var isForeignContent =
node.data.parsoid.tmp.isForeignContent,
- aboutIdMap = {},
+ isEncapsulatedContent =
DU.isEncapsulationWrapper(node),
+ aboutIdMap = new Map(),
n = dummyNode.firstChild;
while (n) {
- var next = n.nextSibling,
- nAbout = n.getAttribute("about");
+ var next = n.nextSibling;
- if (isForeignContent && nAbout) {
+ if (isEncapsulatedContent) {
+ // Set about-id always to ensure the
unwrapped node
+ // is recognized as encapsulated
content as well.
+ n.setAttribute("about", about);
+ } else if (isForeignContent) {
// Replace old about-id with new
about-id that is
- // unique to the global page
environment object
- var newAbout = aboutIdMap[nAbout];
- if (!newAbout) {
- newAbout = env.newAboutId();
- aboutIdMap[nAbout] = newAbout;
+ // unique to the global page
environment object.
+ //
+ // <figure>s are reused from cache.
Note that figure captions
+ // can contain multiple independent
transclusions. Each one
+ // of those individual transclusions
should get a new unique
+ // about id. Hence a need for an
aboutIdMap.
+ var nAbout = n.getAttribute("about");
+ if (nAbout) {
+ var newAbout =
aboutIdMap.get(nAbout);
+ if (!newAbout) {
+ newAbout =
env.newAboutId();
+ aboutIdMap.set(nAbout,
newAbout);
+ }
+ n.setAttribute("about",
newAbout);
}
- n.setAttribute("about", newAbout);
} else {
// Discard unnecessary span wrappers.
//
// If the node has an about-id on it,
it is part of
// transclusion or other generated
content and is required.
DU.loadDataParsoid(n);
- if (n.data.parsoid.tmp.wrapper &&
!nAbout) {
+ if (n.data.parsoid.tmp.wrapper &&
!n.getAttribute("about")) {
DU.migrateChildren(n,
n.parentNode, n);
DU.deleteNode(n);
}
diff --git a/js/lib/ext.core.TemplateHandler.js
b/js/lib/ext.core.TemplateHandler.js
index 7fa6448..6b06e05 100644
--- a/js/lib/ext.core.TemplateHandler.js
+++ b/js/lib/ext.core.TemplateHandler.js
@@ -108,7 +108,7 @@
// cache hit: reuse the expansion DOM
//console.log('cache hit for',
JSON.stringify(text.substr(0, 50)));
var expansion = env.transclusionCache[text],
- opts = {isForeignContent: true },
+ opts = { isForeignContent: true },
toks = DU.encapsulateExpansionHTML(env,
token, expansion, opts);
cb({ tokens: toks });
diff --git a/js/lib/mediawiki.DOMUtils.js b/js/lib/mediawiki.DOMUtils.js
index 599afc3..fb58fd4 100644
--- a/js/lib/mediawiki.DOMUtils.js
+++ b/js/lib/mediawiki.DOMUtils.js
@@ -636,10 +636,6 @@
}
},
- isEncapsulatedElt: function(node) {
- return this.isElt(node) &&
(/(?:^|\s)mw:(?:Transclusion(?=$|\s)|Param(?=$|\s)|Extension\/[^\s]+)/).test(node.getAttribute('typeof'));
- },
-
/**
* Check if node is an ELEMENT node belongs to a template/extension.
*
@@ -999,6 +995,46 @@
},
/**
+ * Is node the first wrapper element of encapsulated content?
+ */
+ isFirstEncapsulationWrapperNode: function(node) {
+ return this.isElt(node) &&
(/(?:^|\s)mw:(?:Transclusion(?=$|\s)|Param(?=$|\s)|Extension\/[^\s]+)/).test(node.getAttribute('typeof'));
+ },
+
+ /**
+ * Is node an encapsulation wrapper elt?
+ *
+ * All root-level nodes of generated content are considered
+ * encapsulation wrappers and share an about-id.
+ */
+ isEncapsulationWrapper: function(node) {
+ // True if it has an encapsulation type or while walking
backwards
+ // over elts with identical about ids, we run into a node with
an
+ // encapsulation type.
+ function hasRightType(n) {
+ return
(/(?:^|\s)mw:(?:Transclusion(?=$|\s)|Param(?=$|\s)|Extension\/[^\s]+)/).test(n.getAttribute("typeof"));
+ }
+
+ if (!this.isElt(node)) {
+ return false;
+ }
+
+ var about = node.getAttribute('about');
+ if (!about) {
+ return false;
+ }
+
+ while (!hasRightType(node)) {
+ node = node.previousSibling;
+ if (!node || !this.isElt(node) ||
node.getAttribute('about') !== about) {
+ return false;
+ }
+ }
+
+ return node !== null;
+ },
+
+ /**
* Gets all siblings that follow 'node' that have an 'about' as
* their about id.
*
diff --git a/js/lib/mediawiki.WikitextSerializer.js
b/js/lib/mediawiki.WikitextSerializer.js
index 8fc740d..aff76a0 100644
--- a/js/lib/mediawiki.WikitextSerializer.js
+++ b/js/lib/mediawiki.WikitextSerializer.js
@@ -2169,7 +2169,7 @@
function wtListEOL(node, otherNode) {
if (otherNode.nodeName === 'BODY' ||
!DU.isElt(otherNode) ||
- DU.isEncapsulatedElt(otherNode))
+ DU.isFirstEncapsulationWrapperNode(otherNode))
{
return {min:0, max:2};
}
--
To view, visit https://gerrit.wikimedia.org/r/89227
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I595e9f46d8ce789a05e4b7dd8b58e443e4c02f98
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
