Bug 1574852 - part 67-5: Rewrite `HTMLEditRules::HandleDeleteNonCollapsedSelection()` with `EditorDOMPoint` r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 06 Sep 2019 00:59:32 +0000
changeset 491972 18c10d385d35d5992942786081b79036648bc58e
parent 491971 221a89193a200500d80441f7f32f38912c639f60
child 491973 2aa14f049e8dc6e9997a994e0ce1af4ef2087675
push id94640
push usermasayuki@d-toybox.com
push dateFri, 06 Sep 2019 05:06:08 +0000
treeherderautoland@18c10d385d35 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1574852
milestone71.0a1
first release with
nightly linux32
18c10d385d35 / 71.0a1 / 20190906094324 / files
nightly linux64
18c10d385d35 / 71.0a1 / 20190906094324 / files
nightly mac
18c10d385d35 / 71.0a1 / 20190906094324 / files
nightly win32
18c10d385d35 / 71.0a1 / 20190906094324 / files
nightly win64
18c10d385d35 / 71.0a1 / 20190906094324 / files
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1574852 - part 67-5: Rewrite `HTMLEditRules::HandleDeleteNonCollapsedSelection()` with `EditorDOMPoint` r=m_kato Before splitting the method, we should make it use `EditorDOMPoint` to treat selection edges. Differential Revision: https://phabricator.services.mozilla.com/D44455
editor/libeditor/HTMLEditRules.cpp
editor/libeditor/SelectionState.h
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -3093,87 +3093,99 @@ EditActionResult HTMLEditRules::HandleDe
       }
     }
   }
 
   // Remember that we did a ranged delete for the benefit of AfterEditInner().
   HTMLEditorRef().TopLevelEditSubActionDataRef().mDidDeleteNonCollapsedRange =
       true;
 
-  // Refresh start and end points
   nsRange* firstRange = SelectionRefPtr()->GetRangeAt(0);
   if (NS_WARN_IF(!firstRange)) {
     return EditActionResult(NS_ERROR_FAILURE);
   }
-  nsCOMPtr<nsINode> startNode = firstRange->GetStartContainer();
-  if (NS_WARN_IF(!startNode)) {
+  EditorDOMPoint firstRangeStart(firstRange->StartRef());
+  EditorDOMPoint firstRangeEnd(firstRange->EndRef());
+  if (NS_WARN_IF(!firstRangeStart.IsSet()) ||
+      NS_WARN_IF(!firstRangeEnd.IsSet())) {
     return EditActionResult(NS_ERROR_FAILURE);
   }
-  int32_t startOffset = firstRange->StartOffset();
-  nsCOMPtr<nsINode> endNode = firstRange->GetEndContainer();
-  if (NS_WARN_IF(!endNode)) {
-    return EditActionResult(NS_ERROR_FAILURE);
-  }
-  int32_t endOffset = firstRange->EndOffset();
 
   // Figure out if the endpoints are in nodes that can be merged.  Adjust
   // surrounding whitespace in preparation to delete selection.
   if (!IsPlaintextEditor()) {
     AutoTransactionsConserveSelection dontChangeMySelection(HTMLEditorRef());
+    nsCOMPtr<nsINode> startNode = firstRangeStart.GetContainer();
+    int32_t startOffset = firstRangeStart.Offset();
+    nsCOMPtr<nsINode> endNode = firstRangeEnd.GetContainer();
+    int32_t endOffset = firstRangeEnd.Offset();
     nsresult rv = WSRunObject::PrepareToDeleteRange(
         MOZ_KnownLive(&HTMLEditorRef()), address_of(startNode), &startOffset,
         address_of(endNode), &endOffset);
     if (NS_WARN_IF(!CanHandleEditAction())) {
       return EditActionResult(NS_ERROR_EDITOR_DESTROYED);
     }
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return EditActionResult(rv);
     }
+    firstRangeStart.Set(startNode, startOffset);
+    firstRangeEnd.Set(endNode, endOffset);
+    if (NS_WARN_IF(!firstRangeStart.IsSet()) ||
+        NS_WARN_IF(!firstRangeEnd.IsSet())) {
+      return EditActionResult(NS_ERROR_FAILURE);
+    }
   }
 
   bool join = false;
   EditActionResult result(NS_OK);
   result.MarkAsHandled();
   {
     // Track location of where we are deleting
+    // NOTE: Right now, firstRangeStart.mOffset and firstRangeEnd.mOffset
+    //       are fixed so that we keep compatibility with older code which
+    //       treated offset directly.
     AutoTrackDOMPoint startTracker(HTMLEditorRef().RangeUpdaterRef(),
-                                   address_of(startNode), &startOffset);
+                                   &firstRangeStart);
     AutoTrackDOMPoint endTracker(HTMLEditorRef().RangeUpdaterRef(),
-                                 address_of(endNode), &endOffset);
+                                 &firstRangeEnd);
     // We are handling all ranged deletions directly now.
 
