Bug 1484113 - part 1: Create HTMLEditor::GetFirstTableRowElement() for internal use of nsITableEditor::GetFirstRow() r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 22 Aug 2018 01:20:23 +0000
changeset 481083 090edc3b798b91e876b40da371c515397743f5fd
parent 481082 af33be3aee822ddf186798da08ca3d9863557b30
child 481084 5e7484d3b2e3ad6ec39e7af04601742a3ff7643e
push id232
push userfmarier@mozilla.com
push dateWed, 05 Sep 2018 20:45:54 +0000
reviewersm_kato
bugs1484113
milestone63.0a1
Bug 1484113 - part 1: Create HTMLEditor::GetFirstTableRowElement() for internal use of nsITableEditor::GetFirstRow() r=m_kato nsITableEditor::GetFirstRow() is an XPCOM method, so, for internal use, we should create non-virtual method, that is GetFirstTableRowElement(). This patch makes it never return NS_SUCCESS_EDITOR_ELEMENT_NOT_FOUND since nobody refers it and it's detectable. If the method returns nullptr without error, it's the case of NS_SUCCESS_EDITOR_ELEMENT_NOT_FOUND. Additionally, this patch changes the return type of GetFirstRow() from Node to Element since it always return an Element node if not null. Differential Revision: https://phabricator.services.mozilla.com/D3780
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
@@ -2556,33 +2556,33 @@ HTMLEditor::Align(const nsAString& aAlig
     return rv;
   }
 
   return rules->DidDoAction(selection, subActionInfo, rv);
 }
 
 Element*
 HTMLEditor::GetElementOrParentByTagName(const nsAtom& aTagName,
-                                        nsINode* aNode)
+                                        nsINode* aNode) const
 {
   MOZ_ASSERT(&aTagName != nsGkAtoms::_empty);
 
   if (aNode) {
     return GetElementOrParentByTagNameInternal(aTagName, *aNode);
   }
   RefPtr<Selection> selection = GetSelection();
   if (NS_WARN_IF(!selection)) {
     return nullptr;
   }
   return GetElementOrParentByTagNameAtSelection(*selection, aTagName);
 }
 
 Element*
 HTMLEditor::GetElementOrParentByTagNameAtSelection(Selection& aSelection,
-                                                   const nsAtom& aTagName)
+                                                   const nsAtom& aTagName) const
 {
   MOZ_ASSERT(&aTagName != nsGkAtoms::_empty);
 
   // If no node supplied, get it from anchor node of current selection
   const EditorRawDOMPoint atAnchor(aSelection.AnchorRef());
   if (NS_WARN_IF(!atAnchor.IsSet())) {
     return nullptr;
   }
@@ -2600,17 +2600,17 @@ HTMLEditor::GetElementOrParentByTagNameA
       return nullptr;
     }
   }
   return GetElementOrParentByTagNameInternal(aTagName, *node);
 }
 
 Element*
 HTMLEditor::GetElementOrParentByTagNameInternal(const nsAtom& aTagName,
-                                                nsINode& aNode)
+                                                nsINode& aNode) const
 {
   MOZ_ASSERT(&aTagName != nsGkAtoms::_empty);
 
   Element* currentElement =
     aNode.IsElement() ? aNode.AsElement() : aNode.GetParentElement();
   if (NS_WARN_IF(!currentElement)) {
     // Neither aNode nor its parent is an element, so no ancestor is
     MOZ_ASSERT(!aNode.GetParentNode() ||
--- a/editor/libeditor/HTMLEditor.h
+++ b/editor/libeditor/HTMLEditor.h
@@ -397,17 +397,17 @@ public:
    *                        has "name" attribute with non-empty value.
    * @param aNode           If non-nullptr, this starts to look for the result
    *                        from it.  Otherwise, i.e., nullptr, starts from
    *                        anchor node of Selection.
    * @return                If an element which matches aTagName, returns
    *                        an Element.  Otherwise, nullptr.
    */
   Element*
-  GetElementOrParentByTagName(const nsAtom& aTagName, nsINode* aNode);
+  GetElementOrParentByTagName(const nsAtom& aTagName, nsINode* aNode) const;
 
   /**
     * Get an active editor's editing host in DOM window.  If this editor isn't
     * active in the DOM window, this returns NULL.
     */
   Element* GetActiveEditingHost() const;
 
 protected: // May be called by friends.
@@ -907,17 +907,17 @@ protected: // Shouldn't be used by frien
    *                        which has "href" attribute with non-empty value.
    *                        If nsGkAtoms::anchor, the result may be <a> which
    *                        has "name" attribute with non-empty value.
    * @return                If an element which matches aTagName, returns
    *                        an Element.  Otherwise, nullptr.
    */
   Element*
   GetElementOrParentByTagNameAtSelection(Selection& aSelection,
-                                         const nsAtom& aTagName);
+                                         const nsAtom& aTagName) const;
 
   /**
    * GetElementOrParentByTagNameInternal() looks for an element node whose
    * name matches aTagName from aNode to <body> element.
    *
    * @param aTagName        The tag name which you want to look for.
    *                        Must not be nsGkAtoms::_empty.
    *                        If nsGkAtoms::list, the result may be <ul>, <ol> or
@@ -928,17 +928,17 @@ protected: // Shouldn't be used by frien
    *                        If nsGkAtoms::anchor, the result may be <a> which
    *                        has "name" attribute with non-empty value.
    * @param aNode           Start node to look for the element.
    * @return                If an element which matches aTagName, returns
    *                        an Element.  Otherwise, nullptr.
    */
   Element*
   GetElementOrParentByTagNameInternal(const nsAtom& aTagName,
-                                      nsINode& aNode);
+                                      nsINode& aNode) const;
 
   /**
    * GetSelectedElement() returns an element node which is in first range of
    * aSelection.  The rule is a little bit complicated and the rules do not
    * make sense except in a few cases.  If you want to use this newly,
    * you should create new method instead.  This needs to be here for
    * comm-central.
    * The rules are:
@@ -968,16 +968,36 @@ protected: // Shouldn't be used by frien
    * @return                    An element in first range of aSelection.
    */
   already_AddRefed<Element>
   GetSelectedElement(Selection& aSelection,
                      const nsAtom* aTagName,
                      ErrorResult& aRv);
 
   /**
+   * GetFirstTableRowElement() returns the first <tr> element in the most
+   * nearest ancestor of aTableOrElementInTable or itself.
+   *
+   * @param aTableOrElementInTable      <table> element or another element.
+   *                                    If this is a <table> element, returns
+   *                                    first <tr> element in it.  Otherwise,
+   *                                    returns first <tr> element in nearest
+   *                                    ancestor <table> element.
+   * @param aRv                         Returns an error code.  When
+   *                                    aTableOrElementInTable is neither
+   *                                    <table> nor in a <table> element,
+   *                                    returns NS_ERROR_FAILURE.
+   *                                    However, if <table> does not have
+   *                                    <tr> element, returns NS_OK.
+   */
+  Element*
+  GetFirstTableRowElement(Element& aTableOrElementInTable,
+                          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
@@ -216,62 +216,70 @@ HTMLEditor::InsertTableCell(int32_t aNum
     }
   }
   // XXX This is perhaps the result of the last call of
   //     InsertNodeWithTransaction() or CreateElementWithDefaults().
   return rv;
 }
 
 NS_IMETHODIMP
