Bug 1484127 - part 1: Create HTMLEditor::GetTableCellElementAt() for internal use of nsITableEditor::GetCellAt() r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 23 Aug 2018 06:39:30 +0000
changeset 488097 3d27e4213ccc750e392a6b5717247886983e8d5a
parent 488096 b95c3522fa1e72dcd5f50726a08477521ab1e951
child 488098 6fe10cab33ca07a9b6ce8e9a0ce5e41a672945e5
push id9719
push userffxbld-merge
push dateFri, 24 Aug 2018 17:49:46 +0000
treeherdermozilla-beta@719ec98fba77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1484127
milestone63.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 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
editor/libeditor/HTMLEditor.h
editor/libeditor/HTMLTableEditor.cpp
editor/nsITableEditor.idl
--- a/editor/libeditor/HTMLEditor.cpp
+++ b/editor/libeditor/HTMLEditor.cpp
@@ -1191,20 +1191,24 @@ HTMLEditor::TabInTable(bool inIsShift,
     RefPtr<Selection> selection;
     RefPtr<Element> tblElement, cell;
     int32_t row;
     rv = GetCellContext(getter_AddRefs(selection),
                         getter_AddRefs(tblElement),
                         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)
     if (cell) {
       selection->Collapse(cell, 0);
     }
   }
 
--- a/editor/libeditor/HTMLEditor.h
+++ b/editor/libeditor/HTMLEditor.h
@@ -1100,16 +1100,39 @@ protected: // Shouldn't be used by frien
      * Update mRowCount and mColumnCount for aTableOrElementInTable.
      * See above for the detail.
      */
     void Update(HTMLEditor& aHTMLEditor, Element& aTableOrElementInTable,
                 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
    * called, this does nothing but returns NS_OK.
    *
    * @param aClipboardType  nsIClipboard::kGlobalClipboard or
    *                        nsIClipboard::kSelectionClipboard.
    */
   nsresult PasteInternal(int32_t aClipboardType);
@@ -1379,17 +1402,17 @@ protected: // Shouldn't be used by frien
 
   nsresult DeleteTable2(Element* aTable, Selection* aSelection);
   nsresult SetColSpan(Element* aCell, int32_t aColSpan);
   nsresult SetRowSpan(Element* aCell, int32_t aRowSpan);
 
   /**
    * 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
    * deleted.  This doesn't count cells that don't start in the given row (are
    * spanning from row above).
    */
   int32_t GetNumberOfCellsInRow(Element* aTable, int32_t rowIndex);
 
--- a/editor/libeditor/HTMLTableEditor.cpp
+++ b/editor/libeditor/HTMLTableEditor.cpp
@@ -683,17 +683,17 @@ HTMLEditor::InsertTableRow(int32_t aNumb
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }
     }
   }
 
   // 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);
   }
 
   return NS_OK;
 }
 
@@ -1311,20 +1311,19 @@ HTMLEditor::DeleteTableRow(int32_t aNumb
     for (int32_t i = 0; i < aNumber; i++) {
       rv = DeleteRow(table, startRowIndex);
       // If failed in current row, try the next
       if (NS_FAILED(rv)) {
         startRowIndex++;
       }
 
       // 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;
       }
     }
   }
   return NS_OK;
 }
 
 // Helper that doesn't batch or change the selection
 nsresult
@@ -2849,21 +2848,24 @@ HTMLEditor::CellIndexes::Update(Element&
     aRv.Throw(NS_ERROR_FAILURE); // not a cell element.
     return;
   }
 
   aRv = tableCellLayout->GetCellIndexes(mRow, mColumn);
   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)
 int32_t
 HTMLEditor::GetNumberOfCellsInRow(Element* aTable,
                                   int32_t rowIndex)
 {
   int32_t cellCount = 0;
@@ -3001,17 +3003,17 @@ HTMLEditor::GetCellDataAt(Element* aTabl
     table =
       GetElementOrParentByTagNameAtSelection(*selection, *nsGkAtoms::table);
     if (!table) {
       return NS_ERROR_FAILURE;
     }
     aTable = table;
   }
 
-  nsTableWrapperFrame* tableFrame = GetTableFrame(aTable);
+  nsTableWrapperFrame* tableFrame = HTMLEditor::GetTableFrame(aTable);
   NS_ENSURE_TRUE(tableFrame, NS_ERROR_FAILURE);
 
   nsTableCellFrame* cellFrame =
     tableFrame->GetCellFrameAt(aRowIndex, aColIndex);
   if (!cellFrame) {
     return NS_ERROR_FAILURE;
   }
 
@@ -3023,67 +3025,74 @@ HTMLEditor::GetCellDataAt(Element* aTabl
   *aActualRowSpan = tableFrame->GetEffectiveRowSpanAt(aRowIndex, aColIndex);
   *aActualColSpan = tableFrame->GetEffectiveColSpanAt(aRowIndex, aColIndex);
   RefPtr<Element> domCell = cellFrame->GetContent()->AsElement();
   domCell.forget(aCell);
 
   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;
-
-  // 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) {
+  if (NS_WARN_IF(!aCellElement)) {
+    return NS_ERROR_INVALID_ARG;
+  }
+
+  *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)
 nsresult
 HTMLEditor::GetCellSpansAt(Element* aTable,
                            int32_t aRowIndex,
                            int32_t aColIndex,
                            int32_t& aActualRowSpan,
                            int32_t& aActualColSpan)
 {
-  nsTableWrapperFrame* tableFrame = GetTableFrame(aTable);
+  nsTableWrapperFrame* tableFrame = HTMLEditor::GetTableFrame(aTable);
   if (!tableFrame) {
     return NS_ERROR_FAILURE;
   }
   aActualRowSpan = tableFrame->GetEffectiveRowSpanAt(aRowIndex, aColIndex);
   aActualColSpan = tableFrame->GetEffectiveColSpanAt(aRowIndex, aColIndex);
 
   return NS_OK;
 }
@@ -3398,21 +3407,17 @@ HTMLEditor::SetSelectionAfterTableEdit(E
   RefPtr<Selection> selection = GetSelection();
   if (!selection) {
     return;
   }
 
   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
         DebugOnly<nsresult> rv = SelectContentInternal(*selection, *cell);
         NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
           "Failed to select the cell");
         return;
       }
--- a/editor/nsITableEditor.idl
+++ b/editor/nsITableEditor.idl
@@ -180,32 +180,36 @@ interface nsITableEditor : nsISupports
    * @param aColumnCount            Number of column count.
    *                                I.e., if colspan is specified with bigger
    *                                number than actual, the value is used
    *                                as this.
    */
   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
     *   be returned multiple times, once for each location it occupies
     * Examine the returned aStartRowIndex and aStartColIndex to see
     *   if it is in the same layout column or layout row:
     *   A "layout row" is all cells sharing the same top edge
     *   A "layout column" is all cells sharing the same left edge