Bug 1483132 - Make EditorBase::AreNodesSameType() non-virtual r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 16 Aug 2018 10:29:20 +0000
changeset 489784 fd90e385d65bc4eeb960adaae509458504bbead8
parent 489783 ef882e3e4b6bb67e0bfef77939d1bd1addc68998
child 489785 a28cf4300f126a9ae4960035c991d9280c9830d6
push id1815
push userffxbld-merge
push dateMon, 15 Oct 2018 10:40:45 +0000
treeherdermozilla-release@18d4c09e9378 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1483132
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 1483132 - Make EditorBase::AreNodesSameType() non-virtual r=m_kato EditorBase::AreNodesSameType() is overridden only by HTMLEditor and the implementation is enough simple to re-implement in EditorBase. Additionally, this is called from condition of a loop in JoinNodesDeepWithTransaction(). So, the virtual call cost may make damage to the performance. Differential Revision: https://phabricator.services.mozilla.com/D3460
editor/libeditor/EditorBase.cpp
editor/libeditor/EditorBase.h
editor/libeditor/HTMLEditRules.cpp
editor/libeditor/HTMLEditor.cpp
editor/libeditor/HTMLEditor.h
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -23,16 +23,17 @@
 #include "InsertNodeTransaction.h"      // for InsertNodeTransaction
 #include "InsertTextTransaction.h"      // for InsertTextTransaction
 #include "JoinNodeTransaction.h"        // for JoinNodeTransaction
 #include "PlaceholderTransaction.h"     // for PlaceholderTransaction
 #include "SplitNodeTransaction.h"       // for SplitNodeTransaction
 #include "TextEditUtils.h"              // for TextEditUtils
 #include "mozilla/CheckedInt.h"         // for CheckedInt
 #include "mozilla/ComputedStyle.h"      // for ComputedStyle
+#include "mozilla/CSSEditUtils.h"       // for CSSEditUtils
 #include "mozilla/EditAction.h"         // for EditSubAction
 #include "mozilla/EditorDOMPoint.h"     // for EditorDOMPoint
 #include "mozilla/EditorSpellCheck.h"   // for EditorSpellCheck
 #include "mozilla/EditorUtils.h"        // for various helper classes.
 #include "mozilla/EditTransactionBase.h" // for EditTransactionBase
 #include "mozilla/FlushType.h"          // for FlushType::Frames
 #include "mozilla/IMEContentObserver.h" // for IMEContentObserver
 #include "mozilla/IMEStateManager.h"    // for IMEStateManager
@@ -3861,23 +3862,37 @@ EditorBase::ResetModificationCount()
   mModCount = 0;
 
   if (doNotify) {
     NotifyDocumentListeners(eDocumentStateChanged);
   }
   return NS_OK;
 }
 
+// static
 bool
