Bug 1316302.

2020-07-16 03:32:21 +03:00 · 2020-07-16 03:32:21 +03:00 · 4eebf89b27
parent 7dd71e41e4
commit 4eebf89b27
3 changed files with 395 additions and 144 deletions
--- a/editor/libeditor/EditorUtils.h
+++ b/editor/libeditor/EditorUtils.h
@ -30,6 +30,119 @@ 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.
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@ -1844,10 +1844,10 @@ HTMLEditRules::WillDeleteSelection(Selection* aSelection,

  // origCollapsed is used later to determine whether we should join blocks. We
  // don't really care about bCollapsed because it will be modified by
-  // ExtendSelectionForDelete later. JoinBlocks should happen if the original
-  // selection is collapsed and the cursor is at the end of a block element, in
-  // which case ExtendSelectionForDelete would always make the selection not
-  // collapsed.
+  // ExtendSelectionForDelete later. TryToJoinBlocks() should happen if the
+  // original selection is collapsed and the cursor is at the end of a block
+  // element, in which case ExtendSelectionForDelete would always make the
+  // selection not collapsed.
  bool bCollapsed = aSelection->Collapsed();
  bool join = false;
  bool origCollapsed = bCollapsed;
@ -2196,11 +2196,28 @@ HTMLEditRules::WillDeleteSelection(Selection* aSelection,
                                  address_of(selPointNode), &selPointOffset);
        NS_ENSURE_STATE(leftNode && leftNode->IsContent() &&
                        rightNode && rightNode->IsContent());
-        *aHandled = true;
-        rv = JoinBlocks(*leftNode->AsContent(), *rightNode->AsContent(),
-                        aCancel);
-        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 ?
+            static_cast<int32_t>(leafNode->Length()) : 0;
+        aSelection->Collapse(leafNode, offset);
+        return WillDeleteSelection(aSelection, aAction, aStripWrappers,
+                                   aCancel, aHandled);
+      }
+
+      // Otherwise, we must have deleted the selection as user expected.
      aSelection->Collapse(selPointNode, selPointOffset);
      return NS_OK;
    }
@ -2247,10 +2264,16 @@ HTMLEditRules::WillDeleteSelection(Selection* aSelection,
        AutoTrackDOMPoint tracker(mHTMLEditor->mRangeUpdater,
                                  address_of(selPointNode), &selPointOffset);
        NS_ENSURE_STATE(leftNode->IsContent() && rightNode->IsContent());
+        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;
-        rv = JoinBlocks(*leftNode->AsContent(), *rightNode->AsContent(),
-                        aCancel);
-        NS_ENSURE_SUCCESS(rv, rv);
+        *aCancel |= ret.Canceled();
+        if (NS_WARN_IF(ret.Failed())) {
+          return ret.Rv();
+        }
      }
      aSelection->Collapse(selPointNode, selPointOffset);
      return NS_OK;
@ -2421,8 +2444,12 @@ HTMLEditRules::WillDeleteSelection(Selection* aSelection,
        }

        if (join) {
-          rv = JoinBlocks(*leftParent, *rightParent, aCancel);
-          NS_ENSURE_SUCCESS(rv, rv);
+          EditActionResult ret = TryToJoinBlocks(*leftParent, *rightParent);
+          MOZ_ASSERT(*aHandled);
+          *aCancel |= ret.Canceled();
+          if (NS_WARN_IF(ret.Failed())) {
+            return ret.Rv();
+          }
        }
      }
    }
@ -2571,60 +2598,58 @@ HTMLEditRules::GetGoodSelPointForNode(nsINode& aNode,
  return ret;
 }

-
-/**
- * This method is used 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.
- */
-nsresult
-HTMLEditRules::JoinBlocks(nsIContent& aLeftNode,
-                          nsIContent& aRightNode,
-                          bool* aCanceled)
+EditActionResult
+HTMLEditRules::TryToJoinBlocks(nsIContent& aLeftNode,
+                               nsIContent& aRightNode)
 {
-  MOZ_ASSERT(aCanceled);
+  if (NS_WARN_IF(!mHTMLEditor)) {
+    return EditActionIgnored(NS_ERROR_UNEXPECTED);
+  }

-  NS_ENSURE_STATE(mHTMLEditor);
  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;
-    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);
+    if (NS_WARN_IF(!rightBlock)) {
+      return EditActionIgnored(NS_ERROR_UNEXPECTED);
+    }
  }
-  NS_ENSURE_STATE(leftBlock && rightBlock);

  // Bail if both blocks the same
  if (leftBlock == rightBlock) {
-    *aCanceled = 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) {
-    return NS_OK;
+    return EditActionHandled();
  }

  // Special rule here: if we are trying to join list items, and they are in
