Bug 890833 - Cleanup nsEditor::MoveNode; r=ehsan

editor/libeditor/base/nsEditor.cpp

@@ -1734,52 +1734,57 @@ nsEditor::InsertContainerAbove(nsIContent* aNode,
 ///////////////////////////////////////////////////////////////////////////
 // MoveNode:  move aNode to {aParent,aOffset}
 nsresult
-nsEditor::MoveNode(nsIContent* aNode, nsINode* aParent, int32_t aOffset)
+nsEditor::MoveNode(nsIDOMNode* aNode, nsIDOMNode* aParent, int32_t aOffset)
 {
-  MOZ_ASSERT(aNode && aParent);
-  MOZ_ASSERT(aOffset == -1 || (0 <= aOffset &&
-                               aOffset <= (int32_t)aParent->Length()));
-  nsresult res = MoveNode(aNode->AsDOMNode(), aParent->AsDOMNode(), aOffset);
-  NS_ASSERTION(NS_SUCCEEDED(res), "MoveNode failed");
-  NS_ENSURE_SUCCESS(res, res);
-  return NS_OK;
+  nsCOMPtr<nsINode> node = do_QueryInterface(aNode);
+  NS_ENSURE_STATE(node);
+
+  nsCOMPtr<nsINode> parent = do_QueryInterface(aParent);
+  NS_ENSURE_STATE(parent);
+
+  return MoveNode(node, parent, aOffset);
 }
 
 nsresult
-nsEditor::MoveNode(nsIDOMNode *aNode, nsIDOMNode *aParent, int32_t aOffset)
+nsEditor::MoveNode(nsINode* aNode, nsINode* aParent, int32_t aOffset)
 {
-  NS_ENSURE_TRUE(aNode && aParent, NS_ERROR_NULL_POINTER);
-  nsresult res;
+  MOZ_ASSERT(aNode);
+  MOZ_ASSERT(aParent);
+  MOZ_ASSERT(aOffset == -1 ||
+             (0 <= aOffset && SafeCast<uint32_t>(aOffset) <= aParent->Length()));
 
   int32_t oldOffset;
-  nsCOMPtr<nsIDOMNode> oldParent = GetNodeLocation(aNode, &oldOffset);
+  nsCOMPtr<nsINode> oldParent = GetNodeLocation(aNode, &oldOffset);
   
-  if (aOffset == -1)
-  {
-    uint32_t unsignedOffset;
-    // magic value meaning "move to end of aParent"
-    res = GetLengthOfDOMNode(aParent, unsignedOffset);
-    NS_ENSURE_SUCCESS(res, res);
-    aOffset = (int32_t)unsignedOffset;
+  if (aOffset == -1) {
+    // Magic value meaning "move to end of aParent".
+    aOffset = SafeCast<int32_t>(aParent->Length());
   }
   
-  // don't do anything if it's already in right place
-  if ((aParent == oldParent.get()) && (oldOffset == aOffset)) return NS_OK;
+  // Don't do anything if it's already in right place.
+  if (aParent == oldParent && aOffset == oldOffset) {
+    return NS_OK;
+  }
   
-  // notify our internal selection state listener
-  nsAutoMoveNodeSelNotify selNotify(mRangeUpdater, oldParent, oldOffset, aParent, aOffset);
+  // Notify our internal selection state listener.
+  nsAutoMoveNodeSelNotify selNotify(mRangeUpdater, oldParent, oldOffset,
+                                    aParent, aOffset);
   
-  // need to adjust aOffset if we are moving aNode further along in its current parent
-  if ((aParent == oldParent.get()) && (oldOffset < aOffset)) 
-  {
-    aOffset--;  // this is because when we delete aNode, it will make the offsets after it off by one
+  // Need to adjust aOffset if we are moving aNode further along in its current
+  // parent.
+  if (aParent == oldParent && oldOffset < aOffset) {
+    // This is because when we delete aNode, it will make the offsets after it
+    // off by one.
+    aOffset--;
   }
 
-  // Hold a reference so aNode doesn't go away when we remove it (bug 772282)
-  nsCOMPtr<nsIDOMNode> node = aNode;
-  res = DeleteNode(node);
-  NS_ENSURE_SUCCESS(res, res);
-  return InsertNode(node, aParent, aOffset);
+  // Hold a reference so aNode doesn't go away when we remove it (bug 772282).
+  nsCOMPtr<nsINode> kungFuDeathGrip = aNode;
+
+  nsresult rv = DeleteNode(aNode);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  return InsertNode(aNode->AsDOMNode(), aParent->AsDOMNode(), aOffset);
 }
 
 
@@ -3073,6 +3078,22 @@ nsEditor::GetNodeLocation(nsIDOMNode* aChild, int32_t* outOffset)
   return parent.forget();
 }
 
+nsINode*
+nsEditor::GetNodeLocation(nsINode* aChild, int32_t* aOffset)
+{
+  MOZ_ASSERT(aChild);
+  MOZ_ASSERT(aOffset);
+
+  nsINode* parent = aChild->GetParentNode();
+  if (parent) {
+    *aOffset = parent->IndexOf(aChild);
+    MOZ_ASSERT(*aOffset != -1);
+  } else {
+    *aOffset = -1;
+  }
+  return parent;
+}
+
 // returns the number of things inside aNode.  
 // If aNode is text, returns number of characters. If not, returns number of children nodes.
 nsresult

editor/libeditor/base/nsEditor.h

@@ -230,7 +230,7 @@ public:
                                 const nsAString *aAttribute = nullptr,
                                 const nsAString *aValue = nullptr);
   nsresult JoinNodes(nsINode* aNodeToKeep, nsIContent* aNodeToMove);
-  nsresult MoveNode(nsIContent* aNode, nsINode* aParent, int32_t aOffset);
+  nsresult MoveNode(nsINode* aNode, nsINode* aParent, int32_t aOffset);
   nsresult MoveNode(nsIDOMNode *aNode, nsIDOMNode *aParent, int32_t aOffset);
 
   /* Method to replace certain CreateElementNS() calls. 
@@ -471,6 +471,7 @@ public:
    */
   static already_AddRefed<nsIDOMNode> GetNodeLocation(nsIDOMNode* aChild,
                                                       int32_t* outOffset);
+  static nsINode* GetNodeLocation(nsINode* aChild, int32_t* aOffset);
 
   /** returns the number of things inside aNode in the out-param aCount.  
     * @param  aNode is the node to get the length of.  

editor/libeditor/base/nsSelectionState.cpp

@@ -592,47 +592,45 @@ nsRangeUpdater::DidInsertContainer()
 }
 
 
-nsresult
+void
 nsRangeUpdater::WillMoveNode()
 {
-  if (mLock) return NS_ERROR_UNEXPECTED;  
   mLock = true;
-  return NS_OK;
 }
 
 
-nsresult
-nsRangeUpdater::DidMoveNode(nsIDOMNode *aOldParent, int32_t aOldOffset, nsIDOMNode *aNewParent, int32_t aNewOffset)
+void
+nsRangeUpdater::DidMoveNode(nsINode* aOldParent, int32_t aOldOffset,
+                            nsINode* aNewParent, int32_t aNewOffset)
 {
-  NS_ENSURE_TRUE(mLock, NS_ERROR_UNEXPECTED);  
+  MOZ_ASSERT(aOldParent);
+  MOZ_ASSERT(aNewParent);
+  NS_ENSURE_TRUE_VOID(mLock);
   mLock = false;
 
-  NS_ENSURE_TRUE(aOldParent && aNewParent, NS_ERROR_NULL_POINTER);
-  uint32_t i, count = mArray.Length();
-  if (!count) {
-    return NS_OK;
-  }
+  nsIDOMNode* oldParent = aOldParent->AsDOMNode();
+  nsIDOMNode* newParent = aNewParent->AsDOMNode();
 
-  nsRangeStore *item;
-  
-  for (i=0; i<count; i++)
-  {
-    item = mArray[i];
-    NS_ENSURE_TRUE(item, NS_ERROR_NULL_POINTER);
+  for (uint32_t i = 0, count = mArray.Length(); i < count; ++i) {
+    nsRangeStore* item = mArray[i];
+    NS_ENSURE_TRUE_VOID(item);
     
     // like a delete in aOldParent
-    if ((item->startNode.get() == aOldParent) && (item->startOffset > aOldOffset))
+    if (item->startNode == oldParent && item->startOffset > aOldOffset) {
       item->startOffset--;
-    if ((item->endNode.get() == aOldParent) && (item->endOffset > aOldOffset))
+    }
+    if (item->endNode == oldParent && item->endOffset > aOldOffset) {
       item->endOffset--;
+    }
       
     // and like an insert in aNewParent
-    if ((item->startNode.get() == aNewParent) && (item->startOffset > aNewOffset))
+    if (item->startNode == newParent && item->startOffset > aNewOffset) {
       item->startOffset++;
-    if ((item->endNode.get() == aNewParent) && (item->endOffset > aNewOffset))
+    }
+    if (item->endNode == newParent && item->endOffset > aNewOffset) {
       item->endOffset++;
+    }
   }
-  return NS_OK;
 }
 
 

editor/libeditor/base/nsSelectionState.h

@@ -102,8 +102,9 @@ class nsRangeUpdater
     nsresult DidRemoveContainer(nsIDOMNode *aNode, nsIDOMNode *aParent, int32_t aOffset, uint32_t aNodeOrigLen);
     nsresult WillInsertContainer();
     nsresult DidInsertContainer();
-    nsresult WillMoveNode();
-    nsresult DidMoveNode(nsIDOMNode *aOldParent, int32_t aOldOffset, nsIDOMNode *aNewParent, int32_t aNewOffset);
+    void WillMoveNode();
+    void DidMoveNode(nsINode* aOldParent, int32_t aOldOffset,
+                     nsINode* aNewParent, int32_t aNewOffset);
   protected:    
     nsTArray<nsRefPtr<nsRangeStore> > mArray;
     bool mLock;
@@ -242,23 +243,25 @@ class MOZ_STACK_CLASS nsAutoMoveNodeSelNotify
 {
   private:
     nsRangeUpdater &mRU;
-    nsIDOMNode *mOldParent;
-    nsIDOMNode *mNewParent;
+    nsINode* mOldParent;
+    nsINode* mNewParent;
     int32_t    mOldOffset;
     int32_t    mNewOffset;
 
   public:
     nsAutoMoveNodeSelNotify(nsRangeUpdater &aRangeUpdater, 
-                            nsIDOMNode *aOldParent, 
+                            nsINode* aOldParent,
                             int32_t aOldOffset, 
-                            nsIDOMNode *aNewParent, 
-                            int32_t aNewOffset) :
-    mRU(aRangeUpdater)
-    ,mOldParent(aOldParent)
-    ,mNewParent(aNewParent)
-    ,mOldOffset(aOldOffset)
-    ,mNewOffset(aNewOffset)
+                            nsINode* aNewParent,
+                            int32_t aNewOffset)
+      : mRU(aRangeUpdater)
+      , mOldParent(aOldParent)
+      , mNewParent(aNewParent)
+      , mOldOffset(aOldOffset)
+      , mNewOffset(aNewOffset)
     {
+      MOZ_ASSERT(aOldParent);
+      MOZ_ASSERT(aNewParent);
       mRU.WillMoveNode();
     }