Bug 1588688 - part 7-2: Pick list element handling part of HTMLEditor::ScanForListAndTableStructure() up r=m_kato

`HTMLEditor::ScanForListAndTableStructure()` is complicated because it has
2 modes, one is for handling list element, the other is for handling table
element.  This patch splits them as 2 methods.

Differential Revision: https://phabricator.services.mozilla.com/D61975

--HG--
extra : moz-landing-system : lando

editor/libeditor/HTMLEditor.h

@@ -3946,13 +3946,24 @@ class HTMLEditor final : public TextEditor,
   int32_t DiscoverPartialListsAndTables(
       nsTArray<OwningNonNull<nsINode>>& aPasteNodes,
       nsTArray<OwningNonNull<Element>>& aListsAndTables);
-  Element* ScanForListAndTableStructure(nsINode& aNodeMaybeInListOrTable,
-                                        Element& aListOrTable);
+  Element* ScanForTableStructure(nsINode& aNodeMaybeInTable,
+                                 Element& aTableElement);
   void ReplaceOrphanedStructure(
       StartOrEnd aStartOrEnd, nsTArray<OwningNonNull<nsINode>>& aNodeArray,
       nsTArray<OwningNonNull<Element>>& aListAndTableArray,
       int32_t aHighWaterMark);
 
