jenkins-bot has submitted this change and it was merged.
Change subject: Clean up some code in template wrapping + edge case bug fixes
......................................................................
Clean up some code in template wrapping + edge case bug fixes
* Replaced tcStart use with range.start to eliminate confusion
and constant syncing of the two.
* Got rid of the updateDP flag and update DSR at the places
where the updates are required. This eliminates potential
confusion around the conditions that triggered this changing
when range.start changes.
* In the process of cleanup, couple potential edge case buggy
scenarios eliminated.
Change-Id: Ia7d70baf41edfebd82f14b7359dc4043f063b90a
---
M lib/wt2html/pp/wrapTemplates.js
1 file changed, 27 insertions(+), 44 deletions(-)
Approvals:
Arlolra: Looks good to me, approved
jenkins-bot: Verified
diff --git a/lib/wt2html/pp/wrapTemplates.js b/lib/wt2html/pp/wrapTemplates.js
index 1565191..c1357ef 100644
--- a/lib/wt2html/pp/wrapTemplates.js
+++ b/lib/wt2html/pp/wrapTemplates.js
@@ -35,12 +35,12 @@
var dumpDOM = require('./dumper.js').dumpDOM;
-function expandRangeToAvoidSpanWrapping(range, knownTextContent) {
+function expandRangeToAvoidSpanWrapping(range, startsWithText) {
// SSS FIXME: Later on, if safe, we could consider expanding the
// range unconditionally rather than only if a span is required.
- var mightAddSpan = knownTextContent;
- if (knownTextContent === undefined) {
+ var mightAddSpan = startsWithText;
+ if (startsWithText === undefined) {
var n = range.start;
if (DU.isTplMarkerMeta(n)) {
n = n.nextSibling;
@@ -55,7 +55,7 @@
// where the the entire template content is contained in a
paragraph.
var contentParent = range.start.parentNode;
expandable = true
- && DU.hasNodeName(contentParent, 'p')
+ && contentParent.nodeName === 'P'
&& !DU.isLiteralHTMLNode(contentParent)
&& contentParent.firstChild === range.startElem
&& contentParent.lastChild === range.endElem
@@ -166,21 +166,19 @@
// Handle unwrappable content in fosterable positions
// and expand template range, if required.
- var updateDP = false;
- var tcStart = range.start;
- if (DU.isFosterablePosition(tcStart) && (
- !DU.isElt(tcStart) ||
+ if (DU.isFosterablePosition(range.start) && (
+ !DU.isElt(range.start) ||
// NOTE: These template marker meta tags are translated from
comments
// *after* the DOM has been built which is why they can show up
in
// fosterable positions in the DOM.
- (DU.isTplMarkerMeta(tcStart) &&
DU.isTplMarkerMeta(tcStart.nextSibling)) ||
- (DU.isTplMarkerMeta(tcStart) &&
!DU.isElt(tcStart.nextSibling)))) {
- var tcStartParent = range.start.parentNode;
+ (DU.isTplMarkerMeta(range.start) &&
DU.isTplMarkerMeta(range.start.nextSibling)) ||
+ (DU.isTplMarkerMeta(range.start) &&
!DU.isElt(range.start.nextSibling)))) {
+ var rangeStartParent = range.start.parentNode;
// 1. If we are in a table in a foster-element position, then
all non-element
// nodes will be white-space and comments. Skip over all of
them and find
// the first table content node
- var newStart = tcStart;
+ var newStart = range.start;
while (newStart && !DU.isElt(newStart)) {
newStart = newStart.nextSibling;
}
@@ -190,7 +188,7 @@
// other (th/td/caption) can change display semantics.
if (newStart && newStart.nodeName in {TBODY: 1, TR: 1}) {
var insertPosition = newStart.firstChild;
- var n = tcStart;
+ var n = range.start;
while (n !== newStart) {
var next = n.nextSibling;
newStart.insertBefore(n, insertPosition);
@@ -198,54 +196,39 @@
}
range.start = newStart;
// Update dsr to point to original start
- updateDP = true;
+ updateDSRForFirstTplNode(range.start, startElem);
} else {
- range.start = tcStartParent;
- range.end = tcStartParent;
-
- // Dont update dsr to original start
- // since we've encapsulated a wider DOM range
- updateDP = false;
+ range.start = rangeStartParent;
+ range.end = rangeStartParent;
}
}
// Ensure range.start is an element node since we want to
// add/update the data-parsoid attribute to it.
- tcStart = range.start;
- if (!DU.isElt(tcStart)) {
- var skipSpan = false;
- updateDP = true;
- if (expandRangeToAvoidSpanWrapping(range, true)) {
- skipSpan = true;
- }
- if (!skipSpan) {
- // wrap tcStart in a span.
- var span = doc.createElement('span');
- tcStart.parentNode.insertBefore(span, tcStart);
- span.appendChild(tcStart);
- tcStart = span;
- }
- range.start = tcStart;
+ if (!DU.isElt(range.start) && !expandRangeToAvoidSpanWrapping(range,
true)) {
+ var span = doc.createElement('span');
+ range.start.parentNode.insertBefore(span, range.start);
+ span.appendChild(range.start);
+ updateDSRForFirstTplNode(span, startElem);
+ range.start = span;
}
- // If we have any fostered content, include it as well.
- if (tcStart.nodeName === 'TABLE') {
- while (DU.isElt(tcStart.previousSibling) &&
-
DU.getDataParsoid(tcStart.previousSibling).fostered) {
- range.start = tcStart = tcStart.previousSibling;
+ if (range.start.nodeName === 'TABLE') {
+ // If we have any fostered content, include it as well.
+ while (DU.isElt(range.start.previousSibling) &&
+
DU.getDataParsoid(range.start.previousSibling).fostered) {
+ range.start = range.start.previousSibling;
}
}
- if (updateDP) {
- updateDSRForFirstTplNode(tcStart, startElem);
- } else if (tcStart === startElem && DU.isElt(tcStart.nextSibling)) {
+ if (range.start === startElem && DU.isElt(range.start.nextSibling)) {
// HACK!
// The strip-double-tds pass has a HACK that requires DSR and
src
// information being set on this element node. So, this HACK
here
// is supporting that HACK there.
//
// (The parser test for T52603 will fail without this fix)
- updateDSRForFirstTplNode(tcStart.nextSibling, startElem);
+ updateDSRForFirstTplNode(range.start.nextSibling, startElem);
}
// Use the negative test since it doesn't mark the range as flipped
--
To view, visit https://gerrit.wikimedia.org/r/252274
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia7d70baf41edfebd82f14b7359dc4043f063b90a
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/services/parsoid
Gerrit-Branch: master
Gerrit-Owner: Subramanya Sastry <ssas...@wikimedia.org>
Gerrit-Reviewer: Arlolra <abrea...@wikimedia.org>
Gerrit-Reviewer: Cscott <canan...@wikimedia.org>
Gerrit-Reviewer: Subramanya Sastry <ssas...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
