Bug 1415414 - Make DeleteRangeTransaction::CreateTxnsToDeleteBetween() and DeleteRangeTransaction::CreateTxnsToDeleteContent() use RawRangeBoundary as their arguments r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 08 Nov 2017 15:01:23 +0900
changeset 694841 c4a86ec3a416b868a4c4042b13afda67db271d81
parent 694769 c873cb28d82a40376fcf450e63efbd7dc5090dfb
child 739459 9cfbc96c4071fc78c4a65d73a75670d850d4bf2a
push id88272
push usermasayuki@d-toybox.com
push dateWed, 08 Nov 2017 11:24:31 +0000
reviewersm_kato
bugs1415414, 1407352
milestone58.0a1
Bug 1415414 - Make DeleteRangeTransaction::CreateTxnsToDeleteBetween() and DeleteRangeTransaction::CreateTxnsToDeleteContent() use RawRangeBoundary as their arguments r?m_kato By the fix of bug 1407352, DeleteRangeTransaction::CreateTxnsToDeleteBetween() starts to use child node instead of a set of container node and offset in it. However, this is ugly and using RawRangeBoundary may reduce computing offset more. So, they should use RawRangeBoundary to refer delete points. MozReview-Commit-ID: pwDHOxpz0E
editor/libeditor/DeleteRangeTransaction.cpp
editor/libeditor/DeleteRangeTransaction.h
--- a/editor/libeditor/DeleteRangeTransaction.cpp
+++ b/editor/libeditor/DeleteRangeTransaction.cpp
@@ -6,16 +6,17 @@
 #include "DeleteRangeTransaction.h"
 
 #include "DeleteNodeTransaction.h"
 #include "DeleteTextTransaction.h"
 #include "mozilla/Assertions.h"
 #include "mozilla/EditorBase.h"
 #include "mozilla/dom/Selection.h"
 #include "mozilla/mozalloc.h"
+#include "mozilla/RangeBoundary.h"
 #include "nsCOMPtr.h"
 #include "nsDebug.h"
 #include "nsError.h"
 #include "nsIContent.h"
 #include "nsIContentIterator.h"
 #include "nsINode.h"
 #include "nsAString.h"
 
@@ -51,56 +52,64 @@ DeleteRangeTransaction::DoTransaction()
   // Swap mRangeToDelete out into a stack variable, so we make sure to null it
   // out on return from this function.  Once this function returns, we no longer
   // need mRangeToDelete, and keeping it alive in the long term slows down all
   // DOM mutations because it's observing them.
   RefPtr<nsRange> rangeToDelete;
   rangeToDelete.swap(mRangeToDelete);
 
   // build the child transactions
-  nsCOMPtr<nsINode> startContainer = rangeToDelete->GetStartContainer();
-  int32_t startOffset = rangeToDelete->StartOffset();
-  nsCOMPtr<nsINode> endContainer = rangeToDelete->GetEndContainer();
-  int32_t endOffset = rangeToDelete->EndOffset();
-  MOZ_ASSERT(startContainer && endContainer);
+  const RangeBoundary& startRef = rangeToDelete->StartRef();
+  const RangeBoundary& endRef = rangeToDelete->EndRef();
+  MOZ_ASSERT(startRef.IsSetAndValid());
+  MOZ_ASSERT(endRef.IsSetAndValid());
 
