Bug 1191356 part 7 - Clean up nsHTMLEditor::GetSelectionContainer; r=ehsan
☠☠ backed out by 211e5067895e ☠ ☠
authorAryeh Gregor <ayg@aryeh.name>
Sun, 01 May 2016 16:17:39 +0300
changeset 295658 aa61db1f8f832ce51b8970c6acf27c9132bde8e7
parent 295657 6dbba6c4a200ac06bdbfab8fce401a2ea8e0e00c
child 295659 7b3284a488dc93e0b3a94fa4ce1c280460a582dc
push id19015
push usercbook@mozilla.com
push dateMon, 02 May 2016 09:39:23 +0000
treeherderfx-team@2080375bc69d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan
bugs1191356
milestone49.0a1
Bug 1191356 part 7 - Clean up nsHTMLEditor::GetSelectionContainer; r=ehsan This makes the XPCOM GetSelectionContainer return an error in cases where previously it would return success but a null pointer. This could theoretically cause problems, but there's no obvious non-ugly way to avoid it.
editor/libeditor/nsHTMLAbsPosition.cpp
editor/libeditor/nsHTMLEditor.cpp
editor/libeditor/nsHTMLEditor.h
--- a/editor/libeditor/nsHTMLAbsPosition.cpp
+++ b/editor/libeditor/nsHTMLAbsPosition.cpp
@@ -78,38 +78,34 @@ nsHTMLEditor::AbsolutePositionSelection(
     return res;
 
   return mRules->DidDoAction(selection, &ruleInfo, res);
 }
 
 NS_IMETHODIMP
 nsHTMLEditor::GetAbsolutelyPositionedSelectionContainer(nsIDOMElement **_retval)
 {
-  nsCOMPtr<nsIDOMElement> element;
-  nsresult res = GetSelectionContainer(getter_AddRefs(element));
-  NS_ENSURE_SUCCESS(res, res);
-
   nsAutoString positionStr;
-  nsCOMPtr<nsINode> node = do_QueryInterface(element);
+  nsCOMPtr<nsINode> node = GetSelectionContainer();
   nsCOMPtr<nsIDOMNode> resultNode;
 
   while (!resultNode && node && !node->IsHTMLElement(nsGkAtoms::html)) {
-    res = mHTMLCSSUtils->GetComputedProperty(*node, *nsGkAtoms::position,
-                                             positionStr);
+    nsresult res =
+      mHTMLCSSUtils->GetComputedProperty(*node, *nsGkAtoms::position,
+                                         positionStr);
     NS_ENSURE_SUCCESS(res, res);
     if (positionStr.EqualsLiteral("absolute"))
       resultNode = GetAsDOMNode(node);
     else {
       node = node->GetParentNode();
     }
   }
 
-  element = do_QueryInterface(resultNode );
-  *_retval = element;
-  NS_IF_ADDREF(*_retval);
+  nsCOMPtr<nsIDOMElement> element = do_QueryInterface(resultNode);
+  element.forget(_retval);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsHTMLEditor::GetSelectionContainerAbsolutelyPositioned(bool *aIsSelectionContainerAbsolutelyPositioned)
 {
   *aIsSelectionContainerAbsolutelyPositioned = (mAbsolutelyPositionedObject != nullptr);
   return NS_OK;
--- a/editor/libeditor/nsHTMLEditor.cpp
+++ b/editor/libeditor/nsHTMLEditor.cpp
@@ -4802,99 +4802,84 @@ nsHTMLEditor::EndUpdateViewBatch()
     RefPtr<Selection> selection = GetSelection();
     NS_ENSURE_TRUE(selection, NS_ERROR_NOT_INITIALIZED);
     res = CheckSelectionStateForAnonymousButtons(selection);
   }
   return res;
 }
 
 NS_IMETHODIMP
-nsHTMLEditor::GetSelectionContainer(nsIDOMElement ** aReturn)
+nsHTMLEditor::GetSelectionContainer(nsIDOMElement** aReturn)
+{
+  nsCOMPtr<nsIDOMElement> container =
+    static_cast<nsIDOMElement*>(GetAsDOMNode(GetSelectionContainer()));
+  NS_ENSURE_TRUE(container, NS_ERROR_FAILURE);
+  container.forget(aReturn);
+  return NS_OK;
+}
+
+Element*
+nsHTMLEditor::GetSelectionContainer()
 {
-  RefPtr<Selection> selection = GetSelection();
-  // if we don't get the selection, just skip this
-  if (!selection) {
-    return NS_ERROR_FAILURE;
-  }
-
-  nsCOMPtr<nsIDOMNode> focusNode;
-
-  nsresult res;
+  // If we don't get the selection, just skip this
+  NS_ENSURE_TRUE(GetSelection(), nullptr);
+
+  OwningNonNull<Selection> selection = *GetSelection();
+
+  nsCOMPtr<nsINode> focusNode;
+
   if (selection->Collapsed()) {
-    res = selection->GetFocusNode(getter_AddRefs(focusNode));
-    NS_ENSURE_SUCCESS(res, res);
+    focusNode = selection->GetFocusNode();
   } else {
-
-    int32_t rangeCount;
-    res = selection->GetRangeCount(&rangeCount);
-    NS_ENSURE_SUCCESS(res, res);
+    int32_t rangeCount = selection->RangeCount();
 
     if (rangeCount == 1) {
-
       RefPtr<nsRange> range = selection->GetRangeAt(0);
-      NS_ENSURE_TRUE(range, NS_ERROR_NULL_POINTER);
-
-      nsCOMPtr<nsIDOMNode> startContainer, endContainer;
-      res = range->GetStartContainer(getter_AddRefs(startContainer));
-      NS_ENSURE_SUCCESS(res, res);
-      res = range->GetEndContainer(getter_AddRefs(endContainer));
-      NS_ENSURE_SUCCESS(res, res);
-      int32_t startOffset, endOffset;
-      res = range->GetStartOffset(&startOffset);
-      NS_ENSURE_SUCCESS(res, res);
-      res = range->GetEndOffset(&endOffset);
-      NS_ENSURE_SUCCESS(res, res);
-
-      nsCOMPtr<nsIDOMElement> focusElement;
+
+      nsCOMPtr<nsINode> startContainer = range->GetStartParent();
+      int32_t startOffset = range->StartOffset();
+      nsCOMPtr<nsINode> endContainer = range->GetEndParent();
+      int32_t endOffset = range->EndOffset();
+
       if (startContainer == endContainer && startOffset + 1 == endOffset) {
-        res = GetSelectedElement(EmptyString(), getter_AddRefs(focusElement));
-        NS_ENSURE_SUCCESS(res, res);
-        if (focusElement)
+        nsCOMPtr<nsIDOMElement> focusElement;
+        nsresult res = GetSelectedElement(EmptyString(),
+                                          getter_AddRefs(focusElement));
+        NS_ENSURE_SUCCESS(res, nullptr);
+        if (focusElement) {
           focusNode = do_QueryInterface(focusElement);
+        }
       }
       if (!focusNode) {
-        res = range->GetCommonAncestorContainer(getter_AddRefs(focusNode));
-        NS_ENSURE_SUCCESS(res, res);
+        focusNode = range->GetCommonAncestor();
       }
-    }
-    else {
-      int32_t i;
-      RefPtr<nsRange> range;
-      for (i = 0; i < rangeCount; i++)
-      {
-        range = selection->GetRangeAt(i);
-        NS_ENSURE_STATE(range);
-        nsCOMPtr<nsIDOMNode> startContainer;
-        res = range->GetStartContainer(getter_AddRefs(startContainer));
-        if (NS_FAILED(res)) continue;
-        if (!focusNode)
+    } else {
+      for (int32_t i = 0; i < rangeCount; i++) {
+        RefPtr<nsRange> range = selection->GetRangeAt(i);
+
+        nsCOMPtr<nsINode> startContainer = range->GetStartParent();
+        if (!focusNode) {
           focusNode = startContainer;
-        else if (focusNode != startContainer) {
-          res = startContainer->GetParentNode(getter_AddRefs(focusNode));
-          NS_ENSURE_SUCCESS(res, res);
+        } else if (focusNode != startContainer) {
+          focusNode = startContainer->GetParentNode();
           break;
         }
       }
     }
   }
 
-  if (focusNode) {
-    uint16_t nodeType;
-    focusNode->GetNodeType(&nodeType);
-    if (nsIDOMNode::TEXT_NODE == nodeType) {
-      nsCOMPtr<nsIDOMNode> parent;
-      res = focusNode->GetParentNode(getter_AddRefs(parent));
-      NS_ENSURE_SUCCESS(res, res);
-      focusNode = parent;
-    }
-  }
-
-  nsCOMPtr<nsIDOMElement> focusElement = do_QueryInterface(focusNode);
-  focusElement.forget(aReturn);
-  return NS_OK;
+  if (focusNode && focusNode->GetAsText()) {
+    focusNode = focusNode->GetParentNode();
+  }
+
+  if (focusNode && focusNode->IsElement()) {
+    return focusNode->AsElement();
+  }
+
+  return nullptr;
 }
 
 NS_IMETHODIMP
 nsHTMLEditor::IsAnonymousElement(nsIDOMElement * aElement, bool * aReturn)
 {
   NS_ENSURE_TRUE(aElement, NS_ERROR_NULL_POINTER);
   nsCOMPtr<nsIContent> content = do_QueryInterface(aElement);
   *aReturn = content->IsRootOfNativeAnonymousSubtree();
--- a/editor/libeditor/nsHTMLEditor.h
+++ b/editor/libeditor/nsHTMLEditor.h
@@ -96,16 +96,17 @@ public:
 // another class. Only the base class should use NS_DECL_ISUPPORTS
   NS_DECL_ISUPPORTS_INHERITED
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(nsHTMLEditor, nsPlaintextEditor)
 
 
   nsHTMLEditor();
 
   bool GetReturnInParagraphCreatesNewParagraph();
+  Element* GetSelectionContainer();
 
   /* ------------ nsPlaintextEditor overrides -------------- */
   NS_IMETHOD GetIsDocumentEditable(bool *aIsDocumentEditable) override;
   NS_IMETHOD BeginningOfDocument() override;
   virtual nsresult HandleKeyPressEvent(nsIDOMKeyEvent* aKeyEvent) override;
   virtual already_AddRefed<nsIContent> GetFocusedContent() override;
   virtual already_AddRefed<nsIContent> GetFocusedContentForIME() override;
   virtual bool IsActiveInDOMWindow() override;