-    if (endNode == startNode) {
+    if (firstRangeStart.GetContainer() == firstRangeEnd.GetContainer()) {
       nsresult rv = MOZ_KnownLive(HTMLEditorRef())
                         .DeleteSelectionWithTransaction(aDirectionAndAmount,
                                                         aStripWrappers);
       if (NS_WARN_IF(!CanHandleEditAction())) {
         return result.SetResult(NS_ERROR_EDITOR_DESTROYED);
       }
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return result.SetResult(rv);
       }
     } else {
       // Figure out mailcite ancestors
       RefPtr<Element> startCiteNode =
-          HTMLEditorRef().GetMostAncestorMailCiteElement(*startNode);
+          HTMLEditorRef().GetMostAncestorMailCiteElement(
+              *firstRangeStart.GetContainer());
       RefPtr<Element> endCiteNode =
-          HTMLEditorRef().GetMostAncestorMailCiteElement(*endNode);
+          HTMLEditorRef().GetMostAncestorMailCiteElement(
+              *firstRangeEnd.GetContainer());
 
       // If we only have a mailcite at one of the two endpoints, set the
       // directionality of the deletion so that the selection will end up
       // outside the mailcite.
       if (startCiteNode && !endCiteNode) {
         aDirectionAndAmount = nsIEditor::eNext;
       } else if (!startCiteNode && endCiteNode) {
         aDirectionAndAmount = nsIEditor::ePrevious;
       }
 
       // Figure out block parents
-      RefPtr<Element> leftBlock = HTMLEditor::GetBlock(*startNode);
-      RefPtr<Element> rightBlock = HTMLEditor::GetBlock(*endNode);
+      RefPtr<Element> leftBlock =
+          HTMLEditor::GetBlock(*firstRangeStart.GetContainer());
+      RefPtr<Element> rightBlock =
+          HTMLEditor::GetBlock(*firstRangeEnd.GetContainer());
 
       // Are endpoint block parents the same?  Use default deletion
       if (leftBlock && leftBlock == rightBlock) {
         MOZ_KnownLive(HTMLEditorRef())
             .DeleteSelectionWithTransaction(aDirectionAndAmount,
                                             aStripWrappers);
         if (NS_WARN_IF(!CanHandleEditAction())) {
           return result.SetResult(NS_ERROR_EDITOR_DESTROYED);
@@ -3271,36 +3283,39 @@ EditActionResult HTMLEditRules::HandleDe
             }
           }
         }
 
         // Check endpoints for possible text deletion.  We can assume that if
         // text node is found, we can delete to end or to begining as
         // appropriate, since the case where both sel endpoints in same text
         // node was already handled (we wouldn't be here)
