Bug 1415800 - part 5: Redesign HTMLEditRules::FindNearSelectableNode() r=m_kato

First, the method name is not correct.  It tries to find an editable node near
the given DOM point.  Therefore, it should be FindNearEditableNode().

Next, the implementation did something odd.  E.g., in the first |if| block,
when |nearNode| is nullptr, it returns nullptr.  However, following |if| block
does something only when |nearNode| is nullptr.  So, we can get rid of the
second |if| block.  Then, nobody will change aDirection.  So, we can make it
not a reference now.

Similarly, in |while| block, if |nearNode| becomes nullptr, it returns error.
However, following block checks if |nearNode| is NOT nullptr.  So, we can get
rid of this |if| statement and outdent its block.

Additionally, |curNode| isn't necessary.  It only increments the refcount
redundantly.  So, we can get rid of it.

Finally, FindNearEditableNode() can return found node directly instead of
error code because error code doesn't make sense.  Not found an editable
node is not illegal.  And also it can take EditorRawDOMPoint instead of
a set of container, child and offset of the child in the container.

MozReview-Commit-ID: CTI581PhJMd

--HG--
extra : rebase_source : 7e05998721ce96727d40dda1be5e7e36b090bcd3

editor/libeditor/HTMLEditRules.cpp

@@ -7990,108 +7990,74 @@ HTMLEditRules::AdjustSelection(Selection* aSelection,
 
   // look for a nearby text node.
   // prefer the correct direction.
-  nsresult rv = FindNearSelectableNode(point.Container(), point.Offset(),
-                                       point.GetChildAtOffset(), aAction,
-                                       address_of(nearNode));
-  NS_ENSURE_SUCCESS(rv, rv);
-
+  nearNode = FindNearEditableNode(point.AsRaw(), aAction);
   if (!nearNode) {
     return NS_OK;
   }
+
   EditorDOMPoint pt = GetGoodSelPointForNode(*nearNode, aAction);
-  rv = aSelection->Collapse(pt.AsRaw());
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
+  ErrorResult error;
+  aSelection->Collapse(pt.AsRaw(), error);
+  if (NS_WARN_IF(error.Failed())) {
+    return error.StealNSResult();
   }
   return NS_OK;
 }
 
