Bug 1149163 part 7 - Clean up nsHTMLEditRules::PromoteRange; r=froydnj
☠☠ backed out by 329dd852c06b ☠ ☠
authorAryeh Gregor <ayg@aryeh.name>
Wed, 22 Apr 2015 14:26:57 +0300
changeset 240502 a31a87b6dfb7ada1f9ac7b05df6c83b7e156486a
parent 240501 e36d2f2512768e8b6af269a9d0c27fc70d12984b
child 240503 54a8ecc0301d40bf1fdefa195802bd36a20a55e7
push id28636
push userkwierso@gmail.com
push dateThu, 23 Apr 2015 00:16:12 +0000
treeherdermozilla-central@a5af73b32ac8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1149163
milestone40.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1149163 part 7 - Clean up nsHTMLEditRules::PromoteRange; r=froydnj
editor/libeditor/nsHTMLEditRules.cpp
editor/libeditor/nsHTMLEditRules.h
--- a/editor/libeditor/nsHTMLEditRules.cpp
+++ b/editor/libeditor/nsHTMLEditRules.cpp
@@ -442,18 +442,17 @@ nsHTMLEditRules::AfterEditInner(EditActi
   if (bDamagedRange && !((action == EditAction::undo) || (action == EditAction::redo)))
   {
     // don't let any txns in here move the selection around behind our back.
     // Note that this won't prevent explicit selection setting from working.
     NS_ENSURE_STATE(mHTMLEditor);
     nsAutoTxnsConserveSelection dontSpazMySelection(mHTMLEditor);
    
     // expand the "changed doc range" as needed
-    res = PromoteRange(mDocChangeRange, action);
-    NS_ENSURE_SUCCESS(res, res);
+    PromoteRange(*mDocChangeRange, action);
 
     // if we did a ranged deletion or handling backspace key, make sure we have
     // a place to put caret.
     // Note we only want to do this if the overall operation was deletion,
     // not if deletion was done along the way for EditAction::loadHTML, EditAction::insertText, etc.
     // That's why this is here rather than DidDeleteSelection().
     if ((action == EditAction::deleteSelection) && mDidRangedDelete)
     {
@@ -5763,115 +5762,95 @@ nsHTMLEditRules::GetPromotedRanges(Selec
 
     // clone range so we don't muck with actual selection ranges
     opRange = selectionRange->CloneRange();
 
     // make a new adjusted range to represent the appropriate block content.
     // The basic idea is to push out the range endpoints
     // to truly enclose the blocks that we will affect.
     // This call alters opRange.
-    res = PromoteRange(opRange, inOperationType);
-    NS_ENSURE_SUCCESS(res, res);
+    PromoteRange(*opRange, inOperationType);
       
     // stuff new opRange into array
     outArrayOfRanges.AppendElement(opRange);
   }
   return res;
 }
 
 
-///////////////////////////////////////////////////////////////////////////
-// PromoteRange: expand a range to include any parents for which all
-//               editable children are already in range. 
-//                       
-nsresult 
-nsHTMLEditRules::PromoteRange(nsRange* inRange, EditAction inOperationType)
-{
-  NS_ENSURE_TRUE(inRange, NS_ERROR_NULL_POINTER);
-  nsresult res;
-  nsCOMPtr<nsIDOMNode> startNode, endNode;
-  int32_t startOffset, endOffset;
-  
-  res = inRange->GetStartContainer(getter_AddRefs(startNode));
-  NS_ENSURE_SUCCESS(res, res);
-  res = inRange->GetStartOffset(&startOffset);
-  NS_ENSURE_SUCCESS(res, res);
-  res = inRange->GetEndContainer(getter_AddRefs(endNode));
-  NS_ENSURE_SUCCESS(res, res);
-  res = inRange->GetEndOffset(&endOffset);
-  NS_ENSURE_SUCCESS(res, res);
-  
+///////////////////////////////////////////////////////////////////////////////
+// PromoteRange: Expand a range to include any parents for which all editable
+//               children are already in range.
+//
+void
+nsHTMLEditRules::PromoteRange(nsRange& aRange, EditAction aOperationType)
+{
+  NS_ENSURE_TRUE(mHTMLEditor, );
+  nsCOMPtr<nsIEditor> kungFuDeathGrip(mHTMLEditor);
+
+  nsCOMPtr<nsINode> startNode = aRange.GetStartParent();
+  nsCOMPtr<nsINode> endNode = aRange.GetEndParent();
+  int32_t startOffset = aRange.StartOffset();
+  int32_t endOffset = aRange.EndOffset();
+
   // MOOSE major hack:
   // GetPromotedPoint doesn't really do the right thing for collapsed ranges
   // inside block elements that contain nothing but a solo <br>.  It's easier
   // to put a workaround here than to revamp GetPromotedPoint.  :-(
-  if ( (startNode == endNode) && (startOffset == endOffset))
-  {
-    nsCOMPtr<nsIDOMNode> block;
-    if (IsBlockNode(startNode)) {
-      block = startNode;
+  if (startNode == endNode && startOffset == endOffset) {
+    nsCOMPtr<Element> block;
+    if (IsBlockNode(GetAsDOMNode(startNode))) {
+      block = startNode->AsElement();
     } else {
-      NS_ENSURE_STATE(mHTMLEditor);
       block = mHTMLEditor->GetBlockNodeParent(startNode);
     }
-    if (block)
-    {
+    if (block) {
       bool bIsEmptyNode = false;
-      // check for the editing host
-      NS_ENSURE_STATE(mHTMLEditor);
-      nsIContent *rootContent = mHTMLEditor->GetActiveEditingHost();
-      nsCOMPtr<nsINode> rootNode = do_QueryInterface(rootContent);
-      nsCOMPtr<nsINode> blockNode = do_QueryInterface(block);
-      NS_ENSURE_TRUE(rootNode && blockNode, NS_ERROR_UNEXPECTED);
+      nsCOMPtr<nsIContent> root = mHTMLEditor->GetActiveEditingHost();
       // Make sure we don't go higher than our root element in the content tree
-      if (!nsContentUtils::ContentIsDescendantOf(rootNode, blockNode))
-      {
-        NS_ENSURE_STATE(mHTMLEditor);
-        res = mHTMLEditor->IsEmptyNode(block, &bIsEmptyNode, true, false);
-      }
-      if (bIsEmptyNode)
-      {
-        uint32_t numChildren;
-        nsEditor::GetLengthOfDOMNode(block, numChildren); 
+      NS_ENSURE_TRUE(root, );
+      if (!nsContentUtils::ContentIsDescendantOf(root, block)) {
+        mHTMLEditor->IsEmptyNode(block, &bIsEmptyNode, true, false);
+      }
+      if (bIsEmptyNode) {
         startNode = block;
         endNode = block;
         startOffset = 0;
-        endOffset = numChildren;
-      }
-    }
-  }
-
-  // make a new adjusted range to represent the appropriate block content.
-  // this is tricky.  the basic idea is to push out the range endpoints
-  // to truly enclose the blocks that we will affect
-  
+        endOffset = block->Length();
+      }
+    }
+  }
+
+  // Make a new adjusted range to represent the appropriate block content.
+  // This is tricky.  The basic idea is to push out the range endpoints to
+  // truly enclose the blocks that we will affect.
+
   nsCOMPtr<nsIDOMNode> opStartNode;
   nsCOMPtr<nsIDOMNode> opEndNode;
   int32_t opStartOffset, opEndOffset;
   nsRefPtr<nsRange> opRange;
-  
-  GetPromotedPoint(kStart, startNode, startOffset, inOperationType,
-                   address_of(opStartNode), &opStartOffset);
-  GetPromotedPoint(kEnd, endNode, endOffset, inOperationType,
+
+  GetPromotedPoint(kStart, GetAsDOMNode(startNode), startOffset,
+                   aOperationType, address_of(opStartNode), &opStartOffset);
+  GetPromotedPoint(kEnd, GetAsDOMNode(endNode), endOffset, aOperationType,
                    address_of(opEndNode), &opEndOffset);
 
   // Make sure that the new range ends up to be in the editable section.
-  NS_ENSURE_STATE(mHTMLEditor);
-  if (!mHTMLEditor->IsDescendantOfEditorRoot(nsEditor::GetNodeAtRangeOffsetPoint(opStartNode, opStartOffset)) ||
-      !mHTMLEditor || // Check again, since it may have gone away
-      !mHTMLEditor->IsDescendantOfEditorRoot(nsEditor::GetNodeAtRangeOffsetPoint(opEndNode, opEndOffset - 1))) {
-    NS_ENSURE_STATE(mHTMLEditor);
-    return NS_OK;
-  }
-
-  res = inRange->SetStart(opStartNode, opStartOffset);
-  NS_ENSURE_SUCCESS(res, res);
-  res = inRange->SetEnd(opEndNode, opEndOffset);
-  return res;
-} 
+  if (!mHTMLEditor->IsDescendantOfEditorRoot(
+        nsEditor::GetNodeAtRangeOffsetPoint(opStartNode, opStartOffset)) ||
+      !mHTMLEditor->IsDescendantOfEditorRoot(
+        nsEditor::GetNodeAtRangeOffsetPoint(opEndNode, opEndOffset - 1))) {
+    return;
+  }
+
+  DebugOnly<nsresult> res = aRange.SetStart(opStartNode, opStartOffset);
+  MOZ_ASSERT(NS_SUCCEEDED(res));
+  res = aRange.SetEnd(opEndNode, opEndOffset);
+  MOZ_ASSERT(NS_SUCCEEDED(res));
+}
 
 class NodeComparator
 {
   public:
     bool Equals(const nsINode* node, const nsIDOMNode* domNode) const
     {
       return domNode == GetAsDOMNode(const_cast<nsINode*>(node));
     }
@@ -6037,33 +6016,30 @@ nsHTMLEditRules::GetChildNodesForOperati
       return NS_ERROR_FAILURE;
     }
   }
   return NS_OK;
 }
 
 
 
-///////////////////////////////////////////////////////////////////////////
-// GetListActionNodes: 
-//                       
-nsresult 
-nsHTMLEditRules::GetListActionNodes(nsCOMArray<nsIDOMNode> &outArrayOfNodes, 
+nsresult
+nsHTMLEditRules::GetListActionNodes(nsCOMArray<nsIDOMNode> &outArrayOfNodes,
                                     bool aEntireList,
                                     bool aDontTouchContent)
 {
   nsresult res = NS_OK;
-  
+
   NS_ENSURE_STATE(mHTMLEditor);
   nsRefPtr<Selection> selection = mHTMLEditor->GetSelection();
   NS_ENSURE_TRUE(selection, NS_ERROR_FAILURE);
   // added this in so that ui code can ask to change an entire list, even if selection
   // is only in part of it.  used by list item dialog.
   if (aEntireList)
-  {       
+  {
     uint32_t rangeCount = selection->RangeCount();
     for (uint32_t rangeIdx = 0; rangeIdx < rangeCount; ++rangeIdx) {
       nsRefPtr<nsRange> range = selection->GetRangeAt(rangeIdx);
       nsCOMPtr<nsIDOMNode> commonParent, parent, tmp;
       range->GetCommonAncestorContainer(getter_AddRefs(commonParent));
       if (commonParent)
       {
         parent = commonParent;
@@ -6089,31 +6065,31 @@ nsHTMLEditRules::GetListActionNodes(nsCO
     NS_ENSURE_STATE(mHTMLEditor);
     nsAutoTxnsConserveSelection dontSpazMySelection(mHTMLEditor);
 
     // contruct a list of nodes to act on.
     res = GetNodesFromSelection(selection, EditAction::makeList,
                                 outArrayOfNodes, aDontTouchContent);
     NS_ENSURE_SUCCESS(res, res);
   }
-               
-  // pre process our list of nodes...                      
+
+  // pre process our list of nodes...
   int32_t listCount = outArrayOfNodes.Count();
   int32_t i;
   for (i=listCount-1; i>=0; i--)
   {
     nsCOMPtr<nsIDOMNode> testNode = outArrayOfNodes[i];
 
     // Remove all non-editable nodes.  Leave them be.
     NS_ENSURE_STATE(mHTMLEditor);
     if (!mHTMLEditor->IsEditable(testNode))
     {
       outArrayOfNodes.RemoveObjectAt(i);
     }
-    
+
     // scan for table elements and divs.  If we find table elements other than table,
     // replace it with a list of any editable non-table content.
     if (nsHTMLEditUtils::IsTableElementButNotTable(testNode))
     {
       int32_t j=i;
       outArrayOfNodes.RemoveObjectAt(i);
       res = GetInnerContent(testNode, outArrayOfNodes, &j, false);
       NS_ENSURE_SUCCESS(res, res);
@@ -6397,18 +6373,17 @@ nsHTMLEditRules::GetNodesFromPoint(::DOM
                                    bool dontTouchContent)
 {
   NS_ENSURE_STATE(point.node);
   nsRefPtr<nsRange> range = new nsRange(point.node);
   nsresult res = range->SetStart(point.node, point.offset);
   NS_ENSURE_SUCCESS(res, res);
   
   // expand the range to include adjacent inlines
-  res = PromoteRange(range, operation);
-  NS_ENSURE_SUCCESS(res, res);
+  PromoteRange(*range, operation);
       
   // make array of ranges
   nsTArray<nsRefPtr<nsRange>> arrayOfRanges;
   
   // stuff new opRange into array
   arrayOfRanges.AppendElement(range);
   
   // use these ranges to contruct a list of nodes to act on.
--- a/editor/libeditor/nsHTMLEditRules.h
+++ b/editor/libeditor/nsHTMLEditRules.h
@@ -270,17 +270,17 @@ protected:
   bool IsLastNode(nsIDOMNode *aNode);
   nsresult NormalizeSelection(mozilla::dom::Selection* aSelection);
   void GetPromotedPoint(RulesEndpoint aWhere, nsIDOMNode* aNode,
                         int32_t aOffset, EditAction actionID,
                         nsCOMPtr<nsIDOMNode>* outNode, int32_t* outOffset);
   nsresult GetPromotedRanges(mozilla::dom::Selection* aSelection, 
                              nsTArray<nsRefPtr<nsRange>>& outArrayOfRanges,
                              EditAction inOperationType);
-  nsresult PromoteRange(nsRange* inRange, EditAction inOperationType);
+  void PromoteRange(nsRange& aRange, EditAction inOperationType);
   enum class TouchContent { no, yes };
   nsresult GetNodesForOperation(nsTArray<nsRefPtr<nsRange>>& aArrayOfRanges,
                                 nsTArray<nsCOMPtr<nsINode>>& aOutArrayOfNodes,
                                 EditAction aOperationType,
                                 TouchContent aTouchContent = TouchContent::yes);
   nsresult GetChildNodesForOperation(nsIDOMNode *inNode, 
                                      nsCOMArray<nsIDOMNode>& outArrayOfNodes);
   nsresult GetNodesFromPoint(::DOMPoint point,