Bug 1480663 - Make EditorBase::IsModifiableNode() non-virtual r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 03 Aug 2018 11:10:46 +0000
changeset 485080 7ae524e2f7746b2260163d6e3decfba3c3100a54
parent 485079 947673e320105d665b4a205803f2a2171399d767
child 485081 c0b095a6a2c93620a65721f6eb63005760ede156
push id9719
push userffxbld-merge
push dateFri, 24 Aug 2018 17:49:46 +0000
treeherdermozilla-beta@719ec98fba77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1480663
milestone63.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 1480663 - Make EditorBase::IsModifiableNode() non-virtual r=m_kato HTMLEditor::IsModifiableNode() is enough simple and can be checked in EditorBase. So, we should make it non-virtual and check if instance is HTMLEditor in EditorBase::IsModifiableNode(). Differential Revision: https://phabricator.services.mozilla.com/D2706
editor/libeditor/DeleteNodeTransaction.cpp
editor/libeditor/DeleteTextTransaction.cpp
editor/libeditor/EditorBase.cpp
editor/libeditor/EditorBase.h
editor/libeditor/HTMLAnonymousNodeEditor.cpp
editor/libeditor/HTMLEditRules.cpp
editor/libeditor/HTMLEditor.cpp
editor/libeditor/HTMLEditor.h
editor/libeditor/JoinNodeTransaction.cpp
editor/libeditor/TextEditRules.cpp
--- a/editor/libeditor/DeleteNodeTransaction.cpp
+++ b/editor/libeditor/DeleteNodeTransaction.cpp
@@ -47,17 +47,17 @@ NS_IMPL_ADDREF_INHERITED(DeleteNodeTrans
 NS_IMPL_RELEASE_INHERITED(DeleteNodeTransaction, EditTransactionBase)
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(DeleteNodeTransaction)
 NS_INTERFACE_MAP_END_INHERITING(EditTransactionBase)
 
 bool
 DeleteNodeTransaction::CanDoIt() const
 {
   if (NS_WARN_IF(!mNodeToDelete) || NS_WARN_IF(!mEditorBase) ||
-      !mParentNode || !mEditorBase->IsModifiableNode(mParentNode)) {
+      !mParentNode || !mEditorBase->IsModifiableNode(*mParentNode)) {
     return false;
   }
   return true;
 }
 
 NS_IMETHODIMP
 DeleteNodeTransaction::DoTransaction()
 {
--- a/editor/libeditor/DeleteTextTransaction.cpp
+++ b/editor/libeditor/DeleteTextTransaction.cpp
@@ -107,17 +107,17 @@ NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(
 NS_INTERFACE_MAP_END_INHERITING(EditTransactionBase)
 
 bool
 DeleteTextTransaction::CanDoIt() const
 {
   if (NS_WARN_IF(!mCharData) || NS_WARN_IF(!mEditorBase)) {
     return false;
   }
-  return mEditorBase->IsModifiableNode(mCharData);
+  return mEditorBase->IsModifiableNode(*mCharData);
 }
 
 NS_IMETHODIMP
 DeleteTextTransaction::DoTransaction()
 {
   if (NS_WARN_IF(!mEditorBase) || NS_WARN_IF(!mCharData)) {
     return NS_ERROR_NOT_AVAILABLE;
   }
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -4949,19 +4949,19 @@ EditorBase::SetTextDirectionTo(TextDirec
     }
     return NS_OK;
   }
 
   return NS_OK;
 }
 
 bool
-EditorBase::IsModifiableNode(nsINode* aNode)
+EditorBase::IsModifiableNode(const nsINode& aNode) const
 {
-  return true;
+  return !AsHTMLEditor() || aNode.IsEditable();
 }
 
 nsIContent*
 EditorBase::GetFocusedContent()
 {
   EventTarget* piTarget = GetDOMEventTarget();
   if (!piTarget) {
     return nullptr;
--- a/editor/libeditor/EditorBase.h
+++ b/editor/libeditor/EditorBase.h
@@ -1436,20 +1436,22 @@ protected: // May be called by friends.
    */
   virtual bool IsContainer(nsINode* aNode);
 
   /**
    * returns true if aNode is an editable node.
    */
   bool IsEditable(nsINode* aNode)
   {
-    NS_ENSURE_TRUE(aNode, false);
+    if (NS_WARN_IF(!aNode)) {
+      return false;
+    }
 
     if (!aNode->IsContent() || IsMozEditorBogusNode(aNode) ||
-        !IsModifiableNode(aNode)) {
+        !IsModifiableNode(*aNode)) {
       return false;
     }
 
     switch (aNode->NodeType()) {
       case nsINode::ELEMENT_NODE:
         // In HTML editors, if we're dealing with an element, then ask it
         // whether it's editable.
         return mIsHTMLEditorClass ? aNode->IsEditable() : true;
@@ -1506,17 +1508,20 @@ protected: // May be called by friends.
    */
   virtual bool AreNodesSameType(nsIContent* aNode1, nsIContent* aNode2);
 
   static bool IsTextNode(nsINode* aNode)
   {
     return aNode->NodeType() == nsINode::TEXT_NODE;
   }
 
-  virtual bool IsModifiableNode(nsINode* aNode);
+  /**
+   * IsModifiableNode() checks whether the node is editable or not.
+   */
+  bool IsModifiableNode(const nsINode& aNode) const;
 
   /**
    * GetNodeAtRangeOffsetPoint() returns the node at this position in a range,
    * assuming that the container is the node itself if it's a text node, or
    * the node's parent otherwise.
    */
   static nsIContent* GetNodeAtRangeOffsetPoint(nsINode* aContainer,
                                                int32_t aOffset)
--- a/editor/libeditor/HTMLAnonymousNodeEditor.cpp
+++ b/editor/libeditor/HTMLAnonymousNodeEditor.cpp
@@ -373,17 +373,17 @@ HTMLEditor::CheckSelectionStateForAnonym
     NS_ENSURE_SUCCESS(rv, rv);
     NS_ASSERTION(!mInlineEditedCell, "HideInlineTableEditingUI failed");
   }
 
   // now, let's display all contextual UI for good
   nsIContent* hostContent = GetActiveEditingHost();
 
   if (mIsObjectResizingEnabled && focusElement &&
-      IsModifiableNode(focusElement) && focusElement != hostContent) {
+      IsModifiableNode(*focusElement) && focusElement != hostContent) {
     if (nsGkAtoms::img == focusTagAtom) {
       mResizedObjectIsAnImage = true;
     }
     if (mResizedObject) {
       nsresult rv = RefreshResizers();
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }
@@ -391,32 +391,32 @@ HTMLEditor::CheckSelectionStateForAnonym
       nsresult rv = ShowResizers(*focusElement);
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }
     }
   }
 
   if (mIsAbsolutelyPositioningEnabled && absPosElement &&
-      IsModifiableNode(absPosElement) && absPosElement != hostContent) {
+      IsModifiableNode(*absPosElement) && absPosElement != hostContent) {
     if (mAbsolutelyPositionedObject) {
       nsresult rv = RefreshGrabber();
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }
     } else {
       nsresult rv = ShowGrabber(*absPosElement);
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }
     }
   }
 
   if (mIsInlineTableEditingEnabled && cellElement &&
-      IsModifiableNode(cellElement) && cellElement != hostContent) {
+      IsModifiableNode(*cellElement) && cellElement != hostContent) {
     if (mInlineEditedCell) {
       nsresult rv = RefreshInlineTableEditingUI();
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }
     } else {
       nsresult rv = ShowInlineTableEditingUI(cellElement);
       if (NS_WARN_IF(NS_FAILED(rv))) {
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -655,32 +655,41 @@ HTMLEditRules::WillDoAction(Selection* a
 
   // Nothing to do if there's no selection to act on
   if (NS_WARN_IF(!SelectionRef().RangeCount())) {
     return NS_OK;
   }
 
   RefPtr<nsRange> range = SelectionRef().GetRangeAt(0);
   nsCOMPtr<nsINode> selStartNode = range->GetStartContainer();
-
-  if (!HTMLEditorRef().IsModifiableNode(selStartNode)) {
+  if (NS_WARN_IF(!selStartNode)) {
+    return NS_ERROR_FAILURE;
+  }
+
+  if (!HTMLEditorRef().IsModifiableNode(*selStartNode)) {
     *aCancel = true;
     return NS_OK;
   }
 
   nsCOMPtr<nsINode> selEndNode = range->GetEndContainer();
+  if (NS_WARN_IF(!selEndNode)) {
+    return NS_ERROR_FAILURE;
+  }
 
   if (selStartNode != selEndNode) {
-    if (!HTMLEditorRef().IsModifiableNode(selEndNode)) {
+    if (!HTMLEditorRef().IsModifiableNode(*selEndNode)) {
       *aCancel = true;
       return NS_OK;
     }
 
-    NS_ENSURE_STATE(mHTMLEditor);
-    if (!HTMLEditorRef().IsModifiableNode(range->GetCommonAncestor())) {
+    nsINode* commonAncestor = range->GetCommonAncestor();
+    if (NS_WARN_IF(!commonAncestor)) {
+      return NS_ERROR_FAILURE;
+    }
+    if (!HTMLEditorRef().IsModifiableNode(*commonAncestor)) {
       *aCancel = true;
       return NS_OK;
     }
   }
 
   switch (aInfo.mEditSubAction) {
     case EditSubAction::eInsertText:
     case EditSubAction::eInsertTextComingFromIME:
@@ -1744,17 +1753,17 @@ HTMLEditRules::WillInsertBreak(bool* aCa
 
   EditorDOMPoint atStartOfSelection(firstRange->StartRef());
   if (NS_WARN_IF(!atStartOfSelection.IsSet())) {
     return NS_ERROR_FAILURE;
   }
   MOZ_ASSERT(atStartOfSelection.IsSetAndValid());
 
   // Do nothing if the node is read-only
-  if (!HTMLEditorRef().IsModifiableNode(atStartOfSelection.GetContainer())) {
+  if (!HTMLEditorRef().IsModifiableNode(*atStartOfSelection.GetContainer())) {
     *aCancel = true;
     return NS_OK;
   }
 
   // If the active editing host is an inline element, or if the active editing
   // host is the block parent itself and we're configured to use <br> as a
   // paragraph separator, just append a <br>.
   RefPtr<Element> host = HTMLEditorRef().GetActiveEditingHost();
--- a/editor/libeditor/HTMLEditor.cpp
+++ b/editor/libeditor/HTMLEditor.cpp
@@ -3185,17 +3185,17 @@ nsresult
 HTMLEditor::DeleteNodeWithTransaction(nsINode& aNode)
 {
   if (NS_WARN_IF(!aNode.IsContent())) {
     return NS_ERROR_INVALID_ARG;
   }
   // Do nothing if the node is read-only.
   // XXX This is not a override method of EditorBase's method.  This might
   //     cause not called accidentally.  We need to investigate this issue.
-  if (NS_WARN_IF(!IsModifiableNode(aNode.AsContent()) &&
+  if (NS_WARN_IF(!IsModifiableNode(*aNode.AsContent()) &&
                  !IsMozEditorBogusNode(aNode.AsContent()))) {
     return NS_ERROR_FAILURE;
   }
   nsresult rv = EditorBase::DeleteNodeWithTransaction(aNode);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
   return NS_OK;
@@ -3215,17 +3215,17 @@ HTMLEditor::DeleteNode(nsINode* aNode)
 }
 
 nsresult
 HTMLEditor::DeleteTextWithTransaction(CharacterData& aCharData,
                                       uint32_t aOffset,
                                       uint32_t aLength)
 {
   // Do nothing if the node is read-only
-  if (!IsModifiableNode(&aCharData)) {
+  if (!IsModifiableNode(aCharData)) {
     return NS_ERROR_FAILURE;
   }
 
   return EditorBase::DeleteTextWithTransaction(aCharData, aOffset, aLength);
 }
 
 nsresult
 HTMLEditor::InsertTextWithTransaction(
@@ -3234,17 +3234,17 @@ HTMLEditor::InsertTextWithTransaction(
               const EditorRawDOMPoint& aPointToInsert,
               EditorRawDOMPoint* aPointAfterInsertedString)
 {
   if (NS_WARN_IF(!aPointToInsert.IsSet())) {
     return NS_ERROR_INVALID_ARG;
   }
 
   // Do nothing if the node is read-only
-  if (!IsModifiableNode(aPointToInsert.GetContainer())) {
+  if (!IsModifiableNode(*aPointToInsert.GetContainer())) {
     return NS_ERROR_FAILURE;
   }
 
   return EditorBase::InsertTextWithTransaction(aDocument, aStringToInsert,
                                                aPointToInsert,
                                                aPointAfterInsertedString);
 }
 
@@ -3353,22 +3353,16 @@ HTMLEditor::ContentRemoved(nsIContent* a
       return;
     }
     // Protect the edit rules object from dying
     RefPtr<TextEditRules> rules(mRules);
     rules->DocumentModified();
   }
 }
 
-bool
-HTMLEditor::IsModifiableNode(nsINode* aNode)
-{
-  return !aNode || aNode->IsEditable();
-}
-
 NS_IMETHODIMP
 HTMLEditor::DebugUnitTests(int32_t* outNumTests,
                            int32_t* outNumTestsFailed)
 {
 #ifdef DEBUG
   NS_ENSURE_TRUE(outNumTests && outNumTestsFailed, NS_ERROR_NULL_POINTER);
 
   TextEditorTest *tester = new TextEditorTest();
--- a/editor/libeditor/HTMLEditor.h
+++ b/editor/libeditor/HTMLEditor.h
@@ -442,18 +442,16 @@ protected: // May be called by friends.
    * @param aElement [IN] the element
    * @param aChange  [IN] relative change to apply to current z-index of
    *                      the element
    * @param aReturn  [OUT] the new z-index of the element
    */
   nsresult RelativeChangeElementZIndex(Element& aElement, int32_t aChange,
                                        int32_t* aReturn);
 
-  virtual bool IsModifiableNode(nsINode* aNode) override;
-
   virtual bool IsBlockNode(nsINode *aNode) override;
   using EditorBase::IsBlockNode;
 
   /**
    * returns true if aParentTag can contain a child of type aChildTag.
    */
   virtual bool TagCanContainTag(nsAtom& aParentTag,
                                 nsAtom& aChildTag) const override;
--- a/editor/libeditor/JoinNodeTransaction.cpp
+++ b/editor/libeditor/JoinNodeTransaction.cpp
@@ -6,17 +6,16 @@
 #include "JoinNodeTransaction.h"
 
 #include "mozilla/EditorBase.h"         // for EditorBase
 #include "mozilla/dom/Text.h"
 #include "nsAString.h"
 #include "nsDebug.h"                    // for NS_ASSERTION, etc.
 #include "nsError.h"                    // for NS_ERROR_NULL_POINTER, etc.
 #include "nsIContent.h"                 // for nsIContent
-#include "nsIEditor.h"                  // for EditorBase::IsModifiableNode
 #include "nsISupportsImpl.h"            // for QueryInterface, etc.
 
 namespace mozilla {
 
 using namespace dom;
 
 // static
 already_AddRefed<JoinNodeTransaction>
@@ -55,17 +54,17 @@ bool
 JoinNodeTransaction::CanDoIt() const
 {
   if (NS_WARN_IF(!mLeftNode) ||
       NS_WARN_IF(!mRightNode) ||
       NS_WARN_IF(!mEditorBase) ||
       !mLeftNode->GetParentNode()) {
     return false;
   }
-  return mEditorBase->IsModifiableNode(mLeftNode->GetParentNode());
+  return mEditorBase->IsModifiableNode(*mLeftNode->GetParentNode());
 }
 
 // After DoTransaction() and RedoTransaction(), the left node is removed from
 // the content tree and right node remains.
 NS_IMETHODIMP
 JoinNodeTransaction::DoTransaction()
 {
   if (NS_WARN_IF(!mEditorBase) ||
--- a/editor/libeditor/TextEditRules.cpp
+++ b/editor/libeditor/TextEditRules.cpp
@@ -1540,17 +1540,17 @@ TextEditRules::CreateBogusNodeIfNeeded()
         !isRootEditable ||
         TextEditorRef().IsEditable(rootChild) ||
         TextEditorRef().IsBlockNode(rootChild)) {
       return NS_OK;
     }
   }
 
   // Skip adding the bogus node if body is read-only.
-  if (!TextEditorRef().IsModifiableNode(rootElement)) {
+  if (!TextEditorRef().IsModifiableNode(*rootElement)) {
     return NS_OK;
   }
 
   // Create a br.
   RefPtr<Element> newBrElement =
     TextEditorRef().CreateHTMLContent(nsGkAtoms::br);
   if (NS_WARN_IF(!CanHandleEditAction())) {
     return NS_ERROR_EDITOR_DESTROYED;