Bug 1484115 - part 1: Create HTMLEditor::GetNextTableRowElement() for internal use of nsITableEditor::GetNextRow() r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 22 Aug 2018 06:52:07 +0000
changeset 487854 05f438b601b5e6d4df5dfef992c2cf1d153757ea
parent 487853 b59a431b833b30bbaf1060937f0bc9ab51b0bf91
child 487855 357d549504fb6e1330bc0aa152cafcd27b0c9a2a
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
bugs1484115
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 1484115 - part 1: Create HTMLEditor::GetNextTableRowElement() for internal use of nsITableEditor::GetNextRow() r=m_kato nsITableEditor::GetNextRow() is an XPCOM method. Therefore, we should have a non-virtual method for internal use of it. This changes the definition in nsITableEditor. First, it allows only <tr> element as what HTMLEditor::GetNextRow() has actually done. Then, changes the return type to Element since it always returns an element node. Differential Revision: https://phabricator.services.mozilla.com/D3948
editor/libeditor/HTMLEditor.h
editor/libeditor/HTMLTableEditor.cpp
editor/nsITableEditor.idl
--- a/editor/libeditor/HTMLEditor.h
+++ b/editor/libeditor/HTMLEditor.h
@@ -988,16 +988,30 @@ protected: // Shouldn't be used by frien
    *                                    However, if <table> does not have
    *                                    <tr> element, returns NS_OK.
    */
   Element*
   GetFirstTableRowElement(Element& aTableOrElementInTable,
                           ErrorResult& aRv) const;
 
   /**
+   * GetNextTableRowElement() returns next <tr> element of aTableRowElement.
+   * This won't cross <table> element boundary but may cross table section
+   * elements like <tbody>.
+   *
+   * @param aTableRowElement    A <tr> element.
+   * @param aRv                 Returns error.  If given element is <tr> but
+   *                            there is no next <tr> element, this returns
+   *                            nullptr but does not return error.
+   */
+  Element*
+  GetNextTableRowElement(Element& aTableRowElement,
+                         ErrorResult& aRv) 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);
--- a/editor/libeditor/HTMLTableEditor.cpp
+++ b/editor/libeditor/HTMLTableEditor.cpp
@@ -273,65 +273,93 @@ HTMLEditor::GetFirstTableRowElement(Elem
       }
     }
   }
   // Don't return error when there is no <tr> element in the <table>.
   return nullptr;
 }
 
 NS_IMETHODIMP
