Bug 1154701 part 13 - Clean up nsHTMLEditor::SetCSSBackgroundColor; r=ehsan
☠☠ backed out by 329dd852c06b ☠ ☠
authorAryeh Gregor <ayg@aryeh.name>
Wed, 22 Apr 2015 14:27:18 +0300
changeset 240529 4fd3566e571c53b5709aa804f44a5bf0b9ae8502
parent 240528 9801778d9d5b9478f6902c8549f93ff0da9e7058
child 240530 e64fa8717641988cf63c5d63e240a4a895ca732c
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)
reviewersehsan
bugs1154701
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 1154701 part 13 - Clean up nsHTMLEditor::SetCSSBackgroundColor; r=ehsan
editor/libeditor/nsHTMLEditor.cpp
--- a/editor/libeditor/nsHTMLEditor.cpp
+++ b/editor/libeditor/nsHTMLEditor.cpp
@@ -4498,206 +4498,166 @@ nsHTMLEditor::SetIsCSSEnabled(bool aIsCS
 
   return SetFlags(flags);
 }
 
 // Set the block background color
 nsresult
 nsHTMLEditor::SetCSSBackgroundColor(const nsAString& aColor)
 {
-  if (!mRules) { return NS_ERROR_NOT_INITIALIZED; }
+  NS_ENSURE_TRUE(mRules, NS_ERROR_NOT_INITIALIZED);
   ForceCompositionEnd();
 
   // Protect the edit rules object from dying
   nsCOMPtr<nsIEditRules> kungFuDeathGrip(mRules);
 
   nsRefPtr<Selection> selection = GetSelection();
+  NS_ENSURE_STATE(selection);
 
   bool isCollapsed = selection->Collapsed();
 
   nsAutoEditBatch batchIt(this);
-  nsAutoRules beginRulesSniffing(this, EditAction::insertElement, nsIEditor::eNext);
+  nsAutoRules beginRulesSniffing(this, EditAction::insertElement,
+                                 nsIEditor::eNext);
   nsAutoSelectionReset selectionResetter(selection, this);
   nsAutoTxnsConserveSelection dontSpazMySelection(this);
-  
+
   bool cancel, handled;
   nsTextRulesInfo ruleInfo(EditAction::setTextProperty);
   nsresult res = mRules->WillDoAction(selection, &ruleInfo, &cancel, &handled);
   NS_ENSURE_SUCCESS(res, res);
-  if (!cancel && !handled)
-  {
-    // loop thru the ranges in the selection
-    nsAutoString bgcolor; bgcolor.AssignLiteral("bgcolor");
-    uint32_t rangeCount = selection->RangeCount();
-    for (uint32_t rangeIdx = 0; rangeIdx < rangeCount; ++rangeIdx) {
-      nsCOMPtr<nsIDOMNode> cachedBlockParent = nullptr;
-      nsRefPtr<nsRange> range = selection->GetRangeAt(rangeIdx);
+  if (!cancel && !handled) {
+    // Loop through the ranges in the selection
+    NS_NAMED_LITERAL_STRING(bgcolor, "bgcolor");
+    for (uint32_t i = 0; i < selection->RangeCount(); i++) {
+      nsRefPtr<nsRange> range = selection->GetRangeAt(i);
       NS_ENSURE_TRUE(range, NS_ERROR_FAILURE);
-      
-      // check for easy case: both range endpoints in same text node
-      nsCOMPtr<nsIDOMNode> startNode, endNode;
-      int32_t startOffset, endOffset;
-      res = range->GetStartContainer(getter_AddRefs(startNode));
-      NS_ENSURE_SUCCESS(res, res);
-      res = range->GetEndContainer(getter_AddRefs(endNode));
-      NS_ENSURE_SUCCESS(res, res);
-      res = range->GetStartOffset(&startOffset);
-      NS_ENSURE_SUCCESS(res, res);
-      res = range->GetEndOffset(&endOffset);
-      NS_ENSURE_SUCCESS(res, res);
-      if ((startNode == endNode) && IsTextNode(startNode))
-      {
-        // let's find the block container of the text node
-        nsCOMPtr<nsIDOMNode> blockParent;
-        blockParent = GetBlockNodeParent(startNode);
-        // and apply the background color to that block container
+
+      nsCOMPtr<Element> cachedBlockParent;
+
+      // Check for easy case: both range endpoints in same text node
+      nsCOMPtr<nsINode> startNode = range->GetStartParent();
+      int32_t startOffset = range->StartOffset();
+      nsCOMPtr<nsINode> endNode = range->GetEndParent();
+      int32_t endOffset = range->EndOffset();
+      if (startNode == endNode && IsTextNode(startNode)) {
+        // Let's find the block container of the text node
+        nsCOMPtr<Element> blockParent = GetBlockNodeParent(startNode);
+        // And apply the background color to that block container
         if (blockParent && cachedBlockParent != blockParent) {
           cachedBlockParent = blockParent;
-          nsCOMPtr<nsIDOMElement> element = do_QueryInterface(blockParent);
-          int32_t count;
-          res = mHTMLCSSUtils->SetCSSEquivalentToHTMLStyle(element, nullptr, &bgcolor, &aColor, &count, false);
-          NS_ENSURE_SUCCESS(res, res);
+          mHTMLCSSUtils->SetCSSEquivalentToHTMLStyle(blockParent, nullptr,
+                                                     &bgcolor, &aColor, false);
         }
-      }
-      else if ((startNode == endNode) && nsTextEditUtils::IsBody(startNode) && isCollapsed)
-      {
-        // we have no block in the document, let's apply the background to the body 
-        nsCOMPtr<nsIDOMElement> element = do_QueryInterface(startNode);
-        int32_t count;
-        res = mHTMLCSSUtils->SetCSSEquivalentToHTMLStyle(element, nullptr, &bgcolor, &aColor, &count, false);
-        NS_ENSURE_SUCCESS(res, res);
-      }
-      else if ((startNode == endNode) && (((endOffset-startOffset) == 1) || (!startOffset && !endOffset)))
-      {
-        // a unique node is selected, let's also apply the background color
-        // to the containing block, possibly the node itself
-        nsCOMPtr<nsIDOMNode> selectedNode = GetChildAt(startNode, startOffset);
-        bool isBlock =false;
-        res = NodeIsBlockStatic(selectedNode, &isBlock);
-        NS_ENSURE_SUCCESS(res, res);
-        nsCOMPtr<nsIDOMNode> blockParent = selectedNode;
-        if (!isBlock) {
+      } else if (startNode == endNode &&
+                 startNode->IsHTMLElement(nsGkAtoms::body) && isCollapsed) {
+        // No block in the document, let's apply the background to the body
+        mHTMLCSSUtils->SetCSSEquivalentToHTMLStyle(startNode->AsElement(),
+                                                   nullptr, &bgcolor, &aColor,
+                                                   false);
+      } else if (startNode == endNode && (endOffset - startOffset == 1 ||
+                                          (!startOffset && !endOffset))) {
+        // A unique node is selected, let's also apply the background color to
+        // the containing block, possibly the node itself
+        nsCOMPtr<nsIContent> selectedNode = startNode->GetChildAt(startOffset);
+        nsCOMPtr<Element> blockParent;
+        if (NodeIsBlockStatic(selectedNode)) {
+          blockParent = selectedNode->AsElement();
+        } else {
           blockParent = GetBlockNodeParent(selectedNode);
         }
         if (blockParent && cachedBlockParent != blockParent) {
           cachedBlockParent = blockParent;
-          nsCOMPtr<nsIDOMElement> element = do_QueryInterface(blockParent);
-          int32_t count;
-          res = mHTMLCSSUtils->SetCSSEquivalentToHTMLStyle(element, nullptr, &bgcolor, &aColor, &count, false);
-          NS_ENSURE_SUCCESS(res, res);
+          mHTMLCSSUtils->SetCSSEquivalentToHTMLStyle(blockParent, nullptr,
+                                                     &bgcolor, &aColor, false);
         }
-      }
-      else
-      {
-        // not the easy case.  range not contained in single text node. 
-        // there are up to three phases here.  There are all the nodes
-        // reported by the subtree iterator to be processed.  And there
-        // are potentially a starting textnode and an ending textnode
-        // which are only partially contained by the range.
-        
-        // lets handle the nodes reported by the iterator.  These nodes
-        // are entirely contained in the selection range.  We build up
-        // a list of them (since doing operations on the document during
-        // iteration would perturb the iterator).
-
-        nsCOMPtr<nsIContentIterator> iter =
-          do_CreateInstance("@mozilla.org/content/subtree-content-iterator;1", &res);
-        NS_ENSURE_SUCCESS(res, res);
-        NS_ENSURE_TRUE(iter, NS_ERROR_FAILURE);
-
-        nsCOMArray<nsIDOMNode> arrayOfNodes;
-        nsCOMPtr<nsIDOMNode> node;
-                
-        // iterate range and build up array
+      } else {
+        // Not the easy case.  Range not contained in single text node.  There
+        // are up to three phases here.  There are all the nodes reported by
+        // the subtree iterator to be processed.  And there are potentially a
+        // starting textnode and an ending textnode which are only partially
+        // contained by the range.
+
+        // Let's handle the nodes reported by the iterator.  These nodes are
+        // entirely contained in the selection range.  We build up a list of
+        // them (since doing operations on the document during iteration would
+        // perturb the iterator).
+
+        OwningNonNull<nsIContentIterator> iter =
+          NS_NewContentSubtreeIterator();
+
+        nsTArray<OwningNonNull<nsINode>> arrayOfNodes;
+        nsCOMPtr<nsINode> node;
+
+        // Iterate range and build up array
         res = iter->Init(range);
-        // init returns an error if no nodes in range.
-        // this can easily happen with the subtree 
-        // iterator if the selection doesn't contain
-        // any *whole* nodes.
-        if (NS_SUCCEEDED(res))
-        {
-          while (!iter->IsDone())
-          {
+        // Init returns an error if no nodes in range.  This can easily happen
+        // with the subtree iterator if the selection doesn't contain any
+        // *whole* nodes.
+        if (NS_SUCCEEDED(res)) {
+          for (; !iter->IsDone(); iter->Next()) {
             node = do_QueryInterface(iter->GetCurrentNode());
             NS_ENSURE_TRUE(node, NS_ERROR_FAILURE);
 
-            if (IsEditable(node))
-            {
-              arrayOfNodes.AppendObject(node);
+            if (IsEditable(node)) {
+              arrayOfNodes.AppendElement(*node);
             }
-
-            iter->Next();
           }
         }
-        // first check the start parent of the range to see if it needs to 
-        // be separately handled (it does if it's a text node, due to how the
+        // First check the start parent of the range to see if it needs to be
+        // separately handled (it does if it's a text node, due to how the
         // subtree iterator works - it will not have reported it).
-        if (IsTextNode(startNode) && IsEditable(startNode))
-        {
-          nsCOMPtr<nsIDOMNode> blockParent;
-          blockParent = GetBlockNodeParent(startNode);
+        if (IsTextNode(startNode) && IsEditable(startNode)) {
+          nsCOMPtr<Element> blockParent = GetBlockNodeParent(startNode);
           if (blockParent && cachedBlockParent != blockParent) {
             cachedBlockParent = blockParent;
-            nsCOMPtr<nsIDOMElement> element = do_QueryInterface(blockParent);
-            int32_t count;
-            res = mHTMLCSSUtils->SetCSSEquivalentToHTMLStyle(element, nullptr, &bgcolor, &aColor, &count, false);
-            NS_ENSURE_SUCCESS(res, res);
+            mHTMLCSSUtils->SetCSSEquivalentToHTMLStyle(blockParent, nullptr,
+                                                       &bgcolor, &aColor,
+                                                       false);
           }
         }
-        
-        // then loop through the list, set the property on each node
-        int32_t listCount = arrayOfNodes.Count();
-        int32_t j;
-        for (j = 0; j < listCount; j++)
-        {
-          node = arrayOfNodes[j];
-          // do we have a block here ?
-          bool isBlock =false;
-          res = NodeIsBlockStatic(node, &isBlock);
-          NS_ENSURE_SUCCESS(res, res);
-          nsCOMPtr<nsIDOMNode> blockParent = node;
-          if (!isBlock) {
-            // no we don't, let's find the block ancestor
+
+        // Then loop through the list, set the property on each node
+        for (auto& node : arrayOfNodes) {
+          nsCOMPtr<Element> blockParent;
+          if (NodeIsBlockStatic(node)) {
+            blockParent = node->AsElement();
+          } else {
             blockParent = GetBlockNodeParent(node);
           }
           if (blockParent && cachedBlockParent != blockParent) {
             cachedBlockParent = blockParent;
-            nsCOMPtr<nsIDOMElement> element = do_QueryInterface(blockParent);
-            int32_t count;
-            // and set the property on it
-            res = mHTMLCSSUtils->SetCSSEquivalentToHTMLStyle(element, nullptr, &bgcolor, &aColor, &count, false);
-            NS_ENSURE_SUCCESS(res, res);
+            mHTMLCSSUtils->SetCSSEquivalentToHTMLStyle(blockParent, nullptr,
+                                                       &bgcolor, &aColor,
+                                                       false);
           }
         }
         arrayOfNodes.Clear();
-        
-        // last check the end parent of the range to see if it needs to 
-        // be separately handled (it does if it's a text node, due to how the
+
+        // Last, check the end parent of the range to see if it needs to be
+        // separately handled (it does if it's a text node, due to how the
         // subtree iterator works - it will not have reported it).
-        if (IsTextNode(endNode) && IsEditable(endNode))
-        {
-          nsCOMPtr<nsIDOMNode> blockParent;
-          blockParent = GetBlockNodeParent(endNode);
+        if (IsTextNode(endNode) && IsEditable(endNode)) {
+          nsCOMPtr<Element> blockParent = GetBlockNodeParent(endNode);
           if (blockParent && cachedBlockParent != blockParent) {
             cachedBlockParent = blockParent;
-            nsCOMPtr<nsIDOMElement> element = do_QueryInterface(blockParent);
-            int32_t count;
-            res = mHTMLCSSUtils->SetCSSEquivalentToHTMLStyle(element, nullptr, &bgcolor, &aColor, &count, false);
-            NS_ENSURE_SUCCESS(res, res);
+            mHTMLCSSUtils->SetCSSEquivalentToHTMLStyle(blockParent, nullptr,
+                                                       &bgcolor, &aColor,
+                                                       false);
           }
         }
       }
     }
   }
-  if (!cancel)
-  {
-    // post-process
+  if (!cancel) {
+    // Post-process
     res = mRules->DidDoAction(selection, &ruleInfo, res);
+    NS_ENSURE_SUCCESS(res, res);
   }
-  return res;
+  return NS_OK;
 }
 
 NS_IMETHODIMP
 nsHTMLEditor::SetBackgroundColor(const nsAString& aColor)
 {
   nsresult res;
   if (IsCSSEnabled()) {
     // if we are in CSS mode, we have to apply the background color to the