Bug 1460509 - part 24: Make HTMLEditRules::PopListItem() return NS_ERROR_EDITOR_DESTROYED if it causes destroying the editor r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Mon, 14 May 2018 18:18:24 +0900
changeset 798742 55aececc29764efadc654cb547c325a1a43bbf8c
parent 798741 6c3ad9fe0dc90a239f2a322472552457fe861e1b
child 798743 a5c1b6709a9979a75a21a81cd44185314133c64e
push id110840
push usermasayuki@d-toybox.com
push dateWed, 23 May 2018 13:41:58 +0000
reviewersm_kato
bugs1460509
milestone62.0a1
Bug 1460509 - part 24: Make HTMLEditRules::PopListItem() return NS_ERROR_EDITOR_DESTROYED if it causes destroying the editor r?m_kato MozReview-Commit-ID: 2655OlPmteT
editor/libeditor/HTMLEditRules.cpp
editor/libeditor/HTMLEditRules.h
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -4133,17 +4133,19 @@ HTMLEditRules::WillRemoveList(bool aOrde
   // Only act on lists or list items in the array
   for (auto& curNode : arrayOfNodes) {
     // here's where we actually figure out what to do
     if (HTMLEditUtils::IsListItem(curNode)) {
       // unlist this listitem
       bool bOutOfList;
       do {
         rv = PopListItem(*curNode->AsContent(), &bOutOfList);
-        NS_ENSURE_SUCCESS(rv, rv);
+        if (NS_WARN_IF(NS_FAILED(rv))) {
+          return rv;
+        }
       } while (!bOutOfList); // keep popping it out until it's not in a list anymore
     } else if (HTMLEditUtils::IsList(curNode)) {
       // node is a list, move list items out
       rv = RemoveListStructure(*curNode->AsElement());
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }
     }
@@ -5014,17 +5016,19 @@ HTMLEditRules::WillOutdent(bool* aCancel
                                   getter_AddRefs(rememberedRightBQ));
           NS_ENSURE_SUCCESS(rv, rv);
           curBlockQuote = nullptr;
           firstBQChild = nullptr;
           lastBQChild = nullptr;
           curBlockQuoteIsIndentedWithCSS = false;
         }
         rv = PopListItem(*curNode->AsContent());