-HTMLEditor::GetNextRow(nsINode* aCurrentRowNode,
-                       nsINode** aRowNode)
+HTMLEditor::GetNextRow(Element* aTableRowElement,
+                       Element** aNextTableRowElement)
 {
-  NS_ENSURE_TRUE(aRowNode, NS_ERROR_NULL_POINTER);
-
-  *aRowNode = nullptr;
-
-  NS_ENSURE_TRUE(aCurrentRowNode, NS_ERROR_NULL_POINTER);
-
-  if (!HTMLEditUtils::IsTableRow(aCurrentRowNode)) {
-    return NS_ERROR_FAILURE;
+  if (NS_WARN_IF(!aTableRowElement) || NS_WARN_IF(!aNextTableRowElement)) {
+    return NS_ERROR_INVALID_ARG;
+  }
+  ErrorResult error;
+  RefPtr<Element> nextRowElement =
+    GetNextTableRowElement(*aTableRowElement, error);
+  if (NS_WARN_IF(error.Failed())) {
+    return error.StealNSResult();
   }
-
-  nsIContent* nextRow = aCurrentRowNode->GetNextSibling();
-
-  // Skip over any textnodes here
-  while (nextRow && !HTMLEditUtils::IsTableRow(nextRow)) {
-    nextRow = nextRow->GetNextSibling();
+  nextRowElement.forget(aNextTableRowElement);
+  return NS_OK;
+}
+
+Element*
+HTMLEditor::GetNextTableRowElement(Element& aTableRowElement,
+                                   ErrorResult& aRv) const
+{
+  MOZ_ASSERT(!aRv.Failed());
+
+  if (NS_WARN_IF(!aTableRowElement.IsHTMLElement(nsGkAtoms::tr))) {
+    aRv.Throw(NS_ERROR_FAILURE);
+    return nullptr;
   }
-  if (nextRow) {
-    *aRowNode = do_AddRef(nextRow).take();
-    return NS_OK;
+
+  for (nsIContent* maybeNextRow = aTableRowElement.GetNextSibling();
+       maybeNextRow;
+       maybeNextRow = maybeNextRow->GetNextSibling()) {
+    if (maybeNextRow->IsHTMLElement(nsGkAtoms::tr)) {
+      return maybeNextRow->AsElement();
+    }
   }
 
-  // No row found, search for rows in other table sections
-  nsINode* rowParent = aCurrentRowNode->GetParentNode();
-  NS_ENSURE_TRUE(rowParent, NS_ERROR_NULL_POINTER);
-
-  nsIContent* parentSibling = rowParent->GetNextSibling();
-
-  while (parentSibling) {
-    nextRow = parentSibling->GetFirstChild();
-
-    // We can encounter textnodes here -- must find a row
-    while (nextRow && !HTMLEditUtils::IsTableRow(nextRow)) {
-      nextRow = nextRow->GetNextSibling();
+  // In current table section (e.g., <tbody>), there is no <tr> element.
+  // Then, check the following table sections.
+  Element* parentElementOfRow = aTableRowElement.GetParentElement();
+  if (NS_WARN_IF(!parentElementOfRow)) {
+    aRv.Throw(NS_ERROR_FAILURE);
+    return nullptr;
+  }
+
+  // Basically, <tr> elements should be in table section elements even if
+  // they are not written in the source explicitly.  However, for preventing
+  // cross table boundary, check it now.
+  if (parentElementOfRow->IsHTMLElement(nsGkAtoms::table)) {
+    // Don't return error since this means just not found.
+    return nullptr;
+  }
+
+  for (nsIContent* maybeNextTableSection = parentElementOfRow->GetNextSibling();
+       maybeNextTableSection;
+       maybeNextTableSection = maybeNextTableSection->GetNextSibling()) {
+    // If the sibling of parent of given <tr> is a table section element,
+    // check its children.
+    if (maybeNextTableSection->IsAnyOfHTMLElements(nsGkAtoms::tbody,
+                                                   nsGkAtoms::thead,
+                                                   nsGkAtoms::tfoot)) {
+      for (nsIContent* maybeNextRow = maybeNextTableSection->GetFirstChild();
+           maybeNextRow;
+           maybeNextRow = maybeNextRow->GetNextSibling()) {
+        if (maybeNextRow->IsHTMLElement(nsGkAtoms::tr)) {
+          return maybeNextRow->AsElement();
+        }
+      }
     }
-    if (nextRow) {
-      *aRowNode = do_AddRef(nextRow).take();
-      return NS_OK;
+    // I'm not sure whether this is a possible case since table section
+    // elements are created automatically.  However, DOM API may create
+    // <tr> elements without table section elements.  So, let's check it.
+    else if (maybeNextTableSection->IsHTMLElement(nsGkAtoms::tr)) {
+      return maybeNextTableSection->AsElement();
     }
-
-    // We arrive here only if a table section has no children
-    //  or first child of section is not a row (bad HTML or more "_moz_text" nodes!)
-    // So look for another section sibling
-    parentSibling = parentSibling->GetNextSibling();
   }
-  // If here, row was not found
-  return NS_SUCCESS_EDITOR_ELEMENT_NOT_FOUND;
+  // Don't return error when the given <tr> element is the last <tr> element in
+  // the <table>.
+  return nullptr;
 }
 
 nsresult
 HTMLEditor::GetLastCellInRow(nsINode* aRowNode,
                              nsINode** aCellNode)
 {
   NS_ENSURE_TRUE(aCellNode, NS_ERROR_NULL_POINTER);
 
@@ -412,17 +440,18 @@ HTMLEditor::InsertTableColumn(int32_t aN
 
   // If we are inserting after all existing columns
   // Make sure table is "well formed"
   //  before appending new column
   if (startColIndex >= colCount) {
     NormalizeTable(table);
   }
 
-  nsCOMPtr<nsINode> rowNode;
+  ErrorResult error;
+  RefPtr<Element> rowElement;
   for (rowIndex = 0; rowIndex < rowCount; rowIndex++) {
     if (startColIndex < colCount) {
       // We are inserting before an existing column
       rv = GetCellDataAt(table, rowIndex, startColIndex,
                          getter_AddRefs(curCell),
                          &curStartRowIndex, &curStartColIndex,
                          &rowSpan, &colSpan,
                          &actualRowSpan, &actualColSpan, &isSelected);
@@ -445,35 +474,41 @@ HTMLEditor::InsertTableColumn(int32_t aN
           // Insert a new cell before current one
           selection->Collapse(curCell, 0);
           rv = InsertTableCell(aNumber, false);
         }
       }
     } else {
       // Get current row and append new cells after last cell in row
       if (!rowIndex) {
-        ErrorResult error;
-        rowNode = GetFirstTableRowElement(*table, error);
+        rowElement = GetFirstTableRowElement(*table, error);
         if (NS_WARN_IF(error.Failed())) {
           return error.StealNSResult();
         }
       } else {
-        nsCOMPtr<nsINode> nextRow;
-        rv = GetNextRow(rowNode, getter_AddRefs(nextRow));
+        if (NS_WARN_IF(!rowElement)) {
+          // XXX Looks like that when rowIndex is 0, startColIndex is always
+          //     same as or larger than colCount.  Is it true?
+          return NS_ERROR_FAILURE;
+        }
+        rowElement = GetNextTableRowElement(*rowElement, error);
+        if (NS_WARN_IF(error.Failed())) {
+          return error.StealNSResult();
+        }
+      }
+
+      if (rowElement) {
+        nsCOMPtr<nsINode> lastCell;
+        rv = GetLastCellInRow(rowElement, getter_AddRefs(lastCell));
         if (NS_WARN_IF(NS_FAILED(rv))) {
           return rv;
         }
-        rowNode = nextRow;
-      }
-
-      if (rowNode) {
-        nsCOMPtr<nsINode> lastCell;
-        rv = GetLastCellInRow(rowNode, getter_AddRefs(lastCell));
-        NS_ENSURE_SUCCESS(rv, rv);
-        NS_ENSURE_TRUE(lastCell, NS_ERROR_FAILURE);
+        if (NS_WARN_IF(!lastCell)) {
+          return NS_ERROR_FAILURE;
+        }
 
         curCell = lastCell->AsElement();
         // Simply add same number of cells to each row
         // Although tempted to check cell indexes for curCell,
         //  the effects of COLSPAN>1 in some cells makes this futile!
         // We must use NormalizeTable first to assure
         //  that there are cells in each cellmap location
         selection->Collapse(curCell, 0);
--- a/editor/nsITableEditor.idl
+++ b/editor/nsITableEditor.idl
@@ -218,27 +218,26 @@ interface nsITableEditor : nsISupports
    *                                does not have <tr> elements, returns
    *                                null without throwing exception.
    *                                Note that this may return anonymous <tr>
    *                                element if <table> has one or more cells
    *                                but <tr> element is not in the source.
    */
   Element getFirstRow(in Element aTableElement);
 
-  /** Get the next row element starting the search from aTableElement
-    *
-    * @param aTableElement Any TR or child-of-TR element in the document
-    *
-    * @return            The row to start search from
-    *                    and the row returned from the search
-    *                    Returns null if there isn't another row
-    * (in C++ returns: NS_EDITOR_ELEMENT_NOT_FOUND if an element is not found
-    *  passes NS_SUCCEEDED macro)
-    */
-  Node getNextRow(in Node aTableElement);
+  /**
+   * getNextRow() returns next <tr> element of given <tr> element.
+   *
+   * @param aTableRowElement     Must be a <tr> element.
+   * @return                     <tr> element next to aTableRowElement if there
+   *                             is.  Note that this may be in different
+   *                             table section element (e.g., <tbody>).
+   *                             However, never crosses <table> boundary.
+   */
+  Element getNextRow(in Element aTableElement);
 
   /** Preferred direction to search for neighboring cell
     * when trying to locate a cell to place caret in after
     * a table editing action.
     * Used for aDirection param in SetSelectionAfterTableEdit
     */
 
   /** Examine the current selection and find