-EditorBase::AreNodesSameType(nsIContent* aNode1,
-                             nsIContent* aNode2)
-{
-  MOZ_ASSERT(aNode1);
-  MOZ_ASSERT(aNode2);
-  return aNode1->NodeInfo()->NameAtom() == aNode2->NodeInfo()->NameAtom();
+EditorBase::AreNodesSameType(nsIContent& aNode1,
+                             nsIContent& aNode2) const
+{
+  if (aNode1.NodeInfo()->NameAtom() != aNode2.NodeInfo()->NameAtom()) {
+    return false;
+  }
+  if (!AsHTMLEditor() || !AsHTMLEditor()->IsCSSEnabled()) {
+    return true;
+  }
+  // If this is an HTMLEditor in CSS mode and they are <span> elements,
+  // let's check their styles.
+  if (!aNode1.IsHTMLElement(nsGkAtoms::span)) {
+    return true;
+  }
+  if (!aNode1.IsElement() || !aNode2.IsElement()) {
+    return false;
+  }
+  return CSSEditUtils::ElementsSameStyle(aNode1.AsElement(),
+                                         aNode2.AsElement());
 }
 
 // static
 nsIContent*
 EditorBase::GetNodeAtRangeOffsetPoint(const RawRangeBoundary& aPoint)
 {
   if (NS_WARN_IF(!aPoint.IsSet())) {
     return nullptr;
@@ -4081,17 +4096,17 @@ EditorBase::JoinNodesDeepWithTransaction
   // up.
 
   nsCOMPtr<nsIContent> leftNodeToJoin = &aLeftNode;
   nsCOMPtr<nsIContent> rightNodeToJoin = &aRightNode;
   nsCOMPtr<nsINode> parentNode = aRightNode.GetParentNode();
 
   EditorDOMPoint ret;
   while (leftNodeToJoin && rightNodeToJoin && parentNode &&
-         AreNodesSameType(leftNodeToJoin, rightNodeToJoin)) {
+         AreNodesSameType(*leftNodeToJoin, *rightNodeToJoin)) {
     uint32_t length = leftNodeToJoin->Length();
 
     // Do the join
     nsresult rv = JoinNodesWithTransaction(*leftNodeToJoin, *rightNodeToJoin);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return EditorDOMPoint();
     }
 
--- a/editor/libeditor/EditorBase.h
+++ b/editor/libeditor/EditorBase.h
@@ -1500,19 +1500,22 @@ protected: // May be called by friends.
   nsINode* GetFirstEditableNode(nsINode* aRoot);
 
   /**
    * Returns true when inserting text should be a part of current composition.
    */
   bool ShouldHandleIMEComposition() const;
 
   /**
-   * From html rules code - migration in progress.
+   * AreNodesSameType() returns true if aNode1 and aNode2 are same type.
+   * If the instance is TextEditor, only their names are checked.
+   * If the instance is HTMLEditor in CSS mode and both of them are <span>
+   * element, their styles are also checked.
    */
-  virtual bool AreNodesSameType(nsIContent* aNode1, nsIContent* aNode2);
+  bool AreNodesSameType(nsIContent& aNode1, nsIContent& aNode2) const;
 
   static bool IsTextNode(nsINode* aNode)
   {
     return aNode->NodeType() == nsINode::TEXT_NODE;
   }
 
   /**
    * IsModifiableNode() checks whether the node is editable or not.
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -3026,17 +3026,17 @@ HTMLEditRules::WillDeleteSelection(nsIEd
         }
 
         // Are the blocks siblings?
         nsCOMPtr<nsINode> leftBlockParent = leftParent->GetParentNode();
         nsCOMPtr<nsINode> rightBlockParent = rightParent->GetParentNode();
 
         // MOOSE: this could conceivably screw up a table.. fix me.
         if (leftBlockParent == rightBlockParent &&
-            HTMLEditorRef().AreNodesSameType(leftParent, rightParent) &&
+            HTMLEditorRef().AreNodesSameType(*leftParent, *rightParent) &&
             // XXX What's special about these three types of block?
             (leftParent->IsHTMLElement(nsGkAtoms::p) ||
              HTMLEditUtils::IsListItem(leftParent) ||
              HTMLEditUtils::IsHeader(*leftParent))) {
           // First delete the selection
           rv = HTMLEditorRef().DeleteSelectionWithTransaction(aAction,
                                                               aStripWrappers);
           if (NS_WARN_IF(!CanHandleEditAction())) {
@@ -9199,17 +9199,17 @@ HTMLEditRules::JoinNearestEditableNodesW
   if (NS_WARN_IF(!CanHandleEditAction())) {
     return NS_ERROR_EDITOR_DESTROYED;
   }
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   if (lastLeft && firstRight &&
-      HTMLEditorRef().AreNodesSameType(lastLeft, firstRight) &&
+      HTMLEditorRef().AreNodesSameType(*lastLeft, *firstRight) &&
       (lastLeft->GetAsText() ||
        (lastLeft->IsElement() && firstRight->IsElement() &&
         CSSEditUtils::ElementsSameStyle(lastLeft->AsElement(),
                                         firstRight->AsElement())))) {
     nsresult rv =
       JoinNearestEditableNodesWithTransaction(*lastLeft, *firstRight,
                                               aNewFirstChildOfRightNode);
     if (NS_WARN_IF(NS_FAILED(rv))) {
--- a/editor/libeditor/HTMLEditor.cpp
+++ b/editor/libeditor/HTMLEditor.cpp
@@ -4588,43 +4588,16 @@ HTMLEditor::SetBackgroundColor(const nsA
     // the document)
     return SetCSSBackgroundColorWithTransaction(aColor);
   }
 
   // but in HTML mode, we can only set the document's background color
   return SetHTMLBackgroundColorWithTransaction(aColor);
 }
 
-/**
- * NodesSameType() does these nodes have the same tag?
- */
-bool
-HTMLEditor::AreNodesSameType(nsIContent* aNode1,
-                             nsIContent* aNode2)
-{
-  MOZ_ASSERT(aNode1);
-  MOZ_ASSERT(aNode2);
-
-  if (aNode1->NodeInfo()->NameAtom() != aNode2->NodeInfo()->NameAtom()) {
-    return false;
-  }
-
-  if (!IsCSSEnabled() || !aNode1->IsHTMLElement(nsGkAtoms::span)) {
-    return true;
-  }
-
-  if (!aNode1->IsElement() || !aNode2->IsElement()) {
-    return false;
-  }
-
-  // If CSS is enabled, we are stricter about span nodes.
-  return CSSEditUtils::ElementsSameStyle(aNode1->AsElement(),
-                                         aNode2->AsElement());
-}
-
 nsresult
 HTMLEditor::CopyLastEditableChildStylesWithTransaction(
               Element& aPreviousBlock,
               Element& aNewBlock,
               RefPtr<Element>* aNewBrElement)
 {
   if (NS_WARN_IF(!aNewBrElement)) {
     return NS_ERROR_INVALID_ARG;
--- a/editor/libeditor/HTMLEditor.h
+++ b/editor/libeditor/HTMLEditor.h
@@ -497,19 +497,16 @@ protected: // May be called by friends.
    */
   virtual bool IsContainer(nsINode* aNode) override;
 
   /**
    * Join together any adjacent editable text nodes in the range.
    */
   nsresult CollapseAdjacentTextNodes(nsRange* aRange);
 
-  virtual bool AreNodesSameType(nsIContent* aNode1,
-                                nsIContent* aNode2) override;
-
   /**
    * IsInVisibleTextFrames() returns true if all text in aText is in visible
    * text frames.  Callers have to guarantee that there is no pending reflow.
    */
   bool IsInVisibleTextFrames(dom::Text& aText);
 
   /**
    * IsVisibleTextNode() returns true if aText has visible text.  If it has
@@ -1673,16 +1670,17 @@ protected:
   void RemoveMouseClickListener(Element* aElement);
 
   nsCOMPtr<nsILinkHandler> mLinkHandler;
 
   ParagraphSeparator mDefaultParagraphSeparator;
 
   friend class AutoSelectionSetterAfterTableEdit;
   friend class CSSEditUtils;
+  friend class EditorBase;
   friend class EmptyEditableFunctor;
   friend class HTMLEditRules;
   friend class TextEditor;
   friend class WSRunObject;
 };
 
 } // namespace mozilla