+  /**
+   * IsReplaceableListElement() is a helper method of
+   * ReplaceOrphanedStructure().  If aNodeMaybeInListElement is a descendant
+   * of aListElement, returns true.  Otherwise, false.
+   *
+   * @param aListElement                Must be a list element.
+   * @param aNodeMaybeInListElement     A node which may be in aListElement.
+   */
+  static bool IsReplaceableListElement(Element& aListElement,
+                                       nsINode& aNodeMaybeInListElement);
+
   /**
    * GetBetterInsertionPointFor() returns better insertion point to insert
    * aNodeToInsert.

editor/libeditor/HTMLEditorDataTransfer.cpp

@@ -2799,53 +2799,90 @@ int32_t HTMLEditor::DiscoverPartialListsAndTables(
   return ret;
 }
 
-Element* HTMLEditor::ScanForListAndTableStructure(
-    nsINode& aNodeMaybeInListOrTable, Element& aListOrTable) {
-  // Look upward from first/last paste node for a piece of this list/table
-  bool isList = HTMLEditUtils::IsList(&aListOrTable);
-  for (Element* element = aNodeMaybeInListOrTable.IsElement()
-                              ? aNodeMaybeInListOrTable.AsElement()
-                              : aNodeMaybeInListOrTable.GetParentElement();
+Element* HTMLEditor::ScanForTableStructure(nsINode& aNodeMaybeInTable,
+                                           Element& aTableElement) {
+  for (Element* element = aNodeMaybeInTable.IsElement()
+                              ? aNodeMaybeInTable.AsElement()
+                              : aNodeMaybeInTable.GetParentElement();
        element; element = element->GetParentElement()) {
-    if (!(isList && HTMLEditUtils::IsListItem(element)) &&
-        !(!isList && HTMLEditUtils::IsTableElement(element) &&
-          !element->IsHTMLElement(nsGkAtoms::table))) {
+    if (!HTMLEditUtils::IsTableElement(element) ||
+        element->IsHTMLElement(nsGkAtoms::table)) {
       continue;
     }
     Element* structureElement = element->GetParentElement();
-    if (isList) {
-      while (structureElement && !HTMLEditUtils::IsList(structureElement)) {
-        structureElement = structureElement->GetParentElement();
-      }
-    } else {
-      while (structureElement &&
-             !structureElement->IsHTMLElement(nsGkAtoms::table)) {
-        structureElement = structureElement->GetParentElement();
-      }
+    while (structureElement &&
+           !structureElement->IsHTMLElement(nsGkAtoms::table)) {
+      structureElement = structureElement->GetParentElement();
     }
-    if (structureElement == &aListOrTable) {
-      return isList ? structureElement : element;
+    if (structureElement == &aTableElement) {
+      return element;
     }
   }
   return nullptr;
 }
 
+// static
+bool HTMLEditor::IsReplaceableListElement(Element& aListElement,
+                                          nsINode& aNodeMaybeInListElement) {
+  MOZ_ASSERT(HTMLEditUtils::IsList(&aListElement));
+  // Perhaps, this is designed for climbing up the DOM tree from
+  // aNodeMaybeInListElement to aListElement and making sure that
+  // aNodeMaybeInListElement itself or its ancestor is an list item.
+  // But this looks really buggy because this loop may skip aListElement
+  // as the following NS_ASSERTION.  We should write automated tests and
+  // check right behavior.
+  for (Element* element = aNodeMaybeInListElement.IsElement()
+                              ? aNodeMaybeInListElement.AsElement()
+                              : aNodeMaybeInListElement.GetParentElement();
+       element; element = element->GetParentElement()) {
+    if (!HTMLEditUtils::IsListItem(element)) {
+      // XXX Perhaps, the original developer of this method assumed that
+      //     aListElement won't be skipped because if it's assumed, we can
+      //     stop climbing up the tree in that case.
+      NS_ASSERTION(element != &aListElement,
+                   "The list element which is looking for is ignored");
+      continue;
+    }
+    Element* listElement = nullptr;
+    for (Element* maybeListElement = element->GetParentElement();
+         maybeListElement;
+         maybeListElement = maybeListElement->GetParentElement()) {
+      if (HTMLEditUtils::IsList(maybeListElement)) {
+        listElement = maybeListElement;
+        break;
+      }
+    }
+    if (listElement == &aListElement) {
+      return true;
+    }
+    // XXX If we find another list element, why don't we keep searching
+    //     from its parent?
+  }
+  return false;
+}
+
 void HTMLEditor::ReplaceOrphanedStructure(
     StartOrEnd aStartOrEnd, nsTArray<OwningNonNull<nsINode>>& aNodeArray,
     nsTArray<OwningNonNull<Element>>& aListAndTableArray,
     int32_t aHighWaterMark) {
   MOZ_ASSERT(!aNodeArray.IsEmpty());
 
+  OwningNonNull<nsINode>& edgeNode =
+      aStartOrEnd == StartOrEnd::end ? aNodeArray.LastElement() : aNodeArray[0];
   OwningNonNull<Element> curNode = aListAndTableArray[aHighWaterMark];
 
   // Find substructure of list or table that must be included in paste.
-  Element* replaceElment = ScanForListAndTableStructure(
-      aStartOrEnd == StartOrEnd::end ? *aNodeArray.LastElement()
-                                     : *aNodeArray[0],
-      curNode);
-
-  if (!replaceElment) {
-    return;
+  Element* replaceElement;
+  if (HTMLEditUtils::IsList(curNode)) {
+    if (!HTMLEditor::IsReplaceableListElement(curNode, edgeNode)) {
+      return;
+    }
+    replaceElement = curNode;
+  } else {
+    replaceElement = ScanForTableStructure(edgeNode, curNode);
+    if (!replaceElement) {
+      return;
+    }
   }
 
   // If we found substructure, paste it instead of its descendants.
@@ -2857,8 +2894,8 @@ void HTMLEditor::ReplaceOrphanedStructure(
     uint32_t idx = aStartOrEnd == StartOrEnd::start ? (i - removedCount)
                                                     : (originalLength - i - 1);
     OwningNonNull<nsINode> endpoint = aNodeArray[idx];
-    if (endpoint == replaceElment ||
-        EditorUtils::IsDescendantOf(*endpoint, *replaceElment)) {
+    if (endpoint == replaceElement ||
+        EditorUtils::IsDescendantOf(*endpoint, *replaceElement)) {
       aNodeArray.RemoveElementAt(idx);
       removedCount++;
     }
@@ -2866,9 +2903,9 @@ void HTMLEditor::ReplaceOrphanedStructure(
 
   // Now replace the removed nodes with the structural parent
   if (aStartOrEnd == StartOrEnd::end) {
-    aNodeArray.AppendElement(*replaceElment);
+    aNodeArray.AppendElement(*replaceElement);
   } else {
-    aNodeArray.InsertElementAt(0, *replaceElment);
+    aNodeArray.InsertElementAt(0, *replaceElement);
   }
 }