Bug 1316302 part.3 Create EditActionResult class for making the methods which return nsresult, handled and canceled with out params r?smaug draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 18 Nov 2016 17:59:23 +0900
changeset 441065 4df56c556d628ce26b396a451e53fc65006d8b26
parent 441064 da758179414a8fd40bd9b815f71140e545c75b1d
child 441066 5c93e5bc78dda98c743168ab8d61a7ddf1158a3a
push id36347
push usermasayuki@d-toybox.com
push dateFri, 18 Nov 2016 10:04:41 +0000
reviewerssmaug
bugs1316302
milestone53.0a1
Bug 1316302 part.3 Create EditActionResult class for making the methods which return nsresult, handled and canceled with out params r?smaug In a lot of places, edit action handlers and their helper methods return nsresult and aHandled and aCanceled with out params. However, the out params cause the code complicated since: * it's not unclear if the method will overwrite aHandled and aCanceled value. * callers need to create temporary variable event if some of them are not necessary. This patch rewrites the helper methods of HTMLEditRules::WillDeleteSelection() with it. MozReview-Commit-ID: CJv75KdOdXf
editor/libeditor/EditorUtils.h
editor/libeditor/HTMLEditRules.cpp
editor/libeditor/HTMLEditRules.h
--- a/editor/libeditor/EditorUtils.h
+++ b/editor/libeditor/EditorUtils.h
@@ -26,16 +26,129 @@ class nsRange;
 namespace mozilla {
 template <class T> class OwningNonNull;
 
 namespace dom {
 class Selection;
 } // namespace dom
 
 /***************************************************************************
+ * EditActionResult is useful to return multiple results of an editor
+ * action handler without out params.
+ * Note that when you return an anonymous instance from a method, you should
+ * use EditActionIgnored(), EditActionHandled() or EditActionCanceled() for
+ * easier to read.  In other words, EditActionResult should be used when
+ * declaring return type of a method, being an argument or defined as a local
+ * variable.
+ */
+class MOZ_STACK_CLASS EditActionResult final
+{
+public:
+  bool Succeeded() const { return NS_SUCCEEDED(mRv); }
+  bool Failed() const { return NS_FAILED(mRv); }
+  nsresult Rv() const { return mRv; }
+  bool Canceled() const { return mCanceled; }
+  bool Handled() const { return mHandled; }
+
+  EditActionResult SetResult(nsresult aRv)
+  {
+    mRv = aRv;
+    return *this;
+  }
+  EditActionResult MarkAsCanceled()
+  {
+    mCanceled = true;
+    return *this;
+  }
+  EditActionResult MarkAsHandled()
+  {
+    mHandled = true;
+    return *this;
+  }
+
+  explicit EditActionResult(nsresult aRv)
+    : mRv(aRv)
+    , mCanceled(false)
+    , mHandled(false)
+  {
+  }
+
+  EditActionResult& operator|=(const EditActionResult& aOther)
+  {
+    mCanceled |= aOther.mCanceled;
+    mHandled |= aOther.mHandled;
+    // When both result are same, keep the result.
+    if (mRv == aOther.mRv) {
+      return *this;
+    }
+    // If one of the results is error, use NS_ERROR_FAILURE.
+    if (Failed() || aOther.Failed()) {
+      mRv = NS_ERROR_FAILURE;
+    } else {
+      // Otherwise, use generic success code, NS_OK.
+      mRv = NS_OK;
+    }
+    return *this;
+  }
+
+private:
+  nsresult mRv;
+  bool mCanceled;
+  bool mHandled;
+
+  EditActionResult(nsresult aRv, bool aCanceled, bool aHandled)
+    : mRv(aRv)
+    , mCanceled(aCanceled)
+    , mHandled(aHandled)
+  {
+  }
+
+  EditActionResult()
+    : mRv(NS_ERROR_NOT_INITIALIZED)
+    , mCanceled(false)
+    , mHandled(false)
+  {
+  }
+
+  friend EditActionResult EditActionIgnored(nsresult aRv);
+  friend EditActionResult EditActionHandled(nsresult aRv);
+  friend EditActionResult EditActionCanceled(nsresult aRv);
+};
+
+/***************************************************************************
+ * When an edit action handler (or its helper) does nothing,
+ * EditActionIgnored should be returned.
+ */
+inline EditActionResult
+EditActionIgnored(nsresult aRv = NS_OK)
+{
+  return EditActionResult(aRv, false, false);
+}
+
+/***************************************************************************
+ * When an edit action handler (or its helper) handled and not canceled,
+ * EditActionHandled should be returned.
+ */
+inline EditActionResult
+EditActionHandled(nsresult aRv = NS_OK)
+{
+  return EditActionResult(aRv, false, true);
+}
+
+/***************************************************************************
+ * When an edit action handler (or its helper) handled and canceled,
+ * EditActionHandled should be returned.
+ */
+inline EditActionResult
+EditActionCanceled(nsresult aRv = NS_OK)
+{
+  return EditActionResult(aRv, true, true);
+}
+
+/***************************************************************************
  * stack based helper class for batching a collection of txns inside a
  * placeholder txn.
  */
 class MOZ_RAII AutoPlaceHolderBatch
 {
 private:
   nsCOMPtr<nsIEditor> mEditor;
   MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -2167,22 +2167,23 @@ HTMLEditRules::WillDeleteSelection(Selec
       nsCOMPtr<nsINode> selPointNode = startNode;
       int32_t selPointOffset = startOffset;
       {
         NS_ENSURE_STATE(mHTMLEditor);
         AutoTrackDOMPoint tracker(mHTMLEditor->mRangeUpdater,
                                   address_of(selPointNode), &selPointOffset);
         NS_ENSURE_STATE(leftNode && leftNode->IsContent() &&
                         rightNode && rightNode->IsContent());
-        bool handled = false, canceled = false;
-        rv = TryToJoinBlocks(*leftNode->AsContent(), *rightNode->AsContent(),
-                             &canceled, &handled);
-        *aHandled |= handled;
-        *aCancel |= canceled;
-        NS_ENSURE_SUCCESS(rv, rv);
+        EditActionResult ret =
+          TryToJoinBlocks(*leftNode->AsContent(), *rightNode->AsContent());
+        *aHandled |= ret.Handled();
+        *aCancel |= ret.Canceled();
+        if (NS_WARN_IF(ret.Failed())) {
+          return ret.Rv();
+        }
       }
 
       // If TryToJoinBlocks() didn't handle it  and it's not canceled,
       // user may want to modify the start leaf node or the last leaf node
       // of the block.
       if (!*aHandled && !*aCancel && leafNode != startNode) {
         int32_t offset =
           aAction == nsIEditor::ePrevious ?
@@ -2234,25 +2235,26 @@ HTMLEditRules::WillDeleteSelection(Selec
 
       nsCOMPtr<nsINode> selPointNode = startNode;
       int32_t selPointOffset = startOffset;
       {
         NS_ENSURE_STATE(mHTMLEditor);
         AutoTrackDOMPoint tracker(mHTMLEditor->mRangeUpdater,
                                   address_of(selPointNode), &selPointOffset);
         NS_ENSURE_STATE(leftNode->IsContent() && rightNode->IsContent());
-        bool handled = false, canceled = false;
-        rv = TryToJoinBlocks(*leftNode->AsContent(), *rightNode->AsContent(),
-                             &canceled, &handled);
+        EditActionResult ret =
+          TryToJoinBlocks(*leftNode->AsContent(), *rightNode->AsContent());
         // This should claim that trying to join the block means that
         // this handles the action because the caller shouldn't do anything
         // anymore in this case.
         *aHandled = true;
-        *aCancel |= canceled;
-        NS_ENSURE_SUCCESS(rv, rv);
+        *aCancel |= ret.Canceled();
+        if (NS_WARN_IF(ret.Failed())) {
+          return ret.Rv();
+        }
       }
       aSelection->Collapse(selPointNode, selPointOffset);
       return NS_OK;
     }
   }
 
 
   // Else we have a non-collapsed selection.  First adjust the selection.
@@ -2413,21 +2415,22 @@ HTMLEditRules::WillDeleteSelection(Selec
           NS_ENSURE_STATE(mHTMLEditor);
           OwningNonNull<nsGenericDOMDataNode> dataNode =
             *static_cast<nsGenericDOMDataNode*>(endNode.get());
           rv = mHTMLEditor->DeleteText(dataNode, 0, endOffset);
           NS_ENSURE_SUCCESS(rv, rv);
         }
 
         if (join) {
-          bool handled = false, canceled = false;
-          rv = TryToJoinBlocks(*leftParent, *rightParent, &canceled, &handled);
+          EditActionResult ret = TryToJoinBlocks(*leftParent, *rightParent);
           MOZ_ASSERT(*aHandled);
-          *aCancel |= canceled;
-          NS_ENSURE_SUCCESS(rv, rv);
+          *aCancel |= ret.Canceled();
+          if (NS_WARN_IF(ret.Failed())) {
+            return ret.Rv();
+          }
         }
       }
     }
   }
 
   // We might have left only collapsed whitespace in the start/end nodes
   {
     AutoTrackDOMPoint startTracker(mHTMLEditor->mRangeUpdater,
@@ -2566,69 +2569,68 @@ HTMLEditRules::GetGoodSelPointForNode(ns
   NS_ENSURE_TRUE(mHTMLEditor, EditorDOMPoint());
   if ((!aNode.IsHTMLElement(nsGkAtoms::br) ||
        mHTMLEditor->IsVisBreak(&aNode)) && isPreviousAction) {
     ret.offset++;
   }
   return ret;
 }
 
-nsresult
+EditActionResult
 HTMLEditRules::TryToJoinBlocks(nsIContent& aLeftNode,
-                               nsIContent& aRightNode,
-                               bool* aCanceled,
-                               bool* aHandled)
-{
-  MOZ_ASSERT(aCanceled);
-  MOZ_ASSERT(aHandled);
-
-  *aCanceled = false;
-  *aHandled = false;
-
-  NS_ENSURE_STATE(mHTMLEditor);
+                               nsIContent& aRightNode)
+{
+  if (NS_WARN_IF(!mHTMLEditor)) {
+    return EditActionIgnored(NS_ERROR_UNEXPECTED);
+  }
+
   RefPtr<HTMLEditor> htmlEditor(mHTMLEditor);
 
   nsCOMPtr<Element> leftBlock = htmlEditor->GetBlock(aLeftNode);
   nsCOMPtr<Element> rightBlock = htmlEditor->GetBlock(aRightNode);
 
   // Sanity checks
-  NS_ENSURE_TRUE(leftBlock && rightBlock, NS_ERROR_NULL_POINTER);
-  NS_ENSURE_STATE(leftBlock != rightBlock);
+  if (NS_WARN_IF(!leftBlock) || NS_WARN_IF(!rightBlock)) {
+    return EditActionIgnored(NS_ERROR_NULL_POINTER);
+  }
+  if (NS_WARN_IF(leftBlock == rightBlock)) {
+    return EditActionIgnored(NS_ERROR_UNEXPECTED);
+  }
 
   if (HTMLEditUtils::IsTableElement(leftBlock) ||
       HTMLEditUtils::IsTableElement(rightBlock)) {
     // Do not try to merge table elements
-    *aCanceled = true;
-    *aHandled = true;
-    return NS_OK;
+    return EditActionCanceled();
   }
 
   // Make sure we don't try to move things into HR's, which look like blocks
   // but aren't containers
   if (leftBlock->IsHTMLElement(nsGkAtoms::hr)) {
     leftBlock = htmlEditor->GetBlockNodeParent(leftBlock);
+    if (NS_WARN_IF(!leftBlock)) {
+      return EditActionIgnored(NS_ERROR_UNEXPECTED);
+    }
   }
   if (rightBlock->IsHTMLElement(nsGkAtoms::hr)) {
     rightBlock = htmlEditor->GetBlockNodeParent(rightBlock);
-  }
-  NS_ENSURE_STATE(leftBlock && rightBlock);
+    if (NS_WARN_IF(!rightBlock)) {
+      return EditActionIgnored(NS_ERROR_UNEXPECTED);
+    }
+  }
 
   // Bail if both blocks the same
   if (leftBlock == rightBlock) {
-    *aCanceled = true;
-    *aHandled = true;
-    return NS_OK;
+    return EditActionIgnored();
   }
 
   // Joining a list item to its parent is a NOP.
   if (HTMLEditUtils::IsList(leftBlock) &&
       HTMLEditUtils::IsListItem(rightBlock) &&
       rightBlock->GetParentNode() == leftBlock) {
-    *aHandled = true;
-    return NS_OK;
+    return EditActionHandled();
   }
 
   // Special rule here: if we are trying to join list items, and they are in
   // different lists, join the lists instead.
   bool mergeLists = false;
   nsIAtom* existingList = nsGkAtoms::_empty;
   int32_t offset;
   nsCOMPtr<Element> leftList, rightList;
@@ -2652,95 +2654,119 @@ HTMLEditRules::TryToJoinBlocks(nsIConten
 
   AutoTransactionsConserveSelection dontSpazMySelection(htmlEditor);
 
   int32_t rightOffset = 0;
   int32_t leftOffset = -1;
 
   // offset below is where you find yourself in rightBlock when you traverse
   // upwards from leftBlock
+  EditActionResult ret(NS_OK);
   if (EditorUtils::IsDescendantOf(leftBlock, rightBlock, &rightOffset)) {
     // Tricky case.  Left block is inside right block.  Do ws adjustment.  This
     // just destroys non-visible ws at boundaries we will be joining.
     rightOffset++;
     nsresult rv = WSRunObject::ScrubBlockBoundary(htmlEditor,
                                                   WSRunObject::kBlockEnd,
                                                   leftBlock);
-    NS_ENSURE_SUCCESS(rv, rv);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return EditActionIgnored(rv);
+    }
 
     {
       // We can't just track rightBlock because it's an Element.
       nsCOMPtr<nsINode> trackingRightBlock(rightBlock);
       AutoTrackDOMPoint tracker(htmlEditor->mRangeUpdater,
                                 address_of(trackingRightBlock), &rightOffset);
       rv = WSRunObject::ScrubBlockBoundary(htmlEditor,
                                            WSRunObject::kAfterBlock,
                                            rightBlock, rightOffset);
-      NS_ENSURE_SUCCESS(rv, rv);
+      if (NS_WARN_IF(NS_FAILED(rv))) {
+        return EditActionIgnored(rv);
+      }
+
       if (trackingRightBlock->IsElement()) {
         rightBlock = trackingRightBlock->AsElement();
       } else {
-        NS_ENSURE_STATE(trackingRightBlock->GetParentElement());
+        if (NS_WARN_IF(!trackingRightBlock->GetParentElement())) {
+          return EditActionIgnored(NS_ERROR_UNEXPECTED);
+        }
         rightBlock = trackingRightBlock->GetParentElement();
       }
     }
     // Do br adjustment.
     nsCOMPtr<Element> brNode =
       CheckForInvisibleBR(*leftBlock, BRLocation::blockEnd);
     if (mergeLists) {
       // The idea here is to take all children in rightList that are past
       // offset, and pull them into leftlist.
       for (nsCOMPtr<nsIContent> child = rightList->GetChildAt(offset);
            child; child = rightList->GetChildAt(rightOffset)) {
         rv = htmlEditor->MoveNode(child, leftList, -1);
-        NS_ENSURE_SUCCESS(rv, rv);
+        if (NS_WARN_IF(NS_FAILED(rv))) {
+          return EditActionIgnored(rv);
+        }
       }
       // XXX Should this set to true only when above for loop moves the node?
-      *aHandled = true;
+      ret.MarkAsHandled();
     } else {
-      bool handled = false;
-      MoveBlock(*leftBlock, *rightBlock, leftOffset, rightOffset, &handled);
-      *aHandled |= handled;
+      // XXX Why do we ignore the result of MoveBlock()?
+      EditActionResult retMoveBlock =
+        MoveBlock(*leftBlock, *rightBlock, leftOffset, rightOffset);
+      if (retMoveBlock.Handled()) {
+        ret.MarkAsHandled();
+      }
     }
     if (brNode && NS_SUCCEEDED(htmlEditor->DeleteNode(brNode))) {
-      *aHandled = true;
+      ret.MarkAsHandled();
     }
   // Offset below is where you find yourself in leftBlock when you traverse
   // upwards from rightBlock
   } else if (EditorUtils::IsDescendantOf(rightBlock, leftBlock, &leftOffset)) {
     // Tricky case.  Right block is inside left block.  Do ws adjustment.  This
     // just destroys non-visible ws at boundaries we will be joining.
     nsresult rv = WSRunObject::ScrubBlockBoundary(htmlEditor,
                                                   WSRunObject::kBlockStart,
                                                   rightBlock);
-    NS_ENSURE_SUCCESS(rv, rv);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return EditActionIgnored(rv);
+    }
+
     {
       // We can't just track leftBlock because it's an Element, so track
       // something else.
       nsCOMPtr<nsINode> trackingLeftBlock(leftBlock);
       AutoTrackDOMPoint tracker(htmlEditor->mRangeUpdater,
                                 address_of(trackingLeftBlock), &leftOffset);
       rv = WSRunObject::ScrubBlockBoundary(htmlEditor,
                                            WSRunObject::kBeforeBlock,
                                            leftBlock, leftOffset);
-      NS_ENSURE_SUCCESS(rv, rv);
+      if (NS_WARN_IF(NS_FAILED(rv))) {
+        return EditActionIgnored(rv);
+      }
+
       if (trackingLeftBlock->IsElement()) {
         leftBlock = trackingLeftBlock->AsElement();
       } else {
-        NS_ENSURE_STATE(trackingLeftBlock->GetParentElement());
+        if (NS_WARN_IF(!trackingLeftBlock->GetParentElement())) {
+          return EditActionIgnored(NS_ERROR_UNEXPECTED);
+        }
         leftBlock = trackingLeftBlock->GetParentElement();
       }
     }
     // Do br adjustment.
     nsCOMPtr<Element> brNode =
       CheckForInvisibleBR(*leftBlock, BRLocation::beforeBlock, leftOffset);
     if (mergeLists) {
-      bool handled = false;
-      MoveContents(*rightList, *leftList, &leftOffset, &handled);
-      *aHandled |= handled;
+      // XXX Why do we ignore the result of MoveContents()?
+      EditActionResult retMoveContents =
+        MoveContents(*rightList, *leftList, &leftOffset);
+      if (retMoveContents.Handled()) {
+        ret.MarkAsHandled();
+      }
     } else {
       // Left block is a parent of right block, and the parent of the previous
       // visible content.  Right block is a child and contains the contents we
       // want to move.
 
       int32_t previousContentOffset;
       nsCOMPtr<nsINode> previousContentParent;
 
@@ -2775,184 +2801,194 @@ HTMLEditRules::TryToJoinBlocks(nsIConten
       nsCOMPtr<Element> editorRoot = htmlEditor->GetEditorRoot();
       if (!editorRoot || &aLeftNode != editorRoot) {
         nsCOMPtr<nsIContent> splittedPreviousContent;
         rv = htmlEditor->SplitStyleAbovePoint(
                            address_of(previousContentParent),
                            &previousContentOffset,
                            nullptr, nullptr, nullptr,
                            getter_AddRefs(splittedPreviousContent));
-        NS_ENSURE_SUCCESS(rv, rv);
+        if (NS_WARN_IF(NS_FAILED(rv))) {
+          return EditActionIgnored(rv);
+        }
 
         if (splittedPreviousContent) {
           previousContentParent = splittedPreviousContent->GetParentNode();
           previousContentOffset = previousContentParent ?
             previousContentParent->IndexOf(splittedPreviousContent) : -1;
         }
       }
 
-      NS_ENSURE_TRUE(previousContentParent, NS_ERROR_NULL_POINTER);
-
-      bool handled = false;
-      rv = MoveBlock(*previousContentParent->AsElement(), *rightBlock,
-                     previousContentOffset, rightOffset, &handled);
-      *aHandled |= handled;
-      NS_ENSURE_SUCCESS(rv, rv);
+      if (NS_WARN_IF(!previousContentParent)) {
+        return EditActionIgnored(NS_ERROR_NULL_POINTER);
+      }
+
+      ret |= MoveBlock(*previousContentParent->AsElement(), *rightBlock,
+                       previousContentOffset, rightOffset);
+      if (NS_WARN_IF(ret.Failed())) {
+        return ret;
+      }
     }
     if (brNode && NS_SUCCEEDED(htmlEditor->DeleteNode(brNode))) {
-      *aHandled = true;
+      ret.MarkAsHandled();
     }
   } else {
     // Normal case.  Blocks are siblings, or at least close enough.  An example
     // of the latter is <p>paragraph</p><ul><li>one<li>two<li>three</ul>.  The
     // first li and the p are not true siblings, but we still want to join them
     // if you backspace from li into p.
 
     // Adjust whitespace at block boundaries
     nsresult rv =
       WSRunObject::PrepareToJoinBlocks(htmlEditor, leftBlock, rightBlock);
-    NS_ENSURE_SUCCESS(rv, rv);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return EditActionIgnored(rv);
+    }
     // Do br adjustment.
     nsCOMPtr<Element> brNode =
       CheckForInvisibleBR(*leftBlock, BRLocation::blockEnd);
     if (mergeLists || leftBlock->NodeInfo()->NameAtom() ==
                       rightBlock->NodeInfo()->NameAtom()) {
       // Nodes are same type.  merge them.
       EditorDOMPoint pt = JoinNodesSmart(*leftBlock, *rightBlock);
       if (pt.node && mergeLists) {
         nsCOMPtr<Element> newBlock;
         ConvertListType(rightBlock, getter_AddRefs(newBlock),
                         existingList, nsGkAtoms::li);
       }
-      *aHandled = true;
+      ret.MarkAsHandled();
     } else {
       // Nodes are dissimilar types.
-      bool handled = false;
-      rv =
-        MoveBlock(*leftBlock, *rightBlock, leftOffset, rightOffset, &handled);
-      *aHandled |= handled;
-      NS_ENSURE_SUCCESS(rv, rv);
+      ret |= MoveBlock(*leftBlock, *rightBlock, leftOffset, rightOffset);
+      if (NS_WARN_IF(ret.Failed())) {
+        return ret;
+      }
     }
     if (brNode) {
       rv = htmlEditor->DeleteNode(brNode);
       // XXX In other top level if/else-if blocks, the result of DeleteNode()
       //     is ignored.  Why does only this result is respected?
-      NS_ENSURE_SUCCESS(rv, rv);
-      *aHandled = true;
-    }
-  }
-  return NS_OK;
-}
-
-nsresult
+      if (NS_WARN_IF(NS_FAILED(rv))) {
+        return ret.SetResult(rv);
+      }
+      ret.MarkAsHandled();
+    }
+  }
+  return ret;
+}
+
+EditActionResult
 HTMLEditRules::MoveBlock(Element& aLeftBlock,
                          Element& aRightBlock,
                          int32_t aLeftOffset,
-                         int32_t aRightOffset,
-                         bool* aHandled)
-{
-  MOZ_ASSERT(aHandled);
-
-  *aHandled = false;
-
+                         int32_t aRightOffset)
+{
   nsTArray<OwningNonNull<nsINode>> arrayOfNodes;
   // GetNodesFromPoint is the workhorse that figures out what we wnat to move.
   nsresult rv = GetNodesFromPoint(EditorDOMPoint(&aRightBlock, aRightOffset),
                                   EditAction::makeList, arrayOfNodes,
                                   TouchContent::yes);
-  NS_ENSURE_SUCCESS(rv, rv);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return EditActionIgnored(rv);
+  }
+
+  EditActionResult ret(NS_OK);
   for (uint32_t i = 0; i < arrayOfNodes.Length(); i++) {
     // get the node to act on
     if (IsBlockNode(arrayOfNodes[i])) {
       // For block nodes, move their contents only, then delete block.
-      bool handled = false;
-      rv = MoveContents(*arrayOfNodes[i]->AsElement(), aLeftBlock,
-                        &aLeftOffset, &handled);
-      *aHandled |= handled;
-      NS_ENSURE_SUCCESS(rv, rv);
-      NS_ENSURE_STATE(mHTMLEditor);
+      ret |=
+        MoveContents(*arrayOfNodes[i]->AsElement(), aLeftBlock, &aLeftOffset);
+      if (NS_WARN_IF(ret.Failed())) {
+        return ret;
+      }
+      if (NS_WARN_IF(!mHTMLEditor)) {
+        return ret.SetResult(NS_ERROR_UNEXPECTED);
+      }
       rv = mHTMLEditor->DeleteNode(arrayOfNodes[i]);
-      *aHandled = true;
+      ret.MarkAsHandled();
     } else {
       // Otherwise move the content as is, checking against the DTD.
-      bool handled = false;
-      rv = MoveNodeSmart(*arrayOfNodes[i]->AsContent(), aLeftBlock,
-                         &aLeftOffset, &handled);
-      *aHandled |= handled;
+      ret |=
+        MoveNodeSmart(*arrayOfNodes[i]->AsContent(), aLeftBlock, &aLeftOffset);
     }
   }
 
   // XXX We're only checking return value of the last iteration
