Bug 1627175 - part 12: Get rid of `HTMLEditor::GetCellFromRange()` r=m_kato

Even though the method returns only in specific cases, but the result affects
only one caller, `HTMLEditor::GetNextSelectedTableCellElement()`.  Therefore,
we can create new generic utility method,
`HTMLEditUtils::GetTableCellElementIfOnlyOneSelected()` and get rid of
`HTMLEditor::GetCellFromRange()`.

Note that the warnings in `HTMLEditor::GetCellFromRange()` is just noise for
any callers.  So, this gets rid of the useless spam warnings.

Differential Revision: https://phabricator.services.mozilla.com/D72586

editor/libeditor/HTMLEditUtils.h

@@ -299,6 +299,12 @@ class HTMLEditUtils final {
     return !end.GetChildAtOffset() ? childAtStart->AsElement() : nullptr;
   }
 
+  static Element* GetTableCellElementIfOnlyOneSelected(
+      const dom::AbstractRange& aRange) {
+    Element* element = HTMLEditUtils::GetElementIfOnlyOneSelected(aRange);
+    return element && HTMLEditUtils::IsTableCell(element) ? element : nullptr;
+  }
+
   static EditAction GetEditActionForInsert(const nsAtom& aTagName);
   static EditAction GetEditActionForRemoveList(const nsAtom& aTagName);
   static EditAction GetEditActionForInsert(const Element& aElement);

editor/libeditor/HTMLEditor.h

@@ -3607,8 +3607,6 @@ class HTMLEditor final : public TextEditor,
 
   nsresult GetLastCellInRow(nsINode* aRowNode, nsINode** aCellNode);
 
-  static nsresult GetCellFromRange(nsRange* aRange, Element** aCell);
-
   /**
    * This sets background on the appropriate container element (table, cell,)
    * or calls into nsTextEditor to set the page background.

editor/libeditor/HTMLTableEditor.cpp

@@ -2905,12 +2905,8 @@ NS_IMETHODIMP HTMLEditor::JoinTableCells(bool aMergeNonContiguousContents) {
         return NS_ERROR_FAILURE;
       }
 
-      RefPtr<Element> deletedCell;
-      DebugOnly<nsresult> rvIgnored =
-          HTMLEditor::GetCellFromRange(range, getter_AddRefs(deletedCell));
-      NS_WARNING_ASSERTION(
-          NS_SUCCEEDED(rvIgnored),
-          "HTMLEditor::GetCellFromRange() failed, but ignored");
+      Element* deletedCell =
+          HTMLEditUtils::GetTableCellElementIfOnlyOneSelected(*range);
       if (!deletedCell) {
         MOZ_KnownLive(SelectionRefPtr())
             ->RemoveRangeAndUnselectFramesAndNotifyListeners(*range,
@@ -3880,38 +3876,6 @@ nsresult HTMLEditor::GetCellContext(Element** aTable, Element** aCell,
   return NS_OK;
 }
 
-// static
-nsresult HTMLEditor::GetCellFromRange(nsRange* aRange, Element** aCell) {
-  // Note: this might return a node that is outside of the range.
-  // Use carefully.
-  if (NS_WARN_IF(!aRange) || NS_WARN_IF(!aCell)) {
-    return NS_ERROR_INVALID_ARG;
-  }
-
-  *aCell = nullptr;
-  Element* element = HTMLEditUtils::GetElementIfOnlyOneSelected(*aRange);
-  if (!element) {
-    // For backward compatibility, the following cases should return error for
-    // now.
-    if (NS_WARN_IF(!aRange->GetStartContainer())) {
-      return NS_ERROR_FAILURE;
-    }
-    if (NS_WARN_IF(!aRange->GetChildAtStartOffset())) {
-      return NS_ERROR_FAILURE;
-    }
-    if (NS_WARN_IF(!aRange->GetEndContainer())) {
-      return NS_ERROR_FAILURE;
-    }
-    return NS_SUCCESS_EDITOR_ELEMENT_NOT_FOUND;
-  }
-
-  if (!HTMLEditUtils::IsTableCell(element)) {
-    return NS_SUCCESS_EDITOR_ELEMENT_NOT_FOUND;
-  }
-  *aCell = do_AddRef(element).take();
-  return NS_OK;
-}
-
 NS_IMETHODIMP HTMLEditor::GetFirstSelectedCell(
     nsRange** aFirstSelectedRange, Element** aFirstSelectedCellElement) {
   if (NS_WARN_IF(!aFirstSelectedCellElement)) {
@@ -3968,17 +3932,12 @@ already_AddRefed<Element> HTMLEditor::GetFirstSelectedTableCellElement(
   // XXX It must be unclear when this is reset...
   mSelectedCellIndex = 0;
 
-  RefPtr<Element> selectedCell;
-  nsresult rv =
-      HTMLEditor::GetCellFromRange(firstRange, getter_AddRefs(selectedCell));
-  if (NS_FAILED(rv)) {
-    // This case occurs only when Selection is in a text node in normal cases.
-    return nullptr;
-  }
+  RefPtr<Element> selectedCell =
+      HTMLEditUtils::GetTableCellElementIfOnlyOneSelected(*firstRange);
   if (!selectedCell) {
-    // This case means that the range does not select only a cell.
-    // E.g., selects non-table cell element, selects two or more cells, or
-    //       does not select any cell element.
+    // This case means that Selection is in a text node in normal cases or
+    // the range does not select only a cell.  E.g., selects non-table cell
+    // element, selects two or more cells, or does not select any cell element.
     return nullptr;
   }
 
@@ -4048,18 +4007,17 @@ already_AddRefed<Element> HTMLEditor::GetNextSelectedTableCellElement(
       return nullptr;
     }
 
-    RefPtr<Element> nextSelectedCellElement;
-    nsresult rv = HTMLEditor::GetCellFromRange(
-        range, getter_AddRefs(nextSelectedCellElement));
-    if (NS_FAILED(rv)) {
-      // Failure means that the range is in non-element node, e.g., a text node.
-      // Returns nullptr without error if not found.
+    if (!range->GetStartContainer() || !range->GetChildAtStartOffset() ||
+        !range->GetEndContainer()) {
+      // This means that the range is not positioned or in non-element node,
+      // e.g., a text node.  Returns nullptr without error if not found.
       // XXX Why don't we just skip such range or incrementing
       //     mSelectedCellIndex for next call?
       return nullptr;
     }
 
-    if (nextSelectedCellElement) {
+    if (RefPtr<Element> nextSelectedCellElement =
+            HTMLEditUtils::GetTableCellElementIfOnlyOneSelected(*range)) {
       mSelectedCellIndex++;
       return nextSelectedCellElement.forget();
     }