-        NS_ENSURE_SUCCESS(rv, rv);
+        if (NS_WARN_IF(NS_FAILED(rv))) {
+          return rv;
+        }
         continue;
       }
       // Do we have a blockquote that we are already committed to removing?
       if (curBlockQuote) {
         // If so, is this node a descendant?
         if (EditorUtils::IsDescendantOf(*curNode, *curBlockQuote)) {
           lastBQChild = curNode;
           // Then we don't need to do anything different for this node
@@ -5096,17 +5100,19 @@ HTMLEditRules::WillOutdent(bool* aCancel
           }
           // handled list item case above
         } else if (HTMLEditUtils::IsList(curNode)) {
           // node is a list, but parent is non-list: move list items out
           nsCOMPtr<nsIContent> child = curNode->GetLastChild();
           while (child) {
             if (HTMLEditUtils::IsListItem(child)) {
               rv = PopListItem(*child);
-              NS_ENSURE_SUCCESS(rv, rv);
+              if (NS_WARN_IF(NS_FAILED(rv))) {
+                return rv;
+              }
             } else if (HTMLEditUtils::IsList(child)) {
               // We have an embedded list, so move it out from under the parent
               // list. Be sure to put it after the parent list because this
               // loop iterates backwards through the parent's list of children.
               EditorRawDOMPoint afterCurrentList(curParent, offset + 1);
               rv = HTMLEditorRef().MoveNodeWithTransaction(*child,
                                                            afterCurrentList);
               if (NS_WARN_IF(NS_FAILED(rv))) {
@@ -9238,79 +9244,93 @@ HTMLEditRules::ListIsEmptyLine(nsTArray<
 
 
 nsresult
 HTMLEditRules::PopListItem(nsIContent& aListItem,
                            bool* aOutOfList)
 {
   MOZ_ASSERT(IsEditorDataAvailable());
 
-  // init out params
   if (aOutOfList) {
     *aOutOfList = false;
   }
 
-  nsCOMPtr<nsIContent> kungFuDeathGrip(&aListItem);
-  Unused << kungFuDeathGrip;
-
-
   if (NS_WARN_IF(!aListItem.GetParent()) ||
       NS_WARN_IF(!aListItem.GetParent()->GetParentNode()) ||
       !HTMLEditUtils::IsListItem(&aListItem)) {
     return NS_ERROR_FAILURE;
   }
 
   // if it's first or last list item, don't need to split the list
   // otherwise we do.
-  bool bIsFirstListItem = HTMLEditorRef().IsFirstEditableChild(&aListItem);
-  bool bIsLastListItem = HTMLEditorRef().IsLastEditableChild(&aListItem);
+  bool isFirstListItem = HTMLEditorRef().IsFirstEditableChild(&aListItem);
+  bool isLastListItem = HTMLEditorRef().IsLastEditableChild(&aListItem);
 
   nsCOMPtr<nsIContent> leftListNode = aListItem.GetParent();
-  if (!bIsFirstListItem && !bIsLastListItem) {
-    EditorDOMPoint atListItem(&aListItem);
+
+  // If it's at middle of parent list element, split the parent list element.
+  // Then, aListItem becomes the first list item of the right list element.
+  nsCOMPtr<nsIContent> listItem(&aListItem);
+  if (!isFirstListItem && !isLastListItem) {
+    EditorDOMPoint atListItem(listItem);
     if (NS_WARN_IF(!atListItem.IsSet())) {
       return NS_ERROR_INVALID_ARG;
     }
     MOZ_ASSERT(atListItem.IsSetAndValid());
-
-    // split the list
     ErrorResult error;
     leftListNode = HTMLEditorRef().SplitNodeWithTransaction(atListItem, error);
+    if (NS_WARN_IF(!CanHandleEditAction())) {
+      error.SuppressException();
+      return NS_ERROR_EDITOR_DESTROYED;
+    }
     if (NS_WARN_IF(error.Failed())) {
       return error.StealNSResult();
     }
   }
 
   // In most cases, insert the list item into the new left list node..
   EditorDOMPoint pointToInsertListItem(leftListNode);
   if (NS_WARN_IF(!pointToInsertListItem.IsSet())) {
     return NS_ERROR_FAILURE;
   }
   MOZ_ASSERT(pointToInsertListItem.IsSetAndValid());
 
   // But when the list item was the first child of the right list, it should
-  // be inserted into the next sibling of the list.  This allows user to hit
+  // be inserted between the both list elements.  This allows user to hit
   // Enter twice at a list item breaks the parent list node.
-  if (!bIsFirstListItem) {
+  if (!isFirstListItem) {
     DebugOnly<bool> advanced = pointToInsertListItem.AdvanceOffset();
     NS_WARNING_ASSERTION(advanced,
       "Failed to advance offset to right list node");
   }
 
   nsresult rv =
-    HTMLEditorRef().MoveNodeWithTransaction(aListItem, pointToInsertListItem);
+    HTMLEditorRef().MoveNodeWithTransaction(*listItem, pointToInsertListItem);
+  if (NS_WARN_IF(!CanHandleEditAction())) {
+    return NS_ERROR_EDITOR_DESTROYED;
+  }
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   // unwrap list item contents if they are no longer in a list
+  // XXX If the parent list element is a child of another list element
+  //     (although invalid tree), the list item element won't be unwrapped.
+  //     That makes the parent ancestor element tree valid, but might be
+  //     unexpected result.
+  // XXX If aListItem is <dl> or <dd> and current parent is <ul> or <ol>,
+  //     the list items won't be unwrapped.  If aListItem is <li> and its
+  //     current parent is <dl>, there is same issue.
   if (!HTMLEditUtils::IsList(pointToInsertListItem.GetContainer()) &&
-      HTMLEditUtils::IsListItem(&aListItem)) {
+      HTMLEditUtils::IsListItem(listItem)) {
     rv = HTMLEditorRef().RemoveBlockContainerWithTransaction(
-                           *aListItem.AsElement());
+                           *listItem->AsElement());
+    if (NS_WARN_IF(!CanHandleEditAction())) {
+      return NS_ERROR_EDITOR_DESTROYED;
+    }
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
     if (aOutOfList) {
       *aOutOfList = true;
     }
   }
   return NS_OK;
--- a/editor/libeditor/HTMLEditRules.h
+++ b/editor/libeditor/HTMLEditRules.h
@@ -552,17 +552,34 @@ protected:
    *                    aLeftNode.
    * @return            The point at the first child of aRightNode.
    */
   EditorDOMPoint
   JoinNearestEditableNodesWithTransaction(nsIContent& aLeftNode,
                                           nsIContent& aRightNode);
 
   Element* GetTopEnclosingMailCite(nsINode& aNode);
-  nsresult PopListItem(nsIContent& aListItem, bool* aOutOfList = nullptr);
+
+  /**
+   * PopListItem() tries to move aListItem outside its parent.  If it's
+   * in a middle of a list element, the parent list element is split before
+   * aListItem.  Then, moves aListItem to before its parent list element.
+   * I.e., moves aListItem between the 2 list elements if original parent
+   * was split.  Then, if new parent is not a list element, the list item
+   * element is removed and its contents are moved to where the list item
+   * element was.
+   *
+   * @param aListItem           Should be a <li>, <dt> or <dd> element.
+   *                            If it's not so, returns NS_ERROR_FAILURE.
+   * @param aOutOfList          Returns true if the list item element is
+   *                            removed (i.e., unwrapped contents of
+   *                            aListItem).  Otherwise, false.
+   */
+  MOZ_MUST_USE nsresult
+  PopListItem(nsIContent& aListItem, bool* aOutOfList = nullptr);
 
   /**
    * RemoveListStructure() destroys the list structure of aListElement.
    * If aListElement has <li>, <dl> or <dt> as a child, the element is removed
    * but its descendants are moved to where the list item element was.
    * If aListElement has another <ul>, <ol> or <dl> as a child, this method
    * is called recursively.
    * If aListElement has other nodes as its child, they are just removed.