-        if (startNode->IsText() &&
-            startNode->Length() > static_cast<uint32_t>(startOffset)) {
+        if (firstRangeStart.IsInTextNode() &&
+            !firstRangeStart.IsEndOfContainer()) {
           // Delete to last character
-          OwningNonNull<Text> textNode = *startNode->AsText();
-          nsresult rv =
-              MOZ_KnownLive(HTMLEditorRef())
-                  .DeleteTextWithTransaction(textNode, startOffset,
-                                             startNode->Length() - startOffset);
+          OwningNonNull<Text> textNode = *firstRangeStart.GetContainerAsText();
+          nsresult rv = MOZ_KnownLive(HTMLEditorRef())
+                            .DeleteTextWithTransaction(
+                                textNode, firstRangeStart.Offset(),
+                                firstRangeStart.GetContainer()->Length() -
+                                    firstRangeStart.Offset());
           if (NS_WARN_IF(!CanHandleEditAction())) {
             return result.SetResult(NS_ERROR_EDITOR_DESTROYED);
           }
           if (NS_WARN_IF(NS_FAILED(rv))) {
             return result.SetResult(rv);
           }
         }
-        if (endNode->IsText() && endOffset) {
+        if (firstRangeEnd.IsInTextNode() &&
+            !firstRangeEnd.IsStartOfContainer()) {
           // Delete to first character
-          OwningNonNull<Text> textNode = *endNode->AsText();
+          OwningNonNull<Text> textNode = *firstRangeEnd.GetContainerAsText();
           nsresult rv = MOZ_KnownLive(HTMLEditorRef())
-                            .DeleteTextWithTransaction(textNode, 0, endOffset);
+                            .DeleteTextWithTransaction(textNode, 0,
+                                                       firstRangeEnd.Offset());
           if (NS_WARN_IF(!CanHandleEditAction())) {
             return result.SetResult(NS_ERROR_EDITOR_DESTROYED);
           }
           if (NS_WARN_IF(NS_FAILED(rv))) {
             return result.SetResult(rv);
           }
         }
 
@@ -3314,65 +3329,68 @@ EditActionResult HTMLEditRules::HandleDe
         }
       }
     }
   }
 
   // If we're handling D&D, this is called to delete dragging item from the
   // tree.  In this case, we should move parent blocks if it becomes empty.
   if (HTMLEditorRef().GetEditAction() == EditAction::eDrop) {
-    MOZ_ASSERT(startNode == endNode);
-    MOZ_ASSERT(startOffset == endOffset);
+    MOZ_ASSERT(firstRangeStart.GetContainer() == firstRangeEnd.GetContainer());
+    MOZ_ASSERT(firstRangeStart.Offset() == firstRangeEnd.Offset());
     {
       AutoTrackDOMPoint startTracker(HTMLEditorRef().RangeUpdaterRef(),
-                                     address_of(startNode), &startOffset);
+                                     &firstRangeStart);
       AutoTrackDOMPoint endTracker(HTMLEditorRef().RangeUpdaterRef(),
-                                   address_of(endNode), &endOffset);
-
-      EditorDOMPoint atStart(startNode, startOffset);
-      nsresult rv = MOZ_KnownLive(HTMLEditorRef())
-                        .DeleteParentBlocksWithTransactionIfEmpty(atStart);
+                                   &firstRangeEnd);
+
+      nsresult rv =
+          MOZ_KnownLive(HTMLEditorRef())
+              .DeleteParentBlocksWithTransactionIfEmpty(firstRangeStart);
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return result.SetResult(rv);
       }
       HTMLEditorRef()
           .TopLevelEditSubActionDataRef()
           .mDidDeleteEmptyParentBlocks = rv == NS_OK;
     }
     // If we removed parent blocks, Selection should be collapsed at where
     // the most ancestor empty block has been.
     if (HTMLEditorRef()
             .TopLevelEditSubActionDataRef()
             .mDidDeleteEmptyParentBlocks) {
-      nsresult rv = SelectionRefPtr()->Collapse(startNode, startOffset);
+      nsresult rv =
+          SelectionRefPtr()->Collapse(firstRangeStart.ToRawRangeBoundary());
       if (NS_WARN_IF(!CanHandleEditAction())) {
         return result.SetResult(NS_ERROR_EDITOR_DESTROYED);
       }
       NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "Selection::Collapse() failed");
       return result.SetResult(rv);
     }
   }
 
   // We might have left only collapsed whitespace in the start/end nodes
   {
     AutoTrackDOMPoint startTracker(HTMLEditorRef().RangeUpdaterRef(),
-                                   address_of(startNode), &startOffset);
+                                   &firstRangeStart);
     AutoTrackDOMPoint endTracker(HTMLEditorRef().RangeUpdaterRef(),
-                                 address_of(endNode), &endOffset);
+                                 &firstRangeEnd);
 
     nsresult rv = MOZ_KnownLive(HTMLEditorRef())