-
-nsresult
-HTMLEditRules::FindNearSelectableNode(nsINode* aSelNode,
-                                      int32_t aSelOffset,
-                                      nsINode* aChildAtOffset,
-                                      nsIEditor::EDirection& aDirection,
-                                      nsCOMPtr<nsIContent>* outSelectableNode)
+nsIContent*
+HTMLEditRules::FindNearEditableNode(const EditorRawDOMPoint& aPoint,
+                                    nsIEditor::EDirection aDirection)
 {
-  NS_ENSURE_TRUE(aSelNode && outSelectableNode, NS_ERROR_NULL_POINTER);
-  *outSelectableNode = nullptr;
+  if (NS_WARN_IF(!aPoint.IsSet()) ||
+      NS_WARN_IF(!mHTMLEditor)) {
+    return nullptr;
+  }
+  MOZ_ASSERT(aPoint.IsSetAndValid());
 
-  EditorRawDOMPoint point(aSelNode,
-                          aChildAtOffset && aChildAtOffset->IsContent() ?
-                            aChildAtOffset->AsContent() : nullptr,
-                          aSelOffset);
+  RefPtr<HTMLEditor> htmlEditor(mHTMLEditor);
 
-  nsCOMPtr<nsIContent> nearNode, curNode;
+  nsIContent* nearNode = nullptr;
   if (aDirection == nsIEditor::ePrevious) {
-    NS_ENSURE_STATE(mHTMLEditor);
-    nearNode = mHTMLEditor->GetPreviousEditableHTMLNode(point);
-    if (NS_WARN_IF(!nearNode)) {
-      return NS_ERROR_FAILURE;
+    nearNode = htmlEditor->GetPreviousEditableHTMLNode(aPoint);
+    if (!nearNode) {
+      return nullptr; // Not illegal.
     }
   } else {
-    NS_ENSURE_STATE(mHTMLEditor);
-    nearNode = mHTMLEditor->GetNextEditableHTMLNode(point);
+    nearNode = htmlEditor->GetNextEditableHTMLNode(aPoint);
     if (NS_WARN_IF(!nearNode)) {
-      return NS_ERROR_FAILURE;
-    }
-  }
-
-  // Try the other direction then.
-  if (!nearNode) {
-    if (aDirection == nsIEditor::ePrevious) {
-      aDirection = nsIEditor::eNext;
-    } else {
-      aDirection = nsIEditor::ePrevious;
-    }
-
-    if (aDirection == nsIEditor::ePrevious) {
-      NS_ENSURE_STATE(mHTMLEditor);
-      nearNode = mHTMLEditor->GetPreviousEditableHTMLNode(point);
-      if (NS_WARN_IF(!nearNode)) {
-        return NS_ERROR_FAILURE;
-      }
-    } else {
-      NS_ENSURE_STATE(mHTMLEditor);
-      nearNode = mHTMLEditor->GetPreviousEditableHTMLNode(point);
-      if (NS_WARN_IF(!nearNode)) {
-        return NS_ERROR_FAILURE;
-      }
+      // Perhaps, illegal because the node pointed by aPoint isn't editable
+      // and nobody of previous nodes is editable.
+      return nullptr;
     }
   }
 
   // scan in the right direction until we find an eligible text node,
   // but don't cross any breaks, images, or table elements.
+  // XXX This comment sounds odd.  |nearNode| may have already crossed breaks
+  //     and/or images.
   while (nearNode && !(EditorBase::IsTextNode(nearNode) ||
                        TextEditUtils::IsBreak(nearNode) ||
                        HTMLEditUtils::IsImage(nearNode))) {
-    curNode = nearNode;
     if (aDirection == nsIEditor::ePrevious) {
-      NS_ENSURE_STATE(mHTMLEditor);
-      nearNode = mHTMLEditor->GetPreviousEditableHTMLNode(*curNode);
+      nearNode = htmlEditor->GetPreviousEditableHTMLNode(*nearNode);
       if (NS_WARN_IF(!nearNode)) {
-        return NS_ERROR_FAILURE;
+        return nullptr;
       }
     } else {
-      NS_ENSURE_STATE(mHTMLEditor);
-      nearNode = mHTMLEditor->GetNextEditableHTMLNode(*curNode);
+      nearNode = htmlEditor->GetNextEditableHTMLNode(*nearNode);
       if (NS_WARN_IF(!nearNode)) {
-        return NS_ERROR_FAILURE;
+        return nullptr;
       }
     }
-    NS_ENSURE_STATE(mHTMLEditor);
   }
 
-  if (nearNode) {
-    // don't cross any table elements
-    if (InDifferentTableElements(nearNode, aSelNode)) {
-      return NS_OK;
-    }
-
-    // otherwise, ok, we have found a good spot to put the selection
-    *outSelectableNode = nearNode;
+  // don't cross any table elements
+  if (InDifferentTableElements(nearNode, aPoint.Container())) {
+    return nullptr;
   }
-  return NS_OK;
+
+  // otherwise, ok, we have found a good spot to put the selection
+  return nearNode;
 }
 
 bool

editor/libeditor/HTMLEditRules.h

@@ -410,11 +410,22 @@ protected:
   void CheckInterlinePosition(Selection& aSelection);
   nsresult AdjustSelection(Selection* aSelection,
                            nsIEditor::EDirection aAction);
-  nsresult FindNearSelectableNode(nsINode* aSelNode,
-                                  int32_t aSelOffset,
-                                  nsINode* aChildAtOffset,
-                                  nsIEditor::EDirection& aDirection,
-                                  nsCOMPtr<nsIContent>* outSelectableNode);
+
+  /**
+   * FindNearEditableNode() tries to find an editable node near aPoint.
+   *
+   * @param aPoint      The DOM point where to start to search from.
+   * @param aDirection  If nsIEditor::ePrevious is set, this searches an
+   *                    editable node from next nodes.  Otherwise, from
+   *                    previous nodes.
+   * @return            If found, returns non-nullptr.  Otherwise, nullptr.
+   *                    Note that if found node is in different table element,
+   *                    this returns nullptr.
+   *                    And also if aDirection is not nsIEditor::ePrevious,
+   *                    the result may be the node pointed by aPoint.
+   */
+  nsIContent* FindNearEditableNode(const EditorRawDOMPoint& aPoint,
+                                   nsIEditor::EDirection aDirection);
   /**
    * Returns true if aNode1 or aNode2 or both is the descendant of some type of
    * table element, but their nearest table element ancestors differ.  "Table