-  if (startContainer == endContainer) {
+  if (startRef.Container() == endRef.Container()) {
     // the selection begins and ends in the same node
-    nsIContent* startChild = rangeToDelete->GetChildAtStartOffset();
-    nsresult rv =
-      CreateTxnsToDeleteBetween(startContainer, startOffset,
-                                startChild, endOffset);
-    NS_ENSURE_SUCCESS(rv, rv);
+    nsresult rv = CreateTxnsToDeleteBetween(startRef.AsRaw(), endRef.AsRaw());
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
   } else {
     // the selection ends in a different node from where it started.  delete
     // the relevant content in the start node
-    nsresult rv =
-      CreateTxnsToDeleteContent(startContainer, startOffset, nsIEditor::eNext);
-    NS_ENSURE_SUCCESS(rv, rv);
+    nsresult rv = CreateTxnsToDeleteContent(startRef.AsRaw(), nsIEditor::eNext);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
     // delete the intervening nodes
     rv = CreateTxnsToDeleteNodesBetween(rangeToDelete);
-    NS_ENSURE_SUCCESS(rv, rv);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
     // delete the relevant content in the end node
-    rv = CreateTxnsToDeleteContent(endContainer, endOffset,
-                                   nsIEditor::ePrevious);
-    NS_ENSURE_SUCCESS(rv, rv);
+    rv = CreateTxnsToDeleteContent(endRef.AsRaw(), nsIEditor::ePrevious);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
   }
 
   // if we've successfully built this aggregate transaction, then do it.
   nsresult rv = EditAggregateTransaction::DoTransaction();
-  NS_ENSURE_SUCCESS(rv, rv);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
 
   // only set selection to deletion point if editor gives permission
   bool bAdjustSelection;
   mEditorBase->ShouldTxnSetSelection(&bAdjustSelection);
   if (bAdjustSelection) {
     RefPtr<Selection> selection = mEditorBase->GetSelection();
-    NS_ENSURE_TRUE(selection, NS_ERROR_NULL_POINTER);
-    rv = selection->Collapse(startContainer, startOffset);
-    NS_ENSURE_SUCCESS(rv, rv);
+    if (NS_WARN_IF(!selection)) {
+      return NS_ERROR_NULL_POINTER;
+    }
+    rv = selection->Collapse(startRef.AsRaw());
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
   }
   // else do nothing - dom range gravity will adjust selection
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 DeleteRangeTransaction::UndoTransaction()
@@ -117,108 +126,118 @@ DeleteRangeTransaction::RedoTransaction(
 NS_IMETHODIMP
 DeleteRangeTransaction::GetTxnDescription(nsAString& aString)
 {
   aString.AssignLiteral("DeleteRangeTransaction");
   return NS_OK;
 }
 
 nsresult
-DeleteRangeTransaction::CreateTxnsToDeleteBetween(nsINode* aNode,
-                                                  int32_t aStartOffset,
-                                                  nsIContent* aChildAtStartOffset,
-                                                  int32_t aEndOffset)
+DeleteRangeTransaction::CreateTxnsToDeleteBetween(
+                          const RawRangeBoundary& aStart,
+                          const RawRangeBoundary& aEnd)
 {
+  if (NS_WARN_IF(!aStart.IsSetAndValid()) ||
+      NS_WARN_IF(!aEnd.IsSetAndValid()) ||
+      NS_WARN_IF(aStart.Container() != aEnd.Container())) {
+    return NS_ERROR_INVALID_ARG;
+  }
+
   if (NS_WARN_IF(!mEditorBase)) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   // see what kind of node we have
-  if (aNode->IsNodeOfType(nsINode::eDATA_NODE)) {
+  if (aStart.Container()->IsNodeOfType(nsINode::eDATA_NODE)) {
     // if the node is a chardata node, then delete chardata content
     int32_t numToDel;
-    if (aStartOffset == aEndOffset) {
+    if (aStart == aEnd) {
       numToDel = 1;
     } else {
-      numToDel = aEndOffset - aStartOffset;
+      numToDel = aEnd.Offset() - aStart.Offset();
+      MOZ_DIAGNOSTIC_ASSERT(numToDel > 0);
     }
 
     RefPtr<nsGenericDOMDataNode> charDataNode =
-      static_cast<nsGenericDOMDataNode*>(aNode);
+      static_cast<nsGenericDOMDataNode*>(aStart.Container());
 
     RefPtr<DeleteTextTransaction> deleteTextTransaction =
-      new DeleteTextTransaction(*mEditorBase, *charDataNode, aStartOffset,
+      new DeleteTextTransaction(*mEditorBase, *charDataNode, aStart.Offset(),
                                 numToDel, mRangeUpdater);
     // If the text node isn't editable, it should be never undone/redone.
     // So, the transaction shouldn't be recorded.
     if (NS_WARN_IF(!deleteTextTransaction->CanDoIt())) {
       return NS_ERROR_FAILURE;
     }
     AppendChild(deleteTextTransaction);
     return NS_OK;
   }
 
-  nsIContent* child = aChildAtStartOffset;
-  for (int32_t i = aStartOffset; i < aEndOffset; ++i) {
-    // Even if we detect invalid range, we should ignore it for removing
-    // specified range's nodes as far as possible.
-    if (NS_WARN_IF(!child)) {
-      break;
-    }
+  // Even if we detect invalid range, we should ignore it for removing
+  // specified range's nodes as far as possible.
+  for (nsIContent* child = aStart.GetChildAtOffset();
+       child && child != aEnd.GetChildAtOffset();
+       child = child->GetNextSibling()) {
     RefPtr<DeleteNodeTransaction> deleteNodeTransaction =
       new DeleteNodeTransaction(*mEditorBase, *child, mRangeUpdater);
     // XXX This is odd handling.  Even if some children are not editable,
     //     editor should append transactions because they could be editable
     //     at undoing/redoing.  Additionally, if the transaction needs to
     //     delete/restore all nodes, it should at undoing/redoing.
     if (deleteNodeTransaction->CanDoIt()) {
       AppendChild(deleteNodeTransaction);
     }
-    child = child->GetNextSibling();
   }
 
   return NS_OK;
 }
 
 nsresult
-DeleteRangeTransaction::CreateTxnsToDeleteContent(nsINode* aNode,
-                                                  int32_t aOffset,
-                                                  nsIEditor::EDirection aAction)
+DeleteRangeTransaction::CreateTxnsToDeleteContent(
+                          const RawRangeBoundary& aPoint,
+                          nsIEditor::EDirection aAction)
 {
+  if (NS_WARN_IF(!aPoint.IsSetAndValid())) {
+    return NS_ERROR_INVALID_ARG;
+  }
+
   if (NS_WARN_IF(!mEditorBase)) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
-  // see what kind of node we have
-  if (aNode->IsNodeOfType(nsINode::eDATA_NODE)) {
-    // if the node is a chardata node, then delete chardata content
-    uint32_t start, numToDelete;
-    if (nsIEditor::eNext == aAction) {
-      start = aOffset;
-      numToDelete = aNode->Length() - aOffset;
-    } else {
-      start = 0;
-      numToDelete = aOffset;
-    }
+  if (!aPoint.Container()->IsNodeOfType(nsINode::eDATA_NODE)) {
+    return NS_OK;
+  }
+
+  // If the node is a chardata node, then delete chardata content
+  uint32_t startOffset, numToDelete;
+  if (nsIEditor::eNext == aAction) {
+    startOffset = aPoint.Offset();
+    numToDelete = aPoint.Container()->Length() - aPoint.Offset();
+  } else {
+    startOffset = 0;
+    numToDelete = aPoint.Offset();
+  }
 
-    if (numToDelete) {
-      RefPtr<nsGenericDOMDataNode> dataNode =
-        static_cast<nsGenericDOMDataNode*>(aNode);
-      RefPtr<DeleteTextTransaction> deleteTextTransaction =
-        new DeleteTextTransaction(*mEditorBase, *dataNode, start, numToDelete,
-                                  mRangeUpdater);
-      // If the text node isn't editable, it should be never undone/redone.
-      // So, the transaction shouldn't be recorded.
-      if (NS_WARN_IF(!deleteTextTransaction->CanDoIt())) {
-        return NS_ERROR_FAILURE;
-      }
-      AppendChild(deleteTextTransaction);
-    }
+  if (!numToDelete) {
+    return NS_OK;
   }
 
+  RefPtr<nsGenericDOMDataNode> dataNode =
+    static_cast<nsGenericDOMDataNode*>(aPoint.Container());
+  RefPtr<DeleteTextTransaction> deleteTextTransaction =
+    new DeleteTextTransaction(*mEditorBase, *dataNode, startOffset, numToDelete,
+                              mRangeUpdater);
+  // If the text node isn't editable, it should be never undone/redone.
+  // So, the transaction shouldn't be recorded.
+  if (NS_WARN_IF(!deleteTextTransaction->CanDoIt())) {
+    return NS_ERROR_FAILURE;
+  }
+  AppendChild(deleteTextTransaction);
+
   return NS_OK;
 }
 
 nsresult
 DeleteRangeTransaction::CreateTxnsToDeleteNodesBetween(nsRange* aRangeToDelete)
 {
   if (NS_WARN_IF(!mEditorBase)) {
     return NS_ERROR_NOT_AVAILABLE;
--- a/editor/libeditor/DeleteRangeTransaction.h
+++ b/editor/libeditor/DeleteRangeTransaction.h
@@ -2,16 +2,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef DeleteRangeTransaction_h
 #define DeleteRangeTransaction_h
 
 #include "EditAggregateTransaction.h"
+#include "mozilla/RangeBoundary.h"
 #include "nsCycleCollectionParticipant.h"
 #include "nsID.h"
 #include "nsIEditor.h"
 #include "nsISupportsImpl.h"
 #include "nsRange.h"
 #include "nscore.h"
 
 class nsINode;
@@ -45,25 +46,63 @@ public:
 
   virtual void LastRelease() override
   {
     mRangeToDelete = nullptr;
     EditAggregateTransaction::LastRelease();
   }
 
 protected:
-  nsresult CreateTxnsToDeleteBetween(nsINode* aNode,
-                                     int32_t aStartOffset,
-                                     nsIContent* aChildAtStartOffset,
-                                     int32_t aEndOffset);
+  /**
+   * CreateTxnsToDeleteBetween() creates a DeleteTextTransaction or some
+   * DeleteNodeTransactions to remove text or nodes between aStart and aEnd
+   * and appends the created transactions to the array.
+   *
+   * @param aStart      Must be set and valid point.
+   * @param aEnd        Must be set and valid point.  Additionally, the
+   *                    container must be same as aStart's container.
+   *                    And of course, this must not be before aStart in
+   *                    the DOM tree order.
+   * @return            Returns NS_OK in most cases.
+   *                    When the arguments are invalid, returns
+   *                    NS_ERROR_INVALID_ARG.
+   *                    When mEditorBase isn't available, returns
+   *                    NS_ERROR_NOT_AVAIALBLE.
+   *                    When created DeleteTextTransaction cannot do its
+   *                    transaction, returns NS_ERROR_FAILURE.
+   *                    Note that even if one of created DeleteNodeTransaction
+   *                    cannot do its transaction, this returns NS_OK.
+   */
+  nsresult CreateTxnsToDeleteBetween(const RawRangeBoundary& aStart,
+                                     const RawRangeBoundary& aEnd);
 
   nsresult CreateTxnsToDeleteNodesBetween(nsRange* aRangeToDelete);
 
-  nsresult CreateTxnsToDeleteContent(nsINode* aParent,
-                                     int32_t aOffset,
+  /**
+   * CreateTxnsToDeleteContent() creates a DeleteTextTransaction to delete
+   * text between start of aPoint.Container() and aPoint or aPoint and end of
+   * aPoint.Container() and appends the created transaction to the array.
+   *
+   * @param aPoint      Must be set and valid point.  If the container is not
+   *                    a data node, this method does nothing.
+   * @param aAction     If nsIEditor::eNext, this method creates a transaction
+   *                    to delete text from aPoint to the end of the data node.
+   *                    Otherwise, this method creates a transaction to delete
+   *                    text from start of the data node to aPoint.
+   * @return            Returns NS_OK in most cases.
+   *                    When the arguments are invalid, returns
+   *                    NS_ERROR_INVALID_ARG.
+   *                    When mEditorBase isn't available, returns
+   *                    NS_ERROR_NOT_AVAIALBLE.
+   *                    When created DeleteTextTransaction cannot do its
+   *                    transaction, returns NS_ERROR_FAILURE.
+   *                    Note that even if no character will be deleted,
+   *                    this returns NS_OK.
+   */
+  nsresult CreateTxnsToDeleteContent(const RawRangeBoundary& aPoint,
                                      nsIEditor::EDirection aAction);
 
   // The editor for this transaction.
   RefPtr<EditorBase> mEditorBase;
 
   // P1 in the range.  This is only non-null until DoTransaction is called and
   // we convert it into child transactions.
   RefPtr<nsRange> mRangeToDelete;