Bug 1574852 - part 2: Replace `HTMLEditRules::IsInlineNode()` with `HTMLEditor::NodeIsInlineStatic()` r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 21 Aug 2019 07:13:41 +0000
changeset 489135 72a225ec2ee24773735596ec1a66fd878c88d98a
parent 489134 2ae319789645bbc606930c683bd1be8ba5194d11
child 489136 c51084cb4fb042d39f33cfe458cda6de59aad49a
push id36465
push userdvarga@mozilla.com
push dateWed, 21 Aug 2019 16:47:43 +0000
treeherdermozilla-central@4ab60925635c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1574852
milestone70.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 1574852 - part 2: Replace `HTMLEditRules::IsInlineNode()` with `HTMLEditor::NodeIsInlineStatic()` r=m_kato `HTMLEditRules::IsInlineNode()` is a wrapper of `HTMLEditor::NodeIsInlineStatic()`, but returns opposite value. This patch moves it into `HTMLEditor` and names it with same rule as `NodeIsBlockStatic()`. Note that this method may return true if given node is unexpected node type. E.g., comment node, CDATA node, etc. However, it's not scope of this bug. Differential Revision: https://phabricator.services.mozilla.com/D42770
editor/libeditor/HTMLEditRules.cpp
editor/libeditor/HTMLEditRules.h
editor/libeditor/HTMLEditor.h
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -2177,17 +2177,17 @@ EditActionResult HTMLEditRules::SplitMai
   if (NS_WARN_IF(error.Failed())) {
     return EditActionIgnored(error.StealNSResult());
   }
 
   // if citeNode wasn't a block, we might also want another break before it.
   // We need to examine the content both before the br we just added and also
   // just after it.  If we don't have another br or block boundary adjacent,
   // then we will need a 2nd br added to achieve blank line that user expects.
