Bug 1627175 - part 65: Move `HTMLEditor::GetDeepestEditableOnlyChildDivBlockquoteOrListElement()` to `HTMLEditUtils` r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Tue, 18 May 2021 05:52:25 +0000
changeset 579852 7ecdf33095634893942b379b952e90d1ae8745c8
parent 579851 99e716181ac9a7b325741188d1aad6f57d71ef88
child 579853 a1b461b1982eb173b8252e50c28198b59535e2b3
push id38469
push userncsoregi@mozilla.com
push dateTue, 18 May 2021 09:45:31 +0000
treeherdermozilla-central@4f930697652e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1627175
milestone90.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 1627175 - part 65: Move `HTMLEditor::GetDeepestEditableOnlyChildDivBlockquoteOrListElement()` to `HTMLEditUtils` r=m_kato And also `HTMLEditor::CountEditableChildren()` is moved since it's used only by it. Despite the long method name, it's really unclear what it does. I try to explain it with new name, but the things which the method does are not make sense. So, if you have better name idea, I'll take it... Differential Revision: https://phabricator.services.mozilla.com/D115177
editor/libeditor/EditorBase.cpp
editor/libeditor/EditorBase.h
editor/libeditor/HTMLEditSubActionHandler.cpp
editor/libeditor/HTMLEditUtils.h
editor/libeditor/HTMLEditor.h
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -2851,29 +2851,16 @@ bool EditorBase::IsDescendantOfEditorRoo
   nsIContent* root = GetEditorRoot();
   if (NS_WARN_IF(!root)) {
     return false;
   }
 
   return aNode->IsInclusiveDescendantOf(root);
 }
 
