Bug 1484127 - part 1: Create HTMLEditor::GetTableCellElementAt() for …

…internal use of nsITableEditor::GetCellAt() r=m_kato

nsITableEditor::GetCellAt() is an XPCOM method, but this is called internally
a lot.  So, we should create non-virtual method for internal use.

The XPCOM method retrieves a <table> element if given element is nullptr.
However, no internal user needs this feature.  So, we can create
GetTableCellElementAt() simply.  Then, we can get rid of nsresult and
ErrorResult from it.

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

editor/libeditor/HTMLEditor.cpp

@@ -1196,10 +1196,14 @@ HTMLEditor::TabInTable(bool inIsShift,
                         getter_AddRefs(cell),
                         nullptr, nullptr,
                         &row, nullptr);
-    NS_ENSURE_SUCCESS(rv, rv);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
+    if (NS_WARN_IF(!tblElement)) {
+      return NS_ERROR_FAILURE;
+    }
     // ...so that we can ask for first cell in that row...
-    rv = GetCellAt(tblElement, row, 0, getter_AddRefs(cell));
-    NS_ENSURE_SUCCESS(rv, rv);
+    cell = GetTableCellElementAt(*tblElement, row, 0);
     // ...and then set selection there.  (Note that normally you should use
     // CollapseSelectionToDeepestNonTableFirstChild(), but we know cell is an
     // empty new cell, so this works fine)

editor/libeditor/HTMLEditor.h

@@ -1104,6 +1104,29 @@ class HTMLEditor final : public TextEditor
                 ErrorResult& aRv);
   };
 
+  /**
+   * GetTableCellElementAt() returns a <td> or <th> element of aTableElement
+   * if there is a cell at the indexes.
+   *
+   * @param aTableElement       Must be a <table> element.
+   * @param aCellIndexes        Indexes of cell which you want.
+   *                            If rowspan and/or colspan is specified 2 or
+   *                            larger, any indexes are allowed to retrieve
+   *                            the cell in the area.
+   * @return                    The cell element if there is in the <table>.
+   *                            Returns nullptr without error if the indexes
+   *                            are out of bounds.
+   */
+  Element* GetTableCellElementAt(Element& aTableElement,
+                                 const CellIndexes& aCellIndexes) const
+  {
+    return GetTableCellElementAt(aTableElement, aCellIndexes.mRow,
+                                 aCellIndexes.mColumn);
+  }
+  Element* GetTableCellElementAt(Element& aTableElement,
+                                 int32_t aRowIndex,
+                                 int32_t aColumnIndex) const;
+
   /**
    * PasteInternal() pasts text with replacing selected content.
    * This tries to dispatch ePaste event first.  If its defaultPrevent() is
@@ -1384,7 +1407,7 @@ class HTMLEditor final : public TextEditor
   /**
    * Helper used to get nsTableWrapperFrame for a table.
    */