-HTMLEditor::GetFirstRow(Element* aTableElement,
-                        nsINode** aRowNode)
+HTMLEditor::GetFirstRow(Element* aTableOrElementInTable,
+                        Element** aFirstRowElement)
 {
-  if (NS_WARN_IF(!aTableElement) || NS_WARN_IF(!aRowNode)) {
+  if (NS_WARN_IF(!aTableOrElementInTable) || NS_WARN_IF(!aFirstRowElement)) {
     return NS_ERROR_INVALID_ARG;
   }
-
-  *aRowNode = nullptr;
+  ErrorResult error;
+  RefPtr<Element> firstRowElement =
+    GetFirstTableRowElement(*aTableOrElementInTable, error);
+  if (NS_WARN_IF(error.Failed())) {
+    return error.StealNSResult();
+  }
+  firstRowElement.forget(aFirstRowElement);
+  return NS_OK;
+}
+
+Element*
+HTMLEditor::GetFirstTableRowElement(Element& aTableOrElementInTable,
+                                    ErrorResult& aRv) const
+{
+  MOZ_ASSERT(!aRv.Failed());
 
   Element* tableElement =
-    GetElementOrParentByTagNameInternal(*nsGkAtoms::table, *aTableElement);
+    GetElementOrParentByTagNameInternal(*nsGkAtoms::table,
+                                        aTableOrElementInTable);
+  // If the element is not in <table>, return error.
   if (NS_WARN_IF(!tableElement)) {
-    return NS_ERROR_FAILURE;
+    aRv.Throw(NS_ERROR_FAILURE);
+    return nullptr;
   }
 
-  nsCOMPtr<nsIContent> tableChild = tableElement->GetFirstChild();
-  while (tableChild) {
+  for (nsIContent* tableChild = tableElement->GetFirstChild();
+       tableChild;
+       tableChild = tableChild->GetNextSibling()) {
     if (tableChild->IsHTMLElement(nsGkAtoms::tr)) {
       // Found a row directly under <table>
-      tableChild.forget(aRowNode);
-      return NS_OK;
+      return tableChild->AsElement();
     }
-    // Look for row in one of the row container elements
+    // <table> can have table section elements like <tbody>.  <tr> elements
+    // may be children of them.
     if (tableChild->IsAnyOfHTMLElements(nsGkAtoms::tbody,
                                         nsGkAtoms::thead,
                                         nsGkAtoms::tfoot)) {
-      nsCOMPtr<nsIContent> rowNode = tableChild->GetFirstChild();
-
-      // We can encounter textnodes here -- must find a row
-      while (rowNode && !HTMLEditUtils::IsTableRow(rowNode)) {
-        rowNode = rowNode->GetNextSibling();
-      }
-
-      if (rowNode) {
-        rowNode.forget(aRowNode);
-        return NS_OK;
+      for (nsIContent* tableSectionChild = tableChild->GetFirstChild();
+           tableSectionChild;
+           tableSectionChild = tableSectionChild->GetNextSibling()) {
+        if (tableSectionChild->IsHTMLElement(nsGkAtoms::tr)) {
+          return tableSectionChild->AsElement();
+        }
       }
     }
-    // Here if table child was a CAPTION or COLGROUP
-    //  or child of a row parent wasn't a row (bad HTML?),
-    //  or first child was a textnode
-    // Look in next table child
-    tableChild = tableChild->GetNextSibling();
   }
-  // If here, row was not found
-  return NS_SUCCESS_EDITOR_ELEMENT_NOT_FOUND;
+  // Don't return error when there is no <tr> element in the <table>.
+  return nullptr;
 }
 
 NS_IMETHODIMP
 HTMLEditor::GetNextRow(nsINode* aCurrentRowNode,
                        nsINode** aRowNode)
 {
   NS_ENSURE_TRUE(aRowNode, NS_ERROR_NULL_POINTER);
 
@@ -437,19 +445,20 @@ 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) {
-        rv = GetFirstRow(table, getter_AddRefs(rowNode));
-        if (NS_WARN_IF(NS_FAILED(rv))) {
-          return rv;
+        ErrorResult error;
+        rowNode = 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(NS_FAILED(rv))) {
           return rv;
         }
         rowNode = nextRow;
--- a/editor/nsITableEditor.idl
+++ b/editor/nsITableEditor.idl
@@ -198,24 +198,35 @@ interface nsITableEditor : nsISupports
   void getCellDataAt(in Element aTable,
                      in long  aRowIndex, in long  aColIndex,
                      out Element aCell,
                      out long  aStartRowIndex, out long  aStartColIndex,
                      out long  aRowSpan, out long  aColSpan,
                      out long  aActualRowSpan, out long  aActualColSpan,
                      out boolean aIsSelected);
 
-  /** Get the first row element in a table
-    *
-    * @return            The row at the requested index
-    *                    Returns null if there are no rows in table
-    * (in C++ returns: NS_EDITOR_ELEMENT_NOT_FOUND if an element is not found
-    *  passes NS_SUCCEEDED macro)
-    */
-  Node getFirstRow(in Element aTableElement);
+  /**
+   * getFirstRow() returns first <tr> element in a <table> element.
+   *
+   * @param aTableOrElementInTable  If a <table> element, returns its first
+   *                                <tr> element.
+   *                                If another element, looks for nearest
+   *                                ancestor <table> element first.  Then,
+   *                                return its first <tr> element.
+   * @return                        <tr> element in the <table> element.
+   *                                If <table> element is not found, this
+   *                                throws an exception.
+   *                                If there is a <table> element but it
+   *                                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