@ -2665,7 +2690,9 @@ HTMLEditRules::JoinBlocks(nsIContent& aLeftNode,
    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.
@ -2675,40 +2702,61 @@ HTMLEditRules::JoinBlocks(nsIContent& aLeftNode,
      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);
+    EditActionResult ret(NS_OK);
    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?
+      ret.MarkAsHandled();
    } else {
-      MoveBlock(*leftBlock, *rightBlock, leftOffset, rightOffset);
+      // XXX Why do we ignore the result of MoveBlock()?
+      EditActionResult retMoveBlock =
+        MoveBlock(*leftBlock, *rightBlock, leftOffset, rightOffset);
+      if (retMoveBlock.Handled()) {
+        ret.MarkAsHandled();
+      }
    }
-    if (brNode) {
-      htmlEditor->DeleteNode(brNode);
+    if (brNode && NS_SUCCEEDED(htmlEditor->DeleteNode(brNode))) {
+      ret.MarkAsHandled();
    }
+    return ret;
+  }
+
  // Offset below is where you find yourself in leftBlock when you traverse
  // upwards from rightBlock
-  } else if (EditorUtils::IsDescendantOf(rightBlock, leftBlock, &leftOffset)) {
+  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.
@ -2718,19 +2766,30 @@ HTMLEditRules::JoinBlocks(nsIContent& aLeftNode,
      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);
+    EditActionResult ret(NS_OK);
    if (mergeLists) {
-      MoveContents(*rightList, *leftList, &leftOffset);
+      // 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
@ -2775,7 +2834,9 @@ HTMLEditRules::JoinBlocks(nsIContent& aLeftNode,
                           &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();
@ -2784,58 +2845,67 @@ HTMLEditRules::JoinBlocks(nsIContent& aLeftNode,
        }
      }

-      NS_ENSURE_TRUE(previousContentParent, NS_ERROR_NULL_POINTER);
-
-      rv = MoveBlock(*previousContentParent->AsElement(), *rightBlock,
-                     previousContentOffset, rightOffset);
-      NS_ENSURE_SUCCESS(rv, rv);
-    }
-    if (brNode) {
-      htmlEditor->DeleteNode(brNode);
-    }
-  } 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);
-    // 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);
+      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;
      }
-    } else {
-      // Nodes are dissimilar types.
-      rv = MoveBlock(*leftBlock, *rightBlock, leftOffset, rightOffset);
-      NS_ENSURE_SUCCESS(rv, rv);
    }
-    if (brNode) {
-      rv = htmlEditor->DeleteNode(brNode);
-      NS_ENSURE_SUCCESS(rv, rv);
+    if (brNode && NS_SUCCEEDED(htmlEditor->DeleteNode(brNode))) {
+      ret.MarkAsHandled();
+    }
+    return ret;
+  }
+
+  // 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);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return EditActionIgnored(rv);
+  }
+  // Do br adjustment.
+  nsCOMPtr<Element> brNode =
+    CheckForInvisibleBR(*leftBlock, BRLocation::blockEnd);
+  EditActionResult ret(NS_OK);
+  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);
+    }
+    ret.MarkAsHandled();
+  } else {
+    // Nodes are dissimilar types.
+    ret |= MoveBlock(*leftBlock, *rightBlock, leftOffset, rightOffset);
+    if (NS_WARN_IF(ret.Failed())) {
+      return ret;
    }
  }
-  return NS_OK;
+  if (brNode) {
+    rv = htmlEditor->DeleteNode(brNode);
+    // XXX In other top level if blocks, the result of DeleteNode()
+    //     is ignored.  Why does only this result is respected?
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return ret.SetResult(rv);
+    }
+    ret.MarkAsHandled();
+  }
+  return ret;
 }

-
-/**
- * Moves the content from aRightBlock starting from aRightOffset into
- * aLeftBlock at aLeftOffset. Note that the "block" might merely be inline
- * nodes between <br>s, or between blocks, etc.  DTD containment rules are
- * followed throughout.
- */
-nsresult
+EditActionResult
 HTMLEditRules::MoveBlock(Element& aLeftBlock,
                         Element& aRightBlock,
                         int32_t aLeftOffset,
@ -2846,41 +2916,51 @@ HTMLEditRules::MoveBlock(Element& aLeftBlock,
  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.
-      rv = MoveContents(*arrayOfNodes[i]->AsElement(), aLeftBlock,
-                        &aLeftOffset);
-      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]);
+      ret.MarkAsHandled();
    } else {
      // Otherwise move the content as is, checking against the DTD.
-      rv = MoveNodeSmart(*arrayOfNodes[i]->AsContent(), aLeftBlock,
-                         &aLeftOffset);
+      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;
+  if (NS_WARN_IF(ret.Failed())) {
+    return ret;
+  }
+
+  return ret;
 }