-  nsTableWrapperFrame* GetTableFrame(Element* aTable);
+  static nsTableWrapperFrame* GetTableFrame(Element* aTable);
 
   /**
    * Needed to do appropriate deleting when last cell or row is about to be

editor/libeditor/HTMLTableEditor.cpp

@@ -688,7 +688,7 @@ HTMLEditor::InsertTableRow(int32_t aNumber,
 
   // SetSelectionAfterTableEdit from AutoSelectionSetterAfterTableEdit will
   // access frame selection, so we need reframe.
-  // Because GetCellAt depends on frame.
+  // Because GetTableCellElementAt() depends on frame.
   nsCOMPtr<nsIPresShell> ps = GetPresShell();
   if (ps) {
     ps->FlushPendingNotifications(FlushType::Frames);
@@ -1316,10 +1316,9 @@ HTMLEditor::DeleteTableRow(int32_t aNumber)
       }
 
       // Check if there's a cell in the "next" row
-      rv = GetCellAt(table, startRowIndex, startColIndex, getter_AddRefs(cell));
-      NS_ENSURE_SUCCESS(rv, rv);
+      cell = GetTableCellElementAt(*table, startRowIndex, startColIndex);
       if (!cell) {
-        break;
+        return NS_OK;
       }
     }
   }
@@ -2854,11 +2853,14 @@ HTMLEditor::CellIndexes::Update(Element& aCellElement,
   NS_WARNING_ASSERTION(!aRv.Failed(), "Failed to get cell indexes");
 }
 
+// static
 nsTableWrapperFrame*
-HTMLEditor::GetTableFrame(Element* aTable)
+HTMLEditor::GetTableFrame(Element* aTableElement)
 {
-  NS_ENSURE_TRUE(aTable, nullptr);
-  return do_QueryFrame(aTable->GetPrimaryFrame());
+  if (NS_WARN_IF(!aTableElement)) {
+    return nullptr;
+  }
+  return do_QueryFrame(aTableElement->GetPrimaryFrame());
 }
 
 //Return actual number of cells (a cell with colspan > 1 counts as just 1)
@@ -3006,7 +3008,7 @@ HTMLEditor::GetCellDataAt(Element* aTable,
     aTable = table;
   }
 
-  nsTableWrapperFrame* tableFrame = GetTableFrame(aTable);
+  nsTableWrapperFrame* tableFrame = HTMLEditor::GetTableFrame(aTable);
   NS_ENSURE_TRUE(tableFrame, NS_ERROR_FAILURE);
 
   nsTableCellFrame* cellFrame =
@@ -3028,46 +3030,53 @@ HTMLEditor::GetCellDataAt(Element* aTable,
   return NS_OK;
 }
 
-// When all you want is the cell
 NS_IMETHODIMP
-HTMLEditor::GetCellAt(Element* aTable,
+HTMLEditor::GetCellAt(Element* aTableElement,
                       int32_t aRowIndex,
-                      int32_t aColIndex,
-                      Element** aCell)
+                      int32_t aColumnIndex,
+                      Element** aCellElement)
 {
-  NS_ENSURE_ARG_POINTER(aCell);
-  *aCell = nullptr;
+  if (NS_WARN_IF(!aCellElement)) {
+    return NS_ERROR_INVALID_ARG;
+  }
 
-  // Needs to live as long as we use aTable
-  // XXX Really? Looks like it's safe to use raw pointer here.
-  //     However, layout code change won't be handled by editor developers
-  //     so that it must be safe to keep using RefPtr here.
-  RefPtr<Element> table;
-  if (!aTable) {
+  *aCellElement = nullptr;
+
+  Element* tableElement = aTableElement;
+  if (!tableElement) {
     RefPtr<Selection> selection = GetSelection();
     if (NS_WARN_IF(!selection)) {
       return NS_ERROR_FAILURE;
     }
     // Get the selected table or the table enclosing the selection anchor.
-    table =
+    tableElement =
       GetElementOrParentByTagNameAtSelection(*selection, *nsGkAtoms::table);
-    if (NS_WARN_IF(!table)) {
+    if (NS_WARN_IF(!tableElement)) {
       return NS_ERROR_FAILURE;
     }
-    aTable = table;
   }
 
-  nsTableWrapperFrame* tableFrame = GetTableFrame(aTable);
+  RefPtr<Element> cellElement =
+    GetTableCellElementAt(*tableElement, aRowIndex, aColumnIndex);
+  cellElement.forget(aCellElement);
+  return NS_OK;
+}
+
+Element*
+HTMLEditor::GetTableCellElementAt(Element& aTableElement,
+                                  int32_t aRowIndex,
+                                  int32_t aColumnIndex) const
+{
+  // Let's grab the <table> element while we're retrieving layout API since
+  // editor developers do not watch all layout API changes.  So, it may
+  // become unsafe.
+  OwningNonNull<Element> tableElement(aTableElement);
+  nsTableWrapperFrame* tableFrame = HTMLEditor::GetTableFrame(tableElement);
   if (!tableFrame) {
-    *aCell = nullptr;
-    return NS_SUCCESS_EDITOR_ELEMENT_NOT_FOUND;
+    return nullptr;
   }
-
-  nsIContent* cell = tableFrame->GetCellAt(aRowIndex, aColIndex);
-  RefPtr<Element> cellElement = cell ? cell->AsElement() : nullptr;
-  cellElement.forget(aCell);
-
-  return NS_OK;
+  nsIContent* cell = tableFrame->GetCellAt(aRowIndex, aColumnIndex);
+  return Element::FromNodeOrNull(cell);
 }
 
 // When all you want are the rowspan and colspan (not exposed in nsITableEditor)
@@ -3078,7 +3087,7 @@ HTMLEditor::GetCellSpansAt(Element* aTable,
                            int32_t& aActualRowSpan,
                            int32_t& aActualColSpan)
 {
-  nsTableWrapperFrame* tableFrame = GetTableFrame(aTable);
+  nsTableWrapperFrame* tableFrame = HTMLEditor::GetTableFrame(aTable);
   if (!tableFrame) {
     return NS_ERROR_FAILURE;
   }
@@ -3403,11 +3412,7 @@ HTMLEditor::SetSelectionAfterTableEdit(Element* aTable,
   RefPtr<Element> cell;
   bool done = false;
   do {
-    nsresult rv = GetCellAt(aTable, aRow, aCol, getter_AddRefs(cell));
-    if (NS_FAILED(rv)) {
-      break;
-    }
-
+    cell = GetTableCellElementAt(*aTable, aRow, aCol);
     if (cell) {
       if (aSelected) {
         // Reselect the cell

editor/nsITableEditor.idl

@@ -185,22 +185,26 @@ interface nsITableEditor : nsISupports
   void getTableSize(in Element aTableOrElementInTable,
                     out long aRowCount, out long aColCount);
 
-  /** Get a cell element at cellmap grid coordinates
-    * A cell that spans across multiple cellmap locations will
-    *   be returned multiple times, once for each location it occupies
-    *
-    * @param aTable                   A table in the document
-    * @param aRowIndex, aColIndex     The 0-based cellmap indexes
-    *
-    * (in C++ returns: NS_EDITOR_ELEMENT_NOT_FOUND if an element is not found
-    *  passes NS_SUCCEEDED macro)
-    *
-    *   You can scan for all cells in a row or column
-    *   by iterating through the appropriate indexes
-    *   until the returned aCell is null
-    */
-  Element getCellAt(in Element aTable,
-                    in long aRowIndex, in long aColIndex);
+  /**
+   * getCellAt() returns a <td> or <th> element in a <table> if there is a
+   * cell at the indexes.
+   *
+   * @param aTableElement       If not null, must be a <table> element.
+   *                            If null, looks for the nearest ancestor <table>
+   *                            to look for a cell.
+   * @param aRowIndex           Row index of the cell.
+   * @param aColumnIndex        Column index of the cell.
+   * @return                    Returns a <td> or <th> element if there is.
+   *                            Otherwise, returns null without throwing
+   *                            exception.
+   *                            If aTableElement is not null and not a <table>
+   *                            element, throwing an exception.
+   *                            If aTableElement is null and anchor of Selection
+   *                            is not in any <table> element, throwing an
+   *                            exception.
+   */
+  Element getCellAt(in Element aTableElement,
+                    in long aRowIndex, in long aColumnIndex);
 
   /** Get a cell at cellmap grid coordinates and associated data
     * A cell that spans across multiple cellmap locations will