jenkins-bot has submitted this change and it was merged.
Change subject: Further fixes to DOM Fragment encapsulation + unpacking
......................................................................
Further fixes to DOM Fragment encapsulation + unpacking
* Fixed figure expansion reuse to go through the same helper
function (encapsulateExpansionHTML) like all other code.
* Added isForeignContent flag for reused expanses (transclusions
and images). This is also used to discard excess span wrappers
on non-foreign content dom fragments, and fix about-ids on
foreign content dom fragments.
* These fixes now eliminate about-id manipulation during fragment
unpacking. This fixes regressions on fr:Vasco_Evtimov and other
pages that showed regressions in RT testing.
* Fixed DOMFragmentBuilder to use the refactored helper in
Util.js for processing content in a sub-pipeline.
* Added parser test to capture the last scenario.
Change-Id: I348f80817dfcd60768171b98d698a0733d4e6e79
---
M js/lib/dom.t.unpackDOMFragments.js
M js/lib/ext.core.DOMFragmentBuilder.js
M js/lib/ext.core.LinkHandler.js
M js/lib/ext.core.TemplateHandler.js
M js/lib/mediawiki.DOMUtils.js
M js/tests/parserTests.txt
6 files changed, 98 insertions(+), 66 deletions(-)
Approvals:
GWicke: Looks good to me, approved
jenkins-bot: Verified
diff --git a/js/lib/dom.t.unpackDOMFragments.js
b/js/lib/dom.t.unpackDOMFragments.js
index 7b1c604..a213d1f 100644
--- a/js/lib/dom.t.unpackDOMFragments.js
+++ b/js/lib/dom.t.unpackDOMFragments.js
@@ -107,8 +107,7 @@
DU.deleteNode(sibling);
}
- var contentNode = dummyNode.firstChild,
- haveOrigType =
Util.hasParsoidTypeOf(contentNode.getAttribute("typeof"));
+ var contentNode = dummyNode.firstChild;
// Update DSR
//
@@ -139,23 +138,32 @@
}
- // Set about and drop wrapper-spans
- var n = dummyNode.firstChild;
+ var isForeignContent =
node.data.parsoid.tmp.isForeignContent,
+ aboutIdMap = {},
+ n = dummyNode.firstChild;
while (n) {
- var next = n.nextSibling;
+ var next = n.nextSibling,
+ nAbout = n.getAttribute("about");
- // Transfer the about attribute so that it is
still unique in the page
- if (haveOrigType && about !== null) {
- n.setAttribute('about', about);
- }
-
- // Discard unnecessary span wrappers
- DU.loadDataParsoid(n);
- if (n.data.parsoid.tmp.wrapper &&
- !haveOrigType &&
!n.getAttribute("typeof"))
- {
- DU.migrateChildren(n, n.parentNode, n);
- DU.deleteNode(n);
+ if (isForeignContent && nAbout) {
+ // 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;
+ }
+ 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) {
+ DU.migrateChildren(n,
n.parentNode, n);
+ DU.deleteNode(n);
+ }
}
n = next;
diff --git a/js/lib/ext.core.DOMFragmentBuilder.js
b/js/lib/ext.core.DOMFragmentBuilder.js
index 167073d..3d77c34 100644
--- a/js/lib/ext.core.DOMFragmentBuilder.js
+++ b/js/lib/ext.core.DOMFragmentBuilder.js
@@ -68,20 +68,26 @@
// Source offsets of content
var srcOffsets = scopeToken.getAttribute("srcOffsets");
- var pipeline =
this.manager.pipeFactory.getPipeline("tokens/x-mediawiki/expanded", {
- inBlockToken: true,
- noPre: scopeToken.getAttribute('noPre'),
- // Without source offsets for the content, it is not
possible to compute DSR
- // and template wrapping in content. So, users of
mw:dom-fragment-token should
- // always set offsets on content that comes from the
top-level document.
- wrapTemplates: !!srcOffsets
- });
-
- if (srcOffsets) {
- pipeline.setSourceOffsets(srcOffsets[0], srcOffsets[1]);
- }
- pipeline.addListener('document',
this.wrapDOMFragment.bind(this, cb, scopeToken));
- pipeline.process(content.concat([new EOFTk()]));
+ // Process tokens
+ Util.processContentInPipeline(
+ this.manager,
+ // Append EOF
+ content.concat([new EOFTk()]),
+ {
+ pipelineType: "tokens/x-mediawiki/expanded",
+ pipelineOpts: {
+ inBlockToken: true,
+ noPre: scopeToken.getAttribute('noPre'),
+ // Without source offsets for the
content, it isn't possible to
+ // compute DSR and template wrapping in
content. So, users of
+ // mw:dom-fragment-token should always
set offsets on content
+ // that comes from the top-level
document.
+ wrapTemplates: !!srcOffsets
+ },
+ srcOffsets: srcOffsets,
+ documentCB: this.wrapDOMFragment.bind(this, cb,
scopeToken)
+ }
+ );
}
};
diff --git a/js/lib/ext.core.LinkHandler.js b/js/lib/ext.core.LinkHandler.js
index fe639b4..f7d9e42 100644
--- a/js/lib/ext.core.LinkHandler.js
+++ b/js/lib/ext.core.LinkHandler.js
@@ -987,21 +987,13 @@
// First check if we have a cached copy of this image expansion, and
// avoid any further processing if we have a cache hit.
- var cachedFile = this.manager.env.fileCache[token.dataAttribs.src];
+ var env = this.manager.env,
+ cachedFile = env.fileCache[token.dataAttribs.src];
if (cachedFile) {
- // Use the cached result.
- // mw:DOMFragment wrapping is simplified as we know that we are
- // dealing with a single subtree rooted either at a figure or a
span.
- var wrapperTokens = DU.getWrapperTokens(cachedFile.nodes),
+ var opts = { isForeignContent: true, noAboutId: true },
+ wrapperTokens = DU.encapsulateExpansionHTML(env, token,
cachedFile, opts),
firstWrapperToken = wrapperTokens[0];
- DU.addTypeOf(firstWrapperToken, 'mw:DOMFragment');
- firstWrapperToken.dataAttribs.html = cachedFile.html;
- // Transfer tsr to the first token
- firstWrapperToken.dataAttribs.tsr = token.dataAttribs.tsr;
-
- // SSS FIXME: This could be deleted
- //
// Capture the delta between the old/new wikitext start posn.
// 'tsr' values are stripped in the original DOM and won't be
// present. Since dsr[0] is identical to tsr[0] in this case,
@@ -1016,10 +1008,9 @@
return;
}
- var env = this.manager.env,
- // distinguish media types
- // if image: parse options
- content = buildLinkAttrs(token.attribs, true, null, null
).contentKVs;
+ // distinguish media types
+ // if image: parse options
+ var content = buildLinkAttrs(token.attribs, true, null, null
).contentKVs;
// extract options
// TODO gwicke: abstract out!
diff --git a/js/lib/ext.core.TemplateHandler.js
b/js/lib/ext.core.TemplateHandler.js
index 0205248..7fa6448 100644
--- a/js/lib/ext.core.TemplateHandler.js
+++ b/js/lib/ext.core.TemplateHandler.js
@@ -108,7 +108,8 @@
// cache hit: reuse the expansion DOM
//console.log('cache hit for',
JSON.stringify(text.substr(0, 50)));
var expansion = env.transclusionCache[text],
- toks = DU.encapsulateExpansionHTML(env,
token, expansion);
+ opts = {isForeignContent: true },
+ toks = DU.encapsulateExpansionHTML(env,
token, expansion, opts);
cb({ tokens: toks });
} else {
diff --git a/js/lib/mediawiki.DOMUtils.js b/js/lib/mediawiki.DOMUtils.js
index 5c9f7ae..0c102c0 100644
--- a/js/lib/mediawiki.DOMUtils.js
+++ b/js/lib/mediawiki.DOMUtils.js
@@ -1091,10 +1091,7 @@
function doExtractExpansions (node) {
- var nodes, expAccum,
- outerHTML = function (n) {
- return n.outerHTML;
- };
+ var nodes, expAccum;
while (node) {
if (node.nodeType === node.ELEMENT_NODE) {
@@ -1126,7 +1123,9 @@
if (key) {
expAccum[key] = {
nodes: nodes,
- html:
nodes.map(outerHTML).join('')
+ html:
nodes.map(function(node) {
+ return
node.outerHTML;
+ }).join('')
};
}
node = nodes.last();
@@ -1284,27 +1283,43 @@
},
- encapsulateExpansionHTML: function(env, token, expansion, aboutId) {
+ encapsulateExpansionHTML: function(env, token, expansion, opts) {
+ opts = opts || {};
+
// Get placeholder tokens to get our subdom through the token
processing
// stages. These will be finally unwrapped on the DOM.
var toks = this.getWrapperTokens(expansion.nodes),
- about = aboutId || env.newAboutId();
+ firstWrapperToken = toks[0];
+ // Add the DOMFragment type so that we get unwrapped later.
+ firstWrapperToken.setAttribute('typeof', 'mw:DOMFragment');
// Assign the HTML fragment to the data-parsoid.html on the
first wrapper token.
- toks[0].dataAttribs.html = expansion.html;
- // Add the DOMFragment type so that we get unwrapped later
- toks[0].setAttribute('typeof', 'mw:DOMFragment');
+ firstWrapperToken.dataAttribs.html = expansion.html;
- // Add the about to all wrapper tokens
- toks.forEach(function(tok) {
- tok.setAttribute('about', about);
- });
+ // Set foreign content flag.
+ if (opts.isForeignContent) {
+ if (!firstWrapperToken.dataAttribs.tmp) {
+ firstWrapperToken.dataAttribs.tmp = {};
+ }
+ firstWrapperToken.dataAttribs.tmp.isForeignContent =
true;
+ }
- // Transfer the tsr. The first token gets the full width, the
following
- // tokens zero width.
+ // Add about to all wrapper tokens, if necessary.
+ var about = opts.aboutId;
+ if (!about && !opts.noAboutId) {
+ about = env.newAboutId();
+ }
+ if (about) {
+ toks.forEach(function(tok) {
+ tok.setAttribute('about', about);
+ });
+ }
+
+ // Transfer the tsr.
+ // The first token gets the full width, the following tokens
zero width.
var tokenTsr = token.dataAttribs ? token.dataAttribs.tsr : null;
if (tokenTsr) {
- toks[0].dataAttribs.tsr = tokenTsr;
+ firstWrapperToken.dataAttribs.tsr = tokenTsr;
var endTsr = [tokenTsr[1],tokenTsr[1]];
for (var i = 1; i < toks.length; i++) {
toks[i].dataAttribs.tsr = endTsr;
@@ -1346,7 +1361,8 @@
nodes: nodes,
html: nodes.map(function(n) { return
n.outerHTML; }).join('')
},
- aboutId);
+ { aboutId: aboutId }
+ );
},
/**
diff --git a/js/tests/parserTests.txt b/js/tests/parserTests.txt
index a6dbcc2..abcfefc 100644
--- a/js/tests/parserTests.txt
+++ b/js/tests/parserTests.txt
@@ -5096,6 +5096,16 @@
</p>
!! end
+!! test
+Parsoid: Scoped parsing should handle mixed transclusions and plain text
+!! options
+parsoid
+!! input
+[[Foo|{{echo|a}} b {{echo|c}}]]
+!! result
+<p data-parsoid='{"dsr":[0,20,0,0]}'><a rel="mw:WikiLink" href="Foo"><span
about="#mwt2" typeof="mw:Transclusion"
data-mw='{"parts":[{"template":{"target":{"wt":"echo","href":"./Template:Echo"},"params":{"1":{"wt":"a"}},"i":0}}]}'>a</span>
b <span about="#mwt3" typeof="mw:Transclusion"
data-mw='{"parts":[{"template":{"target":{"wt":"echo","href":"./Template:Echo"},"params":{"1":{"wt":"c"}},"i":0}}]}'>c</span></a></p>
+!! end
+
###
### Interwiki links (see maintenance/interwiki.sql)
###
--
To view, visit https://gerrit.wikimedia.org/r/88521
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I348f80817dfcd60768171b98d698a0733d4e6e79
Gerrit-PatchSet: 8
Gerrit-Project: mediawiki/extensions/Parsoid
Gerrit-Branch: master
Gerrit-Owner: Subramanya Sastry <[email protected]>
Gerrit-Reviewer: GWicke <[email protected]>
Gerrit-Reviewer: Subramanya Sastry <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits