Bug 1319340 - part6: Implement some interface methods as non-virtual methods of EditorBase or HTMLEditor r=m_kato

nsComposerCommands uses some simple getter methods. They can be simpler non-virtual methods. So, we should do it. Note that this changes that EditorBase::GetIsSelectionEditable() won't return error. However, it has returned error only when selection controller isn't available. That means that the selection controller has been destroyed and the editor will be destroyed. So, this must not be problem since it returns false (non-editable) instead and won't break any behavior since the editor won't be editable by users nor JS anymore. MozReview-Commit-ID: E9ccFspG6na --HG-- extra : rebase_source : bcd1314cb386fcaf175adabfefde5885decd87c0
2017-08-04 18:30:13 +09:00 · 2017-08-04 18:30:13 +09:00 · b5b8d1064d
--- a/editor/composer/nsComposerCommands.cpp
+++ b/editor/composer/nsComposerCommands.cpp
@ -85,7 +85,8 @@ nsBaseStateUpdatingCommand::IsCommandEnabled(const char *aCommandName,
  }
  mozilla::EditorBase* editorBase = editor->AsEditorBase();
  MOZ_ASSERT(editorBase);
-  return editorBase->GetIsSelectionEditable(outCmdEnabled);
+  *outCmdEnabled = editorBase->IsSelectionEditable();
+  return NS_OK;
 }


@ -420,10 +421,7 @@ nsRemoveListCommand::IsCommandEnabled(const char * aCommandName,
  mozilla::EditorBase* editorBase = editor->AsEditorBase();
  MOZ_ASSERT(editorBase);

-  bool isEditable = false;
-  nsresult rv = editorBase->GetIsSelectionEditable(&isEditable);
-  NS_ENSURE_SUCCESS(rv, rv);
-  if (!isEditable) {
+  if (!editorBase->IsSelectionEditable()) {
    return NS_OK;
  }

@ -435,7 +433,7 @@ nsRemoveListCommand::IsCommandEnabled(const char * aCommandName,

  bool bMixed;
  nsAutoString localName;
-  rv = GetListState(htmlEditor, &bMixed, localName);
+  nsresult rv = GetListState(htmlEditor, &bMixed, localName);
  NS_ENSURE_SUCCESS(rv, rv);

  *outCmdEnabled = bMixed || !localName.IsEmpty();
@ -487,7 +485,8 @@ nsIndentCommand::IsCommandEnabled(const char * aCommandName,
  }
  mozilla::EditorBase* editorBase = editor->AsEditorBase();
  MOZ_ASSERT(editorBase);
-  return editorBase->GetIsSelectionEditable(outCmdEnabled);
+  *outCmdEnabled = editorBase->IsSelectionEditable();
+  return NS_OK;
 }

 NS_IMETHODIMP
@ -537,7 +536,8 @@ nsOutdentCommand::IsCommandEnabled(const char * aCommandName,
  }
  mozilla::EditorBase* editorBase = editor->AsEditorBase();
  MOZ_ASSERT(editorBase);
-  return editorBase->GetIsSelectionEditable(outCmdEnabled);
+  *outCmdEnabled = editorBase->IsSelectionEditable();
+  return NS_OK;
 }

 NS_IMETHODIMP
@ -596,7 +596,8 @@ nsMultiStateCommand::IsCommandEnabled(const char * aCommandName,
  mozilla::EditorBase* editorBase = editor->AsEditorBase();
  MOZ_ASSERT(editorBase);
  // should be disabled sometimes, like if the current selection is an image
-  return editorBase->GetIsSelectionEditable(outCmdEnabled);
+  *outCmdEnabled = editorBase->IsSelectionEditable();
+  return NS_OK;
 }

 NS_IMETHODIMP
@ -906,10 +907,10 @@ nsHighlightColorStateCommand::IsCommandEnabled(const char * aCommandName,
  }
  mozilla::EditorBase* editorBase = editor->AsEditorBase();
  MOZ_ASSERT(editorBase);
-  return editorBase->GetIsSelectionEditable(outCmdEnabled);
+  *outCmdEnabled = editorBase->IsSelectionEditable();
+  return NS_OK;
 }

-
 nsBackgroundColorStateCommand::nsBackgroundColorStateCommand()
 : nsMultiStateCommand()
 {
@ -1020,15 +1021,11 @@ nsAbsolutePositioningCommand::IsCommandEnabled(const char * aCommandName,
  if (!htmlEditor) {
    return NS_OK;
  }
-  bool isEditable = false;
-  nsresult rv = htmlEditor->GetIsSelectionEditable(&isEditable);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
-  }
-  if (!isEditable) {
+  if (!htmlEditor->IsSelectionEditable()) {
    return NS_OK;
  }
-  return htmlEditor->GetAbsolutePositioningEnabled(outCmdEnabled);
+  *outCmdEnabled = htmlEditor->AbsolutePositioningEnabled();
+  return NS_OK;
 }

 nsresult
@ -1039,22 +1036,23 @@ nsAbsolutePositioningCommand::GetCurrentState(mozilla::HTMLEditor* aHTMLEditor,
    return NS_ERROR_INVALID_ARG;
  }

-  bool isEnabled;
-  aHTMLEditor->GetAbsolutePositioningEnabled(&isEnabled);
+  bool isEnabled = aHTMLEditor->AbsolutePositioningEnabled();
  if (!isEnabled) {
    aParams->SetBooleanValue(STATE_MIXED,false);
    aParams->SetCStringValue(STATE_ATTRIBUTE, "");
    return NS_OK;
  }

-  nsCOMPtr<nsIDOMElement>  elt;
+  nsCOMPtr<nsINode> container;
  nsresult rv =
-    aHTMLEditor->GetAbsolutelyPositionedSelectionContainer(getter_AddRefs(elt));
+    aHTMLEditor->GetAbsolutelyPositionedSelectionContainer(
+                   getter_AddRefs(container));
  NS_ENSURE_SUCCESS(rv, rv);

  nsAutoString outStateString;
-  if (elt)
+  if (container) {
    outStateString.AssignLiteral("absolute");
+  }

  aParams->SetBooleanValue(STATE_MIXED,false);
  aParams->SetCStringValue(STATE_ATTRIBUTE, NS_ConvertUTF16toUTF8(outStateString).get());
@ -1068,12 +1066,13 @@ nsAbsolutePositioningCommand::ToggleState(mozilla::HTMLEditor* aHTMLEditor)
    return NS_ERROR_INVALID_ARG;
  }

-  nsCOMPtr<nsIDOMElement> elt;
+  nsCOMPtr<nsINode> container;
  nsresult rv =
-    aHTMLEditor->GetAbsolutelyPositionedSelectionContainer(getter_AddRefs(elt));
+    aHTMLEditor->GetAbsolutelyPositionedSelectionContainer(
+                   getter_AddRefs(container));
  NS_ENSURE_SUCCESS(rv, rv);

-  return aHTMLEditor->AbsolutePositionSelection(!elt);
+  return aHTMLEditor->AbsolutePositionSelection(!container);
 }


@ -1091,12 +1090,11 @@ nsDecreaseZIndexCommand::IsCommandEnabled(const char * aCommandName,
    return NS_ERROR_FAILURE;
  }

-  htmlEditor->GetAbsolutePositioningEnabled(outCmdEnabled);
+  *outCmdEnabled = htmlEditor->AbsolutePositioningEnabled();
  if (!(*outCmdEnabled))
    return NS_OK;

-  nsCOMPtr<nsIDOMElement> positionedElement;
-  htmlEditor->GetPositionedElement(getter_AddRefs(positionedElement));
+  RefPtr<Element> positionedElement = htmlEditor->GetPositionedElement();
  *outCmdEnabled = false;
  if (!positionedElement) {
    return NS_OK;
@ -1162,12 +1160,11 @@ nsIncreaseZIndexCommand::IsCommandEnabled(const char * aCommandName,
    return NS_ERROR_FAILURE;
  }

-  htmlEditor->GetAbsolutePositioningEnabled(outCmdEnabled);
+  *outCmdEnabled = htmlEditor->AbsolutePositioningEnabled();
  if (!(*outCmdEnabled))
    return NS_OK;

-  nsCOMPtr<nsIDOMElement> positionedElement;
-  htmlEditor->GetPositionedElement(getter_AddRefs(positionedElement));
+  Element* positionedElement = htmlEditor->GetPositionedElement();
  *outCmdEnabled = (nullptr != positionedElement);
  return NS_OK;
 }
@ -1222,7 +1219,8 @@ nsRemoveStylesCommand::IsCommandEnabled(const char * aCommandName,
  mozilla::EditorBase* editorBase = editor->AsEditorBase();
  MOZ_ASSERT(editorBase);
  // test if we have any styles?
-  return editorBase->GetIsSelectionEditable(outCmdEnabled);
+  *outCmdEnabled = editorBase->IsSelectionEditable();
+  return NS_OK;
 }

 NS_IMETHODIMP
@ -1271,7 +1269,8 @@ nsIncreaseFontSizeCommand::IsCommandEnabled(const char * aCommandName,
  mozilla::EditorBase* editorBase = editor->AsEditorBase();
  MOZ_ASSERT(editorBase);
  // test if we are at max size?
-  return editorBase->GetIsSelectionEditable(outCmdEnabled);
+  *outCmdEnabled = editorBase->IsSelectionEditable();
+  return NS_OK;
 }


@ -1321,7 +1320,8 @@ nsDecreaseFontSizeCommand::IsCommandEnabled(const char * aCommandName,
  mozilla::EditorBase* editorBase = editor->AsEditorBase();
  MOZ_ASSERT(editorBase);
  // test if we are at min size?
-  return editorBase->GetIsSelectionEditable(outCmdEnabled);
+  *outCmdEnabled = editorBase->IsSelectionEditable();
+  return NS_OK;
 }

 NS_IMETHODIMP
@ -1370,7 +1370,8 @@ nsInsertHTMLCommand::IsCommandEnabled(const char * aCommandName,
  }
  mozilla::EditorBase* editorBase = editor->AsEditorBase();
  MOZ_ASSERT(editorBase);
-  return editorBase->GetIsSelectionEditable(outCmdEnabled);
+  *outCmdEnabled = editorBase->IsSelectionEditable();
+  return NS_OK;
 }

 NS_IMETHODIMP
@ -1456,7 +1457,8 @@ nsInsertTagCommand::IsCommandEnabled(const char * aCommandName,
  }
  mozilla::EditorBase* editorBase = editor->AsEditorBase();
  MOZ_ASSERT(editorBase);
-  return editorBase->GetIsSelectionEditable(outCmdEnabled);
+  *outCmdEnabled = editorBase->IsSelectionEditable();
+  return NS_OK;
 }

 // corresponding STATE_ATTRIBUTE is: src (img) and href (a)
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@ -543,17 +543,43 @@ NS_IMETHODIMP
 EditorBase::GetIsSelectionEditable(bool* aIsSelectionEditable)
 {
  NS_ENSURE_ARG_POINTER(aIsSelectionEditable);
+  *aIsSelectionEditable = IsSelectionEditable();
+  return NS_OK;
+}

+bool
+EditorBase::IsSelectionEditable()
+{
  // get current selection
  RefPtr<Selection> selection = GetSelection();
-  NS_ENSURE_TRUE(selection, NS_ERROR_NULL_POINTER);
+  if (NS_WARN_IF(!selection)) {
+    return false;
+  }

-  // XXX we just check that the anchor node is editable at the moment
-  //     we should check that all nodes in the selection are editable
-  nsCOMPtr<nsINode> anchorNode = selection->GetAnchorNode();
-  *aIsSelectionEditable = anchorNode && IsEditable(anchorNode);
+  if (!mIsHTMLEditorClass) {
+    // XXX we just check that the anchor node is editable at the moment
+    //     we should check that all nodes in the selection are editable
+    nsCOMPtr<nsINode> anchorNode = selection->GetAnchorNode();
+    return anchorNode && IsEditable(anchorNode);
+  }

-  return NS_OK;
+  // Per the editing spec as of June 2012: we have to have a selection whose
+  // start and end nodes are editable, and which share an ancestor editing
+  // host.  (Bug 766387.)
+  bool isSelectionEditable = selection->RangeCount() &&
+                             selection->GetAnchorNode()->IsEditable() &&
+                             selection->GetFocusNode()->IsEditable();
+  if (!isSelectionEditable) {
+    return false;
+  }
+
+  nsINode* commonAncestor =
+    selection->GetAnchorFocusRange()->GetCommonAncestor();
+  while (commonAncestor && !commonAncestor->IsEditable()) {
+    commonAncestor = commonAncestor->GetParentNode();
+  }
+  // If there is no editable common ancestor, return false.
+  return !!commonAncestor;
 }

 NS_IMETHODIMP
--- a/editor/libeditor/EditorBase.h
+++ b/editor/libeditor/EditorBase.h
@ -780,6 +780,14 @@ public:
    }
  }

+  /**
+   * Returns true if selection is in an editable element and both the range
+   * start and the range end are editable.  E.g., even if the selection range
+   * includes non-editable elements, returns true when one of common ancestors
+   * of the range start and the range end is editable.  Otherwise, false.
+   */
+  bool IsSelectionEditable();
+
  /**
   * Returns true if aNode is a MozEditorBogus node.
   */
--- a/editor/libeditor/HTMLAbsPositionEditor.cpp
+++ b/editor/libeditor/HTMLAbsPositionEditor.cpp
@ -80,24 +80,43 @@ HTMLEditor::AbsolutePositionSelection(bool aEnabled)
 NS_IMETHODIMP
 HTMLEditor::GetAbsolutelyPositionedSelectionContainer(nsIDOMElement** _retval)
 {
+  nsCOMPtr<nsINode> container;
+  nsresult rv =
+    GetAbsolutelyPositionedSelectionContainer(getter_AddRefs(container));
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    *_retval = nullptr;
+    return rv;
+  }
+
+  nsCOMPtr<nsIDOMElement> domContainer = do_QueryInterface(container);
+  domContainer.forget(_retval);
+  return NS_OK;
+}
+
+nsresult
+HTMLEditor::GetAbsolutelyPositionedSelectionContainer(nsINode** aContainer)
+{
+  MOZ_ASSERT(aContainer);
+
  nsAutoString positionStr;
  nsCOMPtr<nsINode> node = GetSelectionContainer();
-  nsCOMPtr<nsIDOMNode> resultNode;
+  nsCOMPtr<nsINode> resultNode;

  while (!resultNode && node && !node->IsHTMLElement(nsGkAtoms::html)) {
    nsresult rv =
      mCSSEditUtils->GetComputedProperty(*node, *nsGkAtoms::position,
                                         positionStr);
-    NS_ENSURE_SUCCESS(rv, rv);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      *aContainer = nullptr;
+      return rv;
+    }
    if (positionStr.EqualsLiteral("absolute"))
-      resultNode = GetAsDOMNode(node);
+      resultNode = node;
    else {
      node = node->GetParentNode();
    }
  }
-
-  nsCOMPtr<nsIDOMElement> element = do_QueryInterface(resultNode);
-  element.forget(_retval);
+  resultNode.forget(aContainer);
  return NS_OK;
 }

@ -112,7 +131,7 @@ HTMLEditor::GetSelectionContainerAbsolutelyPositioned(
 NS_IMETHODIMP
 HTMLEditor::GetAbsolutePositioningEnabled(bool* aIsEnabled)
 {
-  *aIsEnabled = mIsAbsolutelyPositioningEnabled;
+  *aIsEnabled = AbsolutePositioningEnabled();
  return NS_OK;
 }

@ -189,21 +208,28 @@ HTMLEditor::GetElementZIndex(nsIDOMElement* aElement,
                             int32_t* aZindex)
 {
  nsCOMPtr<Element> element = do_QueryInterface(aElement);
-  NS_ENSURE_STATE(element || !aElement);
+  return GetElementZIndex(element, aZindex);
+}
+
+nsresult
+HTMLEditor::GetElementZIndex(Element* aElement,
+                             int32_t* aZindex)
+{
+  if (NS_WARN_IF(!aElement)) {
+    return NS_ERROR_INVALID_ARG;
+  }
+
  nsAutoString zIndexStr;
  *aZindex = 0;

  nsresult rv =
-    mCSSEditUtils->GetSpecifiedProperty(*element, *nsGkAtoms::z_index,
+    mCSSEditUtils->GetSpecifiedProperty(*aElement, *nsGkAtoms::z_index,
                                        zIndexStr);
  NS_ENSURE_SUCCESS(rv, rv);
  if (zIndexStr.EqualsLiteral("auto")) {
    // we have to look at the positioned ancestors
    // cf. CSS 2 spec section 9.9.1
-    nsCOMPtr<nsIDOMNode> parentNode;
-    rv = aElement->GetParentNode(getter_AddRefs(parentNode));
-    NS_ENSURE_SUCCESS(rv, rv);
-    nsCOMPtr<nsINode> node = do_QueryInterface(parentNode);
+    nsCOMPtr<nsINode> node = aElement->GetParentNode();
    nsAutoString positionStr;
    while (node && zIndexStr.EqualsLiteral("auto") &&
           !node->IsHTMLElement(nsGkAtoms::body)) {
@ -597,7 +623,7 @@ NS_IMETHODIMP
 HTMLEditor::GetPositionedElement(nsIDOMElement** aReturn)
 {
  nsCOMPtr<nsIDOMElement> ret =
-    static_cast<nsIDOMElement*>(GetAsDOMNode(mAbsolutelyPositionedObject));
+    static_cast<nsIDOMElement*>(GetAsDOMNode(GetPositionedElement()));
  ret.forget(aReturn);
  return NS_OK;
 }
--- a/editor/libeditor/HTMLEditor.cpp
+++ b/editor/libeditor/HTMLEditor.cpp
@ -3355,36 +3355,6 @@ HTMLEditor::IsModifiableNode(nsINode* aNode)
  return !aNode || aNode->IsEditable();
 }

-NS_IMETHODIMP
-HTMLEditor::GetIsSelectionEditable(bool* aIsSelectionEditable)
-{
-  MOZ_ASSERT(aIsSelectionEditable);
-
-  RefPtr<Selection> selection = GetSelection();
-  NS_ENSURE_TRUE(selection, NS_ERROR_NULL_POINTER);
-
-  // Per the editing spec as of June 2012: we have to have a selection whose
-  // start and end nodes are editable, and which share an ancestor editing
-  // host.  (Bug 766387.)
-  *aIsSelectionEditable = selection->RangeCount() &&
-                          selection->GetAnchorNode()->IsEditable() &&
-                          selection->GetFocusNode()->IsEditable();
-
-  if (*aIsSelectionEditable) {
-    nsINode* commonAncestor =
-      selection->GetAnchorFocusRange()->GetCommonAncestor();
-    while (commonAncestor && !commonAncestor->IsEditable()) {
-      commonAncestor = commonAncestor->GetParentNode();
-    }
-    if (!commonAncestor) {
-      // No editable common ancestor
-      *aIsSelectionEditable = false;
-    }
-  }
-
-  return NS_OK;
-}
-
 static nsresult
 SetSelectionAroundHeadChildren(Selection* aSelection,
                               nsCOMPtr<nsIDocument>& aDocument)
--- a/editor/libeditor/HTMLEditor.h
+++ b/editor/libeditor/HTMLEditor.h
@ -217,6 +217,18 @@ public:
  static bool NodeIsBlockStatic(const nsINode* aElement);
  static nsresult NodeIsBlockStatic(nsIDOMNode *aNode, bool *aIsBlock);

+  // non-virtual methods of interface methods
+  bool AbsolutePositioningEnabled() const
+  {
+    return mIsAbsolutelyPositioningEnabled;
+  }
+  nsresult GetAbsolutelyPositionedSelectionContainer(nsINode** aContainer);
+  Element* GetPositionedElement() const
+  {
+    return mAbsolutelyPositionedObject;
+  }
+  nsresult GetElementZIndex(Element* aElement, int32_t* aZindex);
+
 protected:
  virtual ~HTMLEditor();

@ -288,8 +300,6 @@ public:
  NS_IMETHOD_(bool) IsModifiableNode(nsIDOMNode* aNode) override;
  virtual bool IsModifiableNode(nsINode* aNode) override;

-  NS_IMETHOD GetIsSelectionEditable(bool* aIsSelectionEditable) override;
-
  NS_IMETHOD SelectAll() override;

  NS_IMETHOD GetRootElement(nsIDOMElement** aRootElement) override;