-uint32_t EditorBase::CountEditableChildren(nsINode* aNode) {
-  MOZ_ASSERT(aNode);
-  uint32_t count = 0;
-  EditorType editorType = GetEditorType();
-  for (nsIContent* child = aNode->GetFirstChild(); child;
-       child = child->GetNextSibling()) {
-    if (EditorUtils::IsEditableContent(*child, editorType)) {
-      ++count;
-    }
-  }
-  return count;
-}
-
 NS_IMETHODIMP EditorBase::IncrementModificationCount(int32_t inNumMods) {
   uint32_t oldModCount = mModCount;
 
   mModCount += inNumMods;
 
   if ((!oldModCount && mModCount) || (oldModCount && !mModCount)) {
     DebugOnly<nsresult> rvIgnored =
         NotifyDocumentListeners(eDocumentStateChanged);
--- a/editor/libeditor/EditorBase.h
+++ b/editor/libeditor/EditorBase.h
@@ -1730,21 +1730,16 @@ class EditorBase : public nsIEditor,
 
   /**
    * Returns true if aNode is a descendant of our root node.
    */
   bool IsDescendantOfRoot(const nsINode* inNode) const;
   bool IsDescendantOfEditorRoot(const nsINode* aNode) const;
 
   /**
-   * Counts number of editable child nodes.
-   */
-  uint32_t CountEditableChildren(nsINode* aNode);
-
-  /**
    * Returns true when inserting text should be a part of current composition.
    */
   bool ShouldHandleIMEComposition() 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.
--- a/editor/libeditor/HTMLEditSubActionHandler.cpp
+++ b/editor/libeditor/HTMLEditSubActionHandler.cpp
@@ -2563,18 +2563,20 @@ EditActionResult HTMLEditor::ChangeSelec
                          "HTMLEditor::CollapseSelectionToStartOf() failed");
     return EditActionResult(rv);
   }
 
   // if there is only one node in the array, and it is a list, div, or
   // blockquote, then look inside of it until we find inner list or content.
   if (arrayOfContents.Length() == 1) {
     if (Element* deepestDivBlockquoteOrListElement =
-            GetDeepestEditableOnlyChildDivBlockquoteOrListElement(
-                arrayOfContents[0])) {
+            HTMLEditUtils::GetInclusiveDeepestFirstChildWhichHasOneChild(
+                arrayOfContents[0], {WalkTreeOption::IgnoreNonEditableNode},
+                nsGkAtoms::div, nsGkAtoms::blockquote, nsGkAtoms::ul,
+                nsGkAtoms::ol, nsGkAtoms::dl)) {
       if (deepestDivBlockquoteOrListElement->IsAnyOfHTMLElements(
               nsGkAtoms::div, nsGkAtoms::blockquote)) {
         arrayOfContents.Clear();
         CollectChildren(*deepestDivBlockquoteOrListElement, arrayOfContents, 0,
                         CollectListChildren::No, CollectTableChildren::No,
                         CollectNonEditableNodes::Yes);
       } else {
         arrayOfContents.ReplaceElementAt(
@@ -6078,18 +6080,20 @@ nsresult HTMLEditor::CollectEditTargetNo
       }
       // If there is only one node in the array, and it is a `<div>`,
       // `<blockquote>` or a list element, then look inside of it until we
       // find inner list or content.
       if (aOutArrayOfContents.Length() != 1) {
         break;
       }
       Element* deepestDivBlockquoteOrListElement =
-          GetDeepestEditableOnlyChildDivBlockquoteOrListElement(
-              aOutArrayOfContents[0]);
+          HTMLEditUtils::GetInclusiveDeepestFirstChildWhichHasOneChild(
+              aOutArrayOfContents[0], {WalkTreeOption::IgnoreNonEditableNode},
+              nsGkAtoms::div, nsGkAtoms::blockquote, nsGkAtoms::ul,
+              nsGkAtoms::ol, nsGkAtoms::dl);
       if (!deepestDivBlockquoteOrListElement) {
         break;
       }
       if (deepestDivBlockquoteOrListElement->IsAnyOfHTMLElements(
               nsGkAtoms::div, nsGkAtoms::blockquote)) {
         aOutArrayOfContents.Clear();
         // XXX Before we're called, non-editable nodes are ignored.  However,
         //     we may append non-editable nodes here.
@@ -6186,36 +6190,16 @@ Element* HTMLEditor::GetParentListElemen
       if (HTMLEditUtils::IsAnyListElement(parent)) {
         return parent->AsElement();
       }
     }
   }
   return nullptr;
 }
 
-Element* HTMLEditor::GetDeepestEditableOnlyChildDivBlockquoteOrListElement(
-    nsINode& aNode) {
-  if (!aNode.IsElement()) {
-    return nullptr;
-  }
-  // XXX If aNode is non-editable, shouldn't we return nullptr here?
-  Element* parentElement = nullptr;
-  for (nsIContent* content = aNode.AsContent();
-       content && content->IsElement() &&
-       (content->IsAnyOfHTMLElements(nsGkAtoms::div, nsGkAtoms::blockquote) ||
-        HTMLEditUtils::IsAnyListElement(content));
-       content = content->GetFirstChild()) {
-    if (CountEditableChildren(content) != 1) {
-      return content->AsElement();
-    }
-    parentElement = content->AsElement();
-  }
-  return parentElement;
-}
-
 nsresult HTMLEditor::SplitParentInlineElementsAtRangeEdges(
     RangeItem& aRangeItem) {
   MOZ_ASSERT(IsEditActionDataAvailable());
 
   RefPtr<Element> editingHost = GetActiveEditingHost();
   if (NS_WARN_IF(!editingHost)) {
     return NS_OK;
   }
--- a/editor/libeditor/HTMLEditUtils.h
+++ b/editor/libeditor/HTMLEditUtils.h
@@ -1254,16 +1254,51 @@ class HTMLEditUtils final {
     const nsRange* firstRange = aSelection.GetRangeAt(0);
     if (NS_WARN_IF(!firstRange) || NS_WARN_IF(!firstRange->IsPositioned())) {
       return nullptr;
     }
     return GetTableCellElementIfOnlyOneSelected(*firstRange);
   }
 
   /**
+   * GetInclusiveFirstChildWhichHasOneChild() returns the deepest element whose
+   * tag name is one of `aFirstElementName` and `aOtherElementNames...` if and
+   * only if the elements have only one child node.   In other words, when
+   * this method meets an element which does not matches any of the tag name
+   * or it has no children or 2+ children.
+   *
+   * XXX This method must be implemented without treating edge cases.  So, the
+   *     behavior is odd.  E.g., why can we ignore non-editable node at counting
+   *     each children?  Why do we dig non-editable aNode or first child of its
+   *     descendants?
+   */
+  template <typename FirstElementName, typename... OtherElementNames>
+  static Element* GetInclusiveDeepestFirstChildWhichHasOneChild(
+      const nsINode& aNode, const WalkTreeOptions& aOptions,
+      FirstElementName aFirstElementName,
+      OtherElementNames... aOtherElementNames) {
+    if (!aNode.IsElement()) {
+      return nullptr;
+    }
+    Element* parentElement = nullptr;
+    for (nsIContent* content = const_cast<nsIContent*>(aNode.AsContent());
+         content && content->IsElement() &&
+         content->IsAnyOfHTMLElements(aFirstElementName, aOtherElementNames...);
+         // XXX Why do we scan only the first child of every element?  If it's
+         //     not editable, why do we ignore it when aOptions specifies so.
+         content = content->GetFirstChild()) {
+      if (HTMLEditUtils::CountChildren(*content, aOptions) != 1) {
+        return content->AsElement();
+      }
+      parentElement = content->AsElement();
+    }
+    return parentElement;
+  }
+
+  /**
    * IsInTableCellSelectionMode() returns true when Gecko's editor thinks that
    * selection is in a table cell selection mode.
    * Note that Gecko's editor traditionally treats selection as in table cell
    * selection mode when first range selects a table cell element.  I.e., even
    * if `nsFrameSelection` is not in table cell selection mode, this may return
    * true.
    */
   static bool IsInTableCellSelectionMode(const Selection& aSelection) {
@@ -1499,16 +1534,33 @@ class HTMLEditUtils final {
     if (aOptions.contains(WalkTreeOption::IgnoreWhiteSpaceOnlyText) &&
         aContent.IsText() &&
         const_cast<dom::Text*>(aContent.AsText())->TextIsOnlyWhitespace()) {
       return true;
     }
     return false;
   }
 
+  static uint32_t CountChildren(const nsINode& aNode,
+                                const WalkTreeOptions& aOptions) {
+    uint32_t count = 0;
+    for (nsIContent* child = aNode.GetFirstChild(); child;
+         child = child->GetNextSibling()) {
+      if (HTMLEditUtils::IsContentIgnored(*child, aOptions)) {
+        continue;
+      }
+      if (aOptions.contains(WalkTreeOption::StopAtBlockBoundary) &&
+          HTMLEditUtils::IsBlockElement(*child)) {
+        break;
+      }
+      ++count;
+    }
+    return count;
+  }
+
   /**
    * Helper for GetPreviousContent() and GetNextContent().
    */
   enum class WalkTreeDirection { Forward, Backward };
   static nsIContent* GetAdjacentLeafContent(
       const nsINode& aNode, WalkTreeDirection aWalkTreeDirection,
       const WalkTreeOptions& aOptions,
       const Element* aAncestorLimiter = nullptr);
--- a/editor/libeditor/HTMLEditor.h
+++ b/editor/libeditor/HTMLEditor.h
@@ -1199,27 +1199,16 @@ class HTMLEditor final : public TextEdit
    */
   nsresult CollectEditTargetNodes(
       nsTArray<RefPtr<nsRange>>& aArrayOfRanges,
       nsTArray<OwningNonNull<nsIContent>>& aOutArrayOfContents,
       EditSubAction aEditSubAction,
       CollectNonEditableNodes aCollectNonEditableNodes);
 
   /**
-   * GetWhiteSpaceEndPoint() returns point at first or last ASCII white-space
-   * or non-breakable space starting from aPoint.  I.e., this returns next or
-   * previous point whether the character is neither ASCII white-space nor
-   * non-brekable space.
-   */
-  enum class ScanDirection { Backward, Forward };
-  template <typename PT, typename RT>
-  static EditorDOMPoint GetWhiteSpaceEndPoint(
-      const RangeBoundaryBase<PT, RT>& aPoint, ScanDirection aScanDirection);
-
-  /**
    * GetCurrentHardLineStartPoint() returns start point of hard line
    * including aPoint.  If the line starts after a `<br>` element, returns
    * next sibling of the `<br>` element.  If the line is first line of a block,
    * returns point of the block.
    * NOTE: The result may be point of editing host.  I.e., the container may
    *       be outside of editing host.
    */
   template <typename PT, typename RT>
@@ -1396,26 +1385,16 @@ class HTMLEditor final : public TextEdit
     aOutArrayOfContents.SetCapacity(aParentNode.GetChildCount());
     for (nsIContent* childContent = aParentNode.GetFirstChild(); childContent;
          childContent = childContent->GetNextSibling()) {
       aOutArrayOfContents.AppendElement(*childContent);
     }
   }
 
   /**
-   * GetDeepestEditableOnlyChildDivBlockquoteOrListElement() returns a `<div>`,
-   * `<blockquote>` or one of list elements.  This method climbs down from
-   * aContent while there is only one editable children and the editable child
-   * is `<div>`, `<blockquote>` or a list element.  When it reaches different
-   * kind of node, returns the last found element.
-   */
-  Element* GetDeepestEditableOnlyChildDivBlockquoteOrListElement(
-      nsINode& aNode);
-
-  /**
    * Try to get parent list element at `Selection`.  This returns first find
    * parent list element of common ancestor of ranges (looking for it from
    * first range to last range).
    */
   Element* GetParentListElementAtSelection() const;
 
   /**
    * MaybeExtendSelectionToHardLineEdgesForBlockEditAction() adjust Selection if