jenkins-bot has submitted this change and it was merged.

Change subject: Only emit shared prefix bullets for the right parents
......................................................................


Only emit shared prefix bullets for the right parents

 * In the other cases (ie. when this child doesn't match expectations),
   we'll have already emitted bullets for the parent node, which is why
   we're seeing duplicates.

 * Fixes a regression introduced in e7e1cf2dc8c5b7683b9207b268dea10d7dfd53f5

 * Identified in rt https://gerrit.wikimedia.org/r/#/c/262723/

 * A test case was added for this edge.

 * Also, add checks for literal html nodes (discussed in review) and a
   test for that as well, for good measure.

Change-Id: Iae4dc35fdfd3058ae3bb12bc950f57e416398cb2
---
M lib/html2wt/DOMHandlers.js
M tests/parserTests.txt
2 files changed, 72 insertions(+), 9 deletions(-)

Approvals:
  Subramanya Sastry: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/lib/html2wt/DOMHandlers.js b/lib/html2wt/DOMHandlers.js
index ca66f51..adb029a 100644
--- a/lib/html2wt/DOMHandlers.js
+++ b/lib/html2wt/DOMHandlers.js
@@ -251,11 +251,12 @@
        }
 }
 
+function isBuilderInsertedElt(node) {
+       var dp = DU.getDataParsoid(node);
+       return dp && dp.autoInsertedStart && dp.autoInsertedEnd;
+}
+
 function buildListHandler(firstChildNames) {
-       function isBuilderInsertedElt(node) {
-               var dp = DU.getDataParsoid(node);
-               return dp && dp.autoInsertedStart && dp.autoInsertedEnd;
-       }
        return {
                forceSOL: true,
                handle: Promise.method(function(node, state, wrapperUnmodified) 
{
@@ -272,7 +273,8 @@
                                firstChildElt = 
DU.firstNonSepChildNode(firstChildElt);
                        }
 
-                       if (!firstChildElt || !(firstChildElt.nodeName in 
firstChildNames)) {
+                       if (!firstChildElt || !(firstChildElt.nodeName in 
firstChildNames) ||
+                                       DU.isLiteralHTMLNode(firstChildElt)) {
                                state.emitChunk(getListBullets(state, node), 
node);
                        }
 
@@ -315,7 +317,8 @@
                handle: Promise.method(function(node, state, wrapperUnmodified) 
{
                        var firstChildElement = DU.firstNonSepChildNode(node);
                        var chunk = (stx === 'row') ? ':' : 
getListBullets(state, node);
-                       if (!DU.isList(firstChildElement)) {
+                       if (!DU.isList(firstChildElement) ||
+                                       
DU.isLiteralHTMLNode(firstChildElement)) {
                                state.emitChunk(chunk, node);
                        }
                        var liHandler = state.serializer.wteHandlers.liHandler
@@ -544,7 +547,8 @@
                forceSOL: true,
                handle: Promise.method(function(node, state, wrapperUnmodified) 
{
                        var firstChildElement = DU.firstNonSepChildNode(node);
-                       if (!DU.isList(firstChildElement)) {
+                       if (!DU.isList(firstChildElement) ||
+                                       
DU.isLiteralHTMLNode(firstChildElement)) {
                                state.emitChunk(getListBullets(state, node), 
node);
                        }
                        var liHandler = state.serializer.wteHandlers.liHandler
@@ -578,7 +582,8 @@
                forceSOL: true,
                handle: Promise.method(function(node, state, wrapperUnmodified) 
{
                        var firstChildElement = DU.firstNonSepChildNode(node);
-                       if (!DU.isList(firstChildElement)) {
+                       if (!DU.isList(firstChildElement) ||
+                                       
DU.isLiteralHTMLNode(firstChildElement)) {
                                state.emitChunk(getListBullets(state, node), 
node);
                        }
                        var liHandler = state.serializer.wteHandlers.liHandler
@@ -1287,9 +1292,33 @@
        },
 });
 
+var parentMap = {
+       LI: { UL: 1, OL: 1},
+       DT: { DL: 1 },
+       DD: { DL: 1 },
+};
+
+function parentBulletsHaveBeenEmitted(node) {
+       if (DU.isLiteralHTMLNode(node)) {
+               return true;
+       } else if (DU.isList(node)) {
+               return !DU.isListItem(node.parentNode);
+       } else {
+               console.assert(DU.isListItem(node));
+               var parentNode = node.parentNode;
+               // Skip builder-inserted wrappers
+               while (isBuilderInsertedElt(parentNode)) {
+                       parentNode = parentNode.parentNode;
+               }
+               return !(parentNode.nodeName in parentMap[node.nodeName]);
+       }
+}
+
 function handleListPrefix(node, state) {
        var bullets = '';
-       if (DU.isListOrListItem(node) && !DU.previousNonSepSibling(node) &&
+       if (DU.isListOrListItem(node) &&
+                       !parentBulletsHaveBeenEmitted(node) &&
+                       !DU.previousNonSepSibling(node) &&  // Maybe consider 
parentNode.
                        isTplListWithoutSharedPrefix(node) &&
                        // Nothing to do for definition list rows,
                        // since we're emitting for the parent node.
diff --git a/tests/parserTests.txt b/tests/parserTests.txt
index 64f9880..bcb80fa 100644
--- a/tests/parserTests.txt
+++ b/tests/parserTests.txt
@@ -8784,6 +8784,40 @@
 !! end
 
 !! test
+1. Nested mixed wikitext and html list
+!! wikitext
+* hi
+* <ul><li>ho</li></ul>
+* hi
+** ho
+!! html/php
+<ul><li> hi</li>
+<li> <ul><li>ho</li></ul></li>
+<li> hi
+<ul><li> ho</li></ul></li></ul>
+
+!! html/parsoid
+<ul><li> hi</li>
+<li> <ul data-parsoid='{"stx":"html"}'><li 
data-parsoid='{"stx":"html"}'>ho</li></ul></li>
+<li> hi
+<ul><li> ho</li></ul></li></ul>
+!! end
+
+!! test
+2. Nested mixed wikitext and html list (incompatible)
+!! wikitext
+; hi
+: {{echo|<li>ho</li>}}
+!! html/php
+<dl><dt> hi</dt>
+<dd> <li>ho</li></dd></dl>
+
+!! html/parsoid
+<dl><dt> hi</dt>
+<dd> <li about="#mwt1" typeof="mw:Transclusion" 
data-parsoid='{"stx":"html","pi":[[{"k":"1","spc":["","","",""]}]]}' 
data-mw='{"parts":[{"template":{"target":{"wt":"echo","href":"./Template:Echo"},"params":{"1":{"wt":"&lt;li>ho&lt;/li>"}},"i":0}}]}'>ho</li></dd></dl>
+!! end
+
+!! test
 Nested lists 1
 !! wikitext
 *foo

-- 
To view, visit https://gerrit.wikimedia.org/r/263009
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Iae4dc35fdfd3058ae3bb12bc950f57e416398cb2
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/services/parsoid
Gerrit-Branch: master
Gerrit-Owner: Arlolra <[email protected]>
Gerrit-Reviewer: Arlolra <[email protected]>
Gerrit-Reviewer: Cscott <[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

Reply via email to