-/**
- * This method is used to move node aNode to (aDestElement, aInOutDestOffset).
- * DTD containment rules are followed throughout.  aInOutDestOffset is updated
- * to point _after_ inserted content.
- */
-nsresult
+EditActionResult
 HTMLEditRules::MoveNodeSmart(nsIContent& aNode,
                             Element& aDestElement,
                             int32_t* aInOutDestOffset)
 {
  MOZ_ASSERT(aInOutDestOffset);

-  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
@ -2888,44 +2968,52 @@ HTMLEditRules::MoveNodeSmart(nsIContent& 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)++;
    }
-  } else {
-    // If it can't, move its children (if any), and then delete it.
-    if (aNode.IsElement()) {
-      nsresult rv =
-        MoveContents(*aNode.AsElement(), aDestElement, aInOutDestOffset);
-      NS_ENSURE_SUCCESS(rv, rv);
-    }
-
-    nsresult rv = htmlEditor->DeleteNode(&aNode);
-    NS_ENSURE_SUCCESS(rv, rv);
+    // XXX Should we check if the node is actually moved in this case?
+    return EditActionHandled();
  }
-  return NS_OK;
+
+  // If it can't, move its children (if any), and then delete it.
+  EditActionResult ret(NS_OK);
+  if (aNode.IsElement()) {
+    ret = MoveContents(*aNode.AsElement(), aDestElement, aInOutDestOffset);
+    if (NS_WARN_IF(ret.Failed())) {
+      return ret;
+    }
+  }
+
+  nsresult rv = htmlEditor->DeleteNode(&aNode);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return ret.SetResult(rv);
+  }
+  return ret.MarkAsHandled();
 }

-/**
- * Moves the _contents_ of aElement to (aDestElement, aInOutDestOffset).  DTD
- * containment rules are followed throughout.  aInOutDestOffset is updated to
- * point _after_ inserted content.
- */
-nsresult
+EditActionResult
 HTMLEditRules::MoveContents(Element& aElement,
                            Element& aDestElement,
                            int32_t* aInOutDestOffset)
 {
  MOZ_ASSERT(aInOutDestOffset);

-  NS_ENSURE_TRUE(&aElement != &aDestElement, NS_ERROR_ILLEGAL_VALUE);
-
-  while (aElement.GetFirstChild()) {
-    nsresult rv = MoveNodeSmart(*aElement.GetFirstChild(), aDestElement,
-                                aInOutDestOffset);
-    NS_ENSURE_SUCCESS(rv, rv);
+  if (NS_WARN_IF(&aElement == &aDestElement)) {
+    return EditActionIgnored(NS_ERROR_ILLEGAL_VALUE);
  }
-  return NS_OK;
+
+  EditActionResult ret(NS_OK);
+  while (aElement.GetFirstChild()) {
+    ret |=
+      MoveNodeSmart(*aElement.GetFirstChild(), aDestElement, aInOutDestOffset);
+    if (NS_WARN_IF(ret.Failed())) {
+      return ret;
+    }
+  }
+  return ret;
 }


--- a/editor/libeditor/HTMLEditRules.h
+++ b/editor/libeditor/HTMLEditRules.h
@ -28,6 +28,7 @@ class nsRange;

 namespace mozilla {

+class EditActionResult;
 class HTMLEditor;
 class RulesInfo;
 class TextEditor;
@ -163,14 +164,63 @@ protected:
  nsresult InsertBRIfNeeded(Selection* aSelection);
  mozilla::EditorDOMPoint GetGoodSelPointForNode(nsINode& aNode,
                                                 nsIEditor::EDirection aAction);
-  nsresult JoinBlocks(nsIContent& aLeftNode, nsIContent& aRightNode,
-                      bool* aCanceled);
-  nsresult MoveBlock(Element& aLeftBlock, Element& aRightBlock,
-                     int32_t aLeftOffset, int32_t aRightOffset);
-  nsresult MoveNodeSmart(nsIContent& aNode, Element& aDestElement,
-                         int32_t* aOffset);
-  nsresult MoveContents(Element& aElement, Element& aDestElement,
-                        int32_t* aOffset);
+
+  /**
+   * 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.
+   *
+   * @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.
+   */
+  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.
+   *
+   * @return            Sets handled to true if this actually joins the nodes.
+   *                    canceled is always false.
+   */
+  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.
+   * @return                        Sets true to handled if this actually moves
+   *                                the nodes.
+   *                                canceled is always false.
+   */
+  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.
+   * @return                        Sets true to handled if this actually moves
+   *                                the nodes.
+   *                                canceled is always false.
+   */
+  EditActionResult MoveContents(Element& aElement, Element& aDestElement,
+                                int32_t* aInOutDestOffset);
+
  nsresult DeleteNonTableElements(nsINode* aNode);
  nsresult WillMakeList(Selection* aSelection,
                        const nsAString* aListType,