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 481102 05f438b601b5e6d4df5dfef992c2cf1d153757ea
parent 481101 b59a431b833b30bbaf1060937f0bc9ab51b0bf91
child 481103 357d549504fb6e1330bc0aa152cafcd27b0c9a2a
push id232
push userfmarier@mozilla.com
push dateWed, 05 Sep 2018 20:45:54 +0000
reviewersm_kato
bugs1484115
milestone63.0a1
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