-  NS_ENSURE_SUCCESS(rv, rv);
-  return NS_OK;
-}
-
-nsresult
+  if (NS_WARN_IF(ret.Failed())) {
+    return ret;
+  }
+
+  return ret;
+}
+
+EditActionResult
 HTMLEditRules::MoveNodeSmart(nsIContent& aNode,
                              Element& aDestElement,
-                             int32_t* aInOutDestOffset,
-                             bool* aHandled)
+                             int32_t* aInOutDestOffset)
 {
   MOZ_ASSERT(aInOutDestOffset);
-  MOZ_ASSERT(aHandled);
-
-  *aHandled = false;
-
-  NS_ENSURE_STATE(mHTMLEditor);
+
+  if (NS_WARN_IF(!mHTMLEditor)) {
+    return EditActionIgnored(NS_ERROR_UNEXPECTED);
+  }
+
   RefPtr<HTMLEditor> htmlEditor(mHTMLEditor);
 
   // Check if this node can go into the destination node
   if (htmlEditor->CanContain(aDestElement, aNode)) {
     // If it can, move it there
     nsresult rv =
       htmlEditor->MoveNode(&aNode, &aDestElement, *aInOutDestOffset);
-    NS_ENSURE_SUCCESS(rv, rv);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return EditActionIgnored(rv);
+    }
     if (*aInOutDestOffset != -1) {
       (*aInOutDestOffset)++;
     }
     // XXX Should we check if the node is actually moved in this case?
-    *aHandled = true;
+    return EditActionHandled();
   } else {
     // If it can't, move its children (if any), and then delete it.
+    EditActionResult ret(NS_OK);
     if (aNode.IsElement()) {
-      bool handled = false;
-      nsresult rv = MoveContents(*aNode.AsElement(), aDestElement,
-                                 aInOutDestOffset, &handled);
-      *aHandled |= handled;
-      NS_ENSURE_SUCCESS(rv, rv);
+      ret = MoveContents(*aNode.AsElement(), aDestElement, aInOutDestOffset);
+      if (NS_WARN_IF(ret.Failed())) {
+        return ret;
+      }
     }
 
     nsresult rv = htmlEditor->DeleteNode(&aNode);
-    NS_ENSURE_SUCCESS(rv, rv);
-    *aHandled = true;
-  }
-  return NS_OK;
-}
-
-nsresult
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return ret.SetResult(rv);
+    }
+    return ret.MarkAsHandled();
+  }
+}
+
+EditActionResult
 HTMLEditRules::MoveContents(Element& aElement,
                             Element& aDestElement,
-                            int32_t* aInOutDestOffset,
-                            bool* aHandled)
+                            int32_t* aInOutDestOffset)
 {
   MOZ_ASSERT(aInOutDestOffset);
-  MOZ_ASSERT(aHandled);
-
-  *aHandled = false;
-
-  NS_ENSURE_TRUE(&aElement != &aDestElement, NS_ERROR_ILLEGAL_VALUE);
-
+
+  if (NS_WARN_IF(&aElement == &aDestElement)) {
+    return EditActionIgnored(NS_ERROR_ILLEGAL_VALUE);
+  }
+
+  EditActionResult ret(NS_OK);
   while (aElement.GetFirstChild()) {
-    bool handled = false;
-    nsresult rv = MoveNodeSmart(*aElement.GetFirstChild(), aDestElement,
-                                aInOutDestOffset, &handled);
-    *aHandled |= handled;
-    NS_ENSURE_SUCCESS(rv, rv);
-  }
-  return NS_OK;
+    ret |=
+      MoveNodeSmart(*aElement.GetFirstChild(), aDestElement, aInOutDestOffset);
+    if (NS_WARN_IF(ret.Failed())) {
+      return ret;
+    }
+  }
+  return ret;
 }
 
 
 nsresult
 HTMLEditRules::DeleteNonTableElements(nsINode* aNode)
 {
   MOZ_ASSERT(aNode);
   if (!HTMLEditUtils::IsTableElementButNotTable(aNode)) {
--- a/editor/libeditor/HTMLEditRules.h
+++ b/editor/libeditor/HTMLEditRules.h
@@ -23,16 +23,17 @@ class nsIDOMDocument;
 class nsIDOMElement;
 class nsIDOMNode;
 class nsIEditor;
 class nsINode;
 class nsRange;
 
 namespace mozilla {
 
+class EditActionResult;
 class HTMLEditor;
 class RulesInfo;
 class TextEditor;
 struct EditorDOMPoint;
 namespace dom {
 class Element;
 class Selection;
 } // namespace dom
@@ -168,70 +169,62 @@ protected:
    * TryToJoinBlocks() tries to join two block elements.  The right element is
    * always joined to the left element.  If the elements are the same type and
    * not nested within each other, JoinNodesSmart() is called (example, joining
    * two list items together into one).  If the elements are not the same type,
    * or one is a descendant of the other, we instead destroy the right block
    * placing its children into leftblock.  DTD containment rules are followed
    * throughout.
    *
-   * @param aCanceled   returns true if the operation should do nothing anymore
-   *                    even if this doesn't join the blocks.
-   *                    NOTE: When this returns an error, nobody should refer
-   *                          the result of this.
-   * @param aHandled    returns true if this actually handles the request.
-   *                    Note that this may return true even if this does not
-   *                    join the block.  E.g., if the blocks shouldn't be
-   *                    joined or it's impossible to join them but it's not
+   * @return            Sets canceled to true if the operation should do
+   *                    nothing anymore even if this doesn't join the blocks.
+   *                    Sets handled to true if this actually handles the
+   *                    request.  Note that this may set it to true even if this
+   *                    does not join the block.  E.g., if the blocks shouldn't
+   *                    be joined or it's impossible to join them but it's not
    *                    unexpected case, this returns true with this.
-   *                    NOTE: When this returns an error, nobody should refer
-   *                          the result of this.
    */
-  nsresult TryToJoinBlocks(nsIContent& aLeftNode, nsIContent& aRightNode,
-                           bool* aCanceled, bool* aHandled);
+  EditActionResult TryToJoinBlocks(nsIContent& aLeftNode,
+                                   nsIContent& aRightNode);
 
   /**
    * MoveBlock() moves the content from aRightBlock starting from aRightOffset
    * into aLeftBlock at aLeftOffset. Note that the "block" can be inline nodes
    * between <br>s, or between blocks, etc.  DTD containment rules are followed
    * throughout.
    *
-   * @param aHandled    returns true if this actually joins the nodes.
-   *                    NOTE: When this returns an error, nobody should refer
-   *                          the result of this.
+   * @return            Sets handled to true if this actually joins the nodes.
+   *                    canceled is always false.
    */
-  nsresult MoveBlock(Element& aLeftBlock, Element& aRightBlock,
-                     int32_t aLeftOffset, int32_t aRightOffset,
-                     bool* aHandled);
+  EditActionResult MoveBlock(Element& aLeftBlock, Element& aRightBlock,
+                             int32_t aLeftOffset, int32_t aRightOffset);
 
   /**
    * MoveNodeSmart() moves aNode to (aDestElement, aInOutDestOffset).
    * DTD containment rules are followed throughout.
    *
    * @param aOffset                 returns the point after inserted content.
-   * @param aHandled                returns true if this actually moves the
-   *                                nodes.
-   *                                NOTE: When this returns an error, nobody
-   *                                      should refer the result of this.
+   * @return                        Sets true to handled if this actually moves
+   *                                the nodes.
+   *                                canceled is always false.
    */
-  nsresult MoveNodeSmart(nsIContent& aNode, Element& aDestElement,
-                         int32_t* aInOutDestOffset, bool* aHandled);
+  EditActionResult MoveNodeSmart(nsIContent& aNode, Element& aDestElement,
+                                 int32_t* aInOutDestOffset);
 
   /**
    * MoveContents() moves the contents of aElement to (aDestElement,
    * aInOutDestOffset).  DTD containment rules are followed throughout.
    *
    * @param aInOutDestOffset        updated to point after inserted content.
-   * @param aHandled                returns true if this actually moves the
-   *                                nodes.
-   *                                NOTE: When this returns an error, nobody
-   *                                      should refer the result of this.
+   * @return                        Sets true to handled if this actually moves
+   *                                the nodes.
+   *                                canceled is always false.
    */
-  nsresult MoveContents(Element& aElement, Element& aDestElement,
-                        int32_t* aInOutDestOffset, bool* aHandled);
+  EditActionResult MoveContents(Element& aElement, Element& aDestElement,
+                                int32_t* aInOutDestOffset);
 
   nsresult DeleteNonTableElements(nsINode* aNode);
   nsresult WillMakeList(Selection* aSelection,
                         const nsAString* aListType,
                         bool aEntireList,
                         const nsAString* aBulletType,
                         bool* aCancel, bool* aHandled,
                         const nsAString* aItemType = nullptr);