-                      .DeleteNodeIfInvisibleAndEditableTextNode(*startNode);
+                      .DeleteNodeIfInvisibleAndEditableTextNode(
+                          MOZ_KnownLive(*firstRangeStart.GetContainer()));
     if (NS_WARN_IF(rv == NS_ERROR_EDITOR_DESTROYED)) {
       return result.SetResult(NS_ERROR_EDITOR_DESTROYED);
     }
     NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
                          "DeleteNodeIfInvisibleAndEditableTextNode() failed to "
                          "remove start node, but ignored");
     rv = MOZ_KnownLive(HTMLEditorRef())
-             .DeleteNodeIfInvisibleAndEditableTextNode(*endNode);
+             .DeleteNodeIfInvisibleAndEditableTextNode(
+                 MOZ_KnownLive(*firstRangeEnd.GetContainer()));
     if (NS_WARN_IF(rv == NS_ERROR_EDITOR_DESTROYED)) {
       return result.SetResult(NS_ERROR_EDITOR_DESTROYED);
     }
     NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
                          "DeleteNodeIfInvisibleAndEditableTextNode() failed to "
                          "remove end node, but ignored");
   }
 
@@ -3380,25 +3398,27 @@ EditActionResult HTMLEditRules::HandleDe
   // collapsed to the end of the selection, if deleting backward the selection
   // should be collapsed to the beginning of the selection. But if we're not
   // joining then the selection should collapse to the beginning of the
   // selection if we'redeleting forward, because the end of the selection will
   // still be in the next block. And same thing for deleting backwards
   // (selection should collapse to the end, because the beginning will still be
   // in the first block). See Bug 507936
   if (aDirectionAndAmount == (join ? nsIEditor::eNext : nsIEditor::ePrevious)) {
-    nsresult rv = SelectionRefPtr()->Collapse(endNode, endOffset);
+    nsresult rv =
+        SelectionRefPtr()->Collapse(firstRangeEnd.ToRawRangeBoundary());
     if (NS_WARN_IF(!CanHandleEditAction())) {
       return result.SetResult(NS_ERROR_EDITOR_DESTROYED);
     }
     NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "Selection::Collapse() failed");
     return result.SetResult(rv);
   }
 
-  nsresult rv = SelectionRefPtr()->Collapse(startNode, startOffset);
+  nsresult rv =
+      SelectionRefPtr()->Collapse(firstRangeStart.ToRawRangeBoundary());
   if (NS_WARN_IF(!CanHandleEditAction())) {
     return result.SetResult(NS_ERROR_EDITOR_DESTROYED);
   }
   NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "Selection::Collapse() failed");
   return result.SetResult(rv);
 }
 
 nsresult HTMLEditor::DeleteNodeIfInvisibleAndEditableTextNode(nsINode& aNode) {
--- a/editor/libeditor/SelectionState.h
+++ b/editor/libeditor/SelectionState.h
@@ -204,16 +204,29 @@ class MOZ_STACK_CLASS AutoTrackDOMPoint 
     mRangeItem->mStartOffset = mPoint->Offset();
     mRangeItem->mEndOffset = mPoint->Offset();
     mRangeUpdater.RegisterRangeItem(mRangeItem);
   }
 
   ~AutoTrackDOMPoint() {
     mRangeUpdater.DropRangeItem(mRangeItem);
     if (mPoint) {
+      // Setting `mPoint` with invalid DOM point causes hitting `NS_ASSERTION()`
+      // and the number of times may be too many.  (E.g., 1533913.html hits
+      // over 700 times!)  We should just put warning instead.
+      if (NS_WARN_IF(!mRangeItem->mStartContainer) ||
+          NS_WARN_IF(mRangeItem->mStartOffset < 0)) {
+        mPoint->Clear();
+        return;
+      }
+      if (NS_WARN_IF(mRangeItem->mStartContainer->Length() <
+                     static_cast<uint32_t>(mRangeItem->mStartOffset))) {
+        mPoint->SetToEndOf(mRangeItem->mStartContainer);
+        return;
+      }
       mPoint->Set(mRangeItem->mStartContainer, mRangeItem->mStartOffset);
       return;
     }
     *mNode = mRangeItem->mStartContainer;
     *mOffset = mRangeItem->mStartOffset;
   }
 };