-  if (IsInlineNode(*citeNode)) {
+  if (HTMLEditor::NodeIsInlineStatic(*citeNode)) {
     // Use DOM point which we tried to collapse to.
     EditorDOMPoint pointToCreateNewBrNode(atBrNode.GetContainer(),
                                           atBrNode.Offset());
 
     WSRunObject wsObj(&HTMLEditorRef(), pointToCreateNewBrNode);
     WSType wsType;
     wsObj.PriorVisibleNode(pointToCreateNewBrNode, nullptr, nullptr, &wsType);
     if (wsType == WSType::normalWS || wsType == WSType::text ||
@@ -4290,17 +4290,17 @@ nsresult HTMLEditRules::MakeList(nsAtom&
       // atCurNode is now referring the right node with mOffset but
       // referring the left node with mRef.  So, invalidate it now.
       atCurNode.Clear();
     }
 
     // if curNode isn't a list item, we must wrap it in one
     nsCOMPtr<Element> listItem;
     if (!HTMLEditUtils::IsListItem(curNode)) {
-      if (IsInlineNode(curNode) && prevListItem) {
+      if (HTMLEditor::NodeIsInlineStatic(curNode) && prevListItem) {
         // this is a continuation of some inline nodes that belong together in
         // the same list item.  use prevListItem
         rv = MOZ_KnownLive(HTMLEditorRef())
                  .MoveNodeToEndWithTransaction(*curNode, *prevListItem);
         if (NS_WARN_IF(!CanHandleEditAction())) {
           return NS_ERROR_EDITOR_DESTROYED;
         }
         if (NS_WARN_IF(NS_FAILED(rv))) {
@@ -4323,17 +4323,17 @@ nsresult HTMLEditRules::MakeList(nsAtom&
                          .InsertContainerWithTransaction(*curNode, aItemType);
           if (NS_WARN_IF(!CanHandleEditAction())) {
             return NS_ERROR_EDITOR_DESTROYED;
           }
           if (NS_WARN_IF(!listItem)) {
             return NS_ERROR_FAILURE;
           }
         }
-        if (IsInlineNode(curNode)) {
+        if (HTMLEditor::NodeIsInlineStatic(curNode)) {
           prevListItem = listItem;
         } else {
           prevListItem = nullptr;
         }
       }
     } else {
       listItem = curNode->AsElement();
     }
@@ -6392,17 +6392,17 @@ nsresult HTMLEditRules::AlignBlockConten
 }
 
 nsresult HTMLEditRules::MaybeDeleteTopMostEmptyAncestor(
     nsINode& aStartNode, Element& aEditingHostElement,
     nsIEditor::EDirection aAction, bool* aHandled) {
   MOZ_ASSERT(IsEditorDataAvailable());
 
   // If the editing host is an inline element, bail out early.
-  if (IsInlineNode(aEditingHostElement)) {
+  if (HTMLEditor::NodeIsInlineStatic(aEditingHostElement)) {
     return NS_OK;
   }
 
   // If we are inside an empty block, delete it.  Note: do NOT delete table
   // elements this way.
   RefPtr<Element> block = HTMLEditorRef().GetBlock(aStartNode);
   RefPtr<Element> emptyBlock;
   if (block && block != &aEditingHostElement) {
@@ -7346,17 +7346,18 @@ nsresult HTMLEditRules::GetNodesForOpera
       aEditSubAction == EditSubAction::eCreateOrChangeList ||
       aEditSubAction == EditSubAction::eSetOrClearAlignment ||
       aEditSubAction == EditSubAction::eSetPositionToAbsolute ||
       aEditSubAction == EditSubAction::eIndent ||
       aEditSubAction == EditSubAction::eOutdent) {
     for (int32_t i = aOutArrayOfNodes.Length() - 1; i >= 0; i--) {
       OwningNonNull<nsINode> node = aOutArrayOfNodes[i];
       // XXX Why do we run this loop even when aTouchContent is "no"?
-      if (aTouchContent == TouchContent::yes && IsInlineNode(node) &&
+      if (aTouchContent == TouchContent::yes &&
+          HTMLEditor::NodeIsInlineStatic(node) &&
           HTMLEditorRef().IsContainer(node) && !EditorBase::IsTextNode(node)) {
         nsTArray<OwningNonNull<nsINode>> arrayOfInlines;
         nsresult rv = BustUpInlinesAtBRs(MOZ_KnownLive(*node->AsContent()),
                                          arrayOfInlines);
         if (NS_WARN_IF(NS_FAILED(rv))) {
           return rv;
         }
 
@@ -7692,17 +7693,17 @@ nsIContent* HTMLEditRules::GetHighestInl
   //     calling this expensive method.
   if (NS_WARN_IF(!EditorUtils::IsDescendantOf(aNode, *host))) {
     return nullptr;
   }
 
   // Looks for the highest inline parent in the editing host.
   nsIContent* content = aNode.AsContent();
   for (nsIContent* parent = content->GetParent();
-       parent && parent != host && IsInlineNode(*parent);
+       parent && parent != host && HTMLEditor::NodeIsInlineStatic(*parent);
        parent = parent->GetParent()) {
     content = parent;
   }
   return content;
 }
 
 nsresult HTMLEditRules::GetNodesFromPoint(
     const EditorDOMPoint& aPoint, EditSubAction aEditSubAction,
@@ -8649,17 +8650,17 @@ nsresult HTMLEditRules::RemoveBlockStyle
       GetChildNodesForOperation(*curNode, childArray);
       nsresult rv = RemoveBlockStyle(childArray);
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }
       continue;
     }
 
-    if (IsInlineNode(curNode)) {
+    if (HTMLEditor::NodeIsInlineStatic(curNode)) {
       if (curBlock) {
         // If so, is this node a descendant?
         if (EditorUtils::IsDescendantOf(*curNode, *curBlock)) {
           // Then we don't need to do anything different for this node
           lastNode = curNode->AsContent();
           continue;
         }
         // Otherwise, we have progressed beyond end of curBlock, so let's
@@ -8877,17 +8878,17 @@ nsresult HTMLEditRules::ApplyBlockStyle(
         return NS_ERROR_EDITOR_DESTROYED;
       }
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }
       continue;
     }
 
-    if (IsInlineNode(curNode)) {
+    if (HTMLEditor::NodeIsInlineStatic(curNode)) {
       // If curNode is inline, pull it into curBlock.  Note: it's assumed that
       // consecutive inline nodes in aNodeArray are actually members of the
       // same block parent.  This happens to be true now as a side effect of
       // how aNodeArray is contructed, but some additional logic should be
       // added here if that should change
       //
       // If curNode is a non editable, drop it if we are going to <pre>.
       if (&aBlockTag == nsGkAtoms::pre &&
@@ -9860,17 +9861,18 @@ nsresult HTMLEditRules::SelectionEndpoin
     }
   }
   return NS_OK;
 }
 
 bool HTMLEditRules::IsEmptyInline(nsINode& aNode) {
   MOZ_ASSERT(IsEditorDataAvailable());
 
-  if (IsInlineNode(aNode) && HTMLEditorRef().IsContainer(&aNode)) {
+  if (HTMLEditor::NodeIsInlineStatic(aNode) &&
+      HTMLEditorRef().IsContainer(&aNode)) {
     bool isEmpty = true;
     HTMLEditorRef().IsEmptyNode(&aNode, &isEmpty);
     return isEmpty;
   }
   return false;
 }
 
 bool HTMLEditRules::ListIsEmptyLine(
--- a/editor/libeditor/HTMLEditRules.h
+++ b/editor/libeditor/HTMLEditRules.h
@@ -113,20 +113,16 @@ class HTMLEditRules : public TextEditRul
  protected:
   virtual ~HTMLEditRules() = default;
 
   HTMLEditor& HTMLEditorRef() const {
     MOZ_ASSERT(mData);
     return mData->HTMLEditorRef();
   }
 
-  static bool IsInlineNode(const nsINode& aNode) {
-    return !HTMLEditor::NodeIsBlockStatic(aNode);
-  }
-
   enum RulesEndpoint { kStart, kEnd };
 
   void InitFields();
 
   /**
    * Called before inserting something into the editor.
    * This method may removes mBougsNode if there is.  Therefore, this method
    * might cause destroying the editor.
--- a/editor/libeditor/HTMLEditor.h
+++ b/editor/libeditor/HTMLEditor.h
@@ -799,16 +799,25 @@ class HTMLEditor final : public TextEdit
 
   /**
    * NodeIsBlockStatic() returns true if aElement is an element node and
    * should be treated as a block.
    */
   static bool NodeIsBlockStatic(const nsINode& aElement);
 
   /**
+   * NodeIsInlineStatic() returns true if aElement is an element node but
+   * shouldn't be treated as a block or aElement is not an element.
+   * XXX This looks odd.  For example, how about a comment node?
+   */
+  static bool NodeIsInlineStatic(const nsINode& aElement) {
+    return !NodeIsBlockStatic(aElement);
+  }
+
+  /**
    * extracts an element from the normal flow of the document and
    * positions it, and puts it back in the normal flow.
    * @param aElement [IN] the element
    * @param aEnabled [IN] true to absolutely position the element,
    *                      false to put it back in the normal flow
    */
   MOZ_CAN_RUN_SCRIPT
   nsresult SetPositionToAbsoluteOrStatic(Element& aElement, bool aEnabled);