Bug 1730442 - part 6: Make HTMLEditor::InsertTableColumnsWithTransaction() collect all necessary data before touching the DOM tree r=m_kato

It needs to work with the latest layout information to consider which cell
element is the insertion point due to rowspan and colspan.  Therefore,
this patch makes it collects all cell data before touching the DOM except
the case that it needs to normalize the table to make it rectanble.

Note that the case requiring the normalizer should be fixed in a later patch.

This method is corresponding to an XPCOM method.  Therefore, this is tested
by `test_nsITableEditor_insertTableColumn.html`.

And also it's used by the inline table editor, but we don't have automated tests
for this because of no API to get the buttons.  Therefore, I tested it by my
hand.

Note that the old code fails to put caret to newly inserted cell at the reftest
situation.  This fixes the bug too.  Therefore, this changes the reftest's
reference.

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

editor/libeditor/HTMLEditUtils.h

@@ -1384,6 +1384,54 @@ class HTMLEditUtils final {
     return nullptr;
   }
 
+  /**
+   * GetFirstTableCellElementChild() and GetLastTableCellElementChild()
+   * return the first/last element child of <tr> element if it's a table
+   * cell element.
+   */
+  static Element* GetFirstTableCellElementChild(
+      const Element& aTableRowElement) {
+    MOZ_ASSERT(aTableRowElement.IsHTMLElement(nsGkAtoms::tr));
+    Element* firstElementChild = aTableRowElement.GetFirstElementChild();
+    return firstElementChild && HTMLEditUtils::IsTableCell(firstElementChild)
+               ? firstElementChild
+               : nullptr;
+  }
+  static Element* GetLastTableCellElementChild(
+      const Element& aTableRowElement) {
+    MOZ_ASSERT(aTableRowElement.IsHTMLElement(nsGkAtoms::tr));
+    Element* lastElementChild = aTableRowElement.GetLastElementChild();
+    return lastElementChild && HTMLEditUtils::IsTableCell(lastElementChild)
+               ? lastElementChild
+               : nullptr;
+  }
+
+  /**
+   * GetPreviousTableCellElementSibling() and GetNextTableCellElementSibling()
+   * return a table cell element of previous/next element sibling of given
+   * content node if and only if the element sibling is a table cell element.
+   */
+  static Element* GetPreviousTableCellElementSibling(
+      const nsIContent& aChildOfTableRow) {
+    MOZ_ASSERT(aChildOfTableRow.GetParentNode());
+    MOZ_ASSERT(aChildOfTableRow.GetParentNode()->IsHTMLElement(nsGkAtoms::tr));
+    Element* previousElementSibling =
+        aChildOfTableRow.GetPreviousElementSibling();
+    return previousElementSibling &&
+                   HTMLEditUtils::IsTableCell(previousElementSibling)
+               ? previousElementSibling
+               : nullptr;
+  }
+  static Element* GetNextTableCellElementSibling(
+      const nsIContent& aChildOfTableRow) {
+    MOZ_ASSERT(aChildOfTableRow.GetParentNode());
+    MOZ_ASSERT(aChildOfTableRow.GetParentNode()->IsHTMLElement(nsGkAtoms::tr));
+    Element* nextElementSibling = aChildOfTableRow.GetNextElementSibling();
+    return nextElementSibling && HTMLEditUtils::IsTableCell(nextElementSibling)
+               ? nextElementSibling
+               : nullptr;
+  }
+
   /**
    * GetMostDistantAncestorInlineElement() returns the most distant ancestor
    * inline element between aContent and the aEditingHost.  Even if aEditingHost

editor/libeditor/HTMLEditor.h

@@ -3081,6 +3081,9 @@ class HTMLEditor final : public EditorBase,
     [[nodiscard]] bool IsSpannedFromOtherRow() const {
       return mElement && mCurrent.mRow != mFirst.mRow;
     }
+    [[nodiscard]] bool IsNextColumnSpannedFromOtherColumn() const {
+      return mElement && mCurrent.mColumn + 1 < NextColumnIndex();
+    }
 
     /**
      * NextColumnIndex() and NextRowIndex() return column/row index of
@@ -3242,6 +3245,13 @@ class HTMLEditor final : public EditorBase,
   Result<RefPtr<Element>, nsresult> GetSelectedOrParentTableElement(
       bool* aIsCellSelected = nullptr) const;
 
+  /**
+   * GetFirstSelectedCellElementInTable() returns <td> or <th> element at
+   * first selection (using GetSelectedOrParentTableElement).  If found cell
+   * element is not in <table> or <tr> element, this returns nullptr.
+   */
+  Result<RefPtr<Element>, nsresult> GetFirstSelectedCellElementInTable() const;
+
   /**
    * PasteInternal() pasts text with replacing selected content.
    * This tries to dispatch ePaste event first.  If its defaultPrevent() is
@@ -3599,19 +3609,13 @@ class HTMLEditor final : public EditorBase,
                                   int32_t aNumberOfCellsToInsert);
 
   /**
-   * InsertTableColumnsWithTransaction() inserts columns before or after
+   * InsertTableColumnsWithTransaction() inserts cell elements to every rows
-   * a cell element containing first selection range.  I.e., if the cell
+   * at same column index as the cell specified by aPointToInsert.
-   * spans columns and aInsertPosition is eAfterSelectedCell, new columns
-   * will be inserted after the right-most row which contains the cell.
-   * If first selection range is not in table cell element, this does nothing
-   * but does not return error.
    *
    * @param aNumberOfColumnsToInsert    Number of columns to insert.
-   * @param aInsertPosition             Before or after the target cell which
-   *                                    contains first selection range.
    */
-  MOZ_CAN_RUN_SCRIPT nsresult InsertTableColumnsWithTransaction(
+  [[nodiscard]] MOZ_CAN_RUN_SCRIPT nsresult InsertTableColumnsWithTransaction(
-      int32_t aNumberOfColumnsToInsert, InsertPosition aInsertPosition);
+      const EditorDOMPoint& aPointToInsert, int32_t aNumberOfColumnsToInsert);
 
   /**
    * InsertTableRowsWithTransaction() inserts <tr> elements before or after

editor/libeditor/HTMLInlineTableEditor.cpp

@@ -209,7 +209,10 @@ nsresult HTMLEditor::DoInlineTableEditingAction(const Element& aElement) {
     return NS_OK;
   }
 
-  if (NS_WARN_IF(!mInlineEditedCell)) {
+  if (NS_WARN_IF(!mInlineEditedCell) ||
+      NS_WARN_IF(!mInlineEditedCell->IsInComposedDoc()) ||
+      NS_WARN_IF(
+          !HTMLEditUtils::IsTableRow(mInlineEditedCell->GetParentNode()))) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
@@ -244,12 +247,13 @@ nsresult HTMLEditor::DoInlineTableEditingAction(const Element& aElement) {
           "CanHandleAndMaybeDispatchBeforeInputEvent(), failed");
       return EditorBase::ToGenericNSResult(rv);
     }
-    DebugOnly<nsresult> rvIgnored = InsertTableColumnsWithTransaction(
+
-        1, InsertPosition::eBeforeSelectedCell);
+    DebugOnly<nsresult> rvIgnored =
+        InsertTableColumnsWithTransaction(EditorDOMPoint(mInlineEditedCell), 1);
     NS_WARNING_ASSERTION(
         NS_SUCCEEDED(rvIgnored),
-        "HTMLEditor::InsertTableColumnsWithTransaction(1, "
+        "HTMLEditor::InsertTableColumnsWithTransaction("
-        "InsertPosition::eBeforeSelectedCell) failed, but ignored");
+        "EditorDOMPoint(mInlineEditedCell), 1) failed, but ignored");
   } else if (anonclass.EqualsLiteral("mozTableAddColumnAfter")) {
     AutoEditActionDataSetter editActionData(*this,
                                             EditAction::eInsertTableColumn);
@@ -260,12 +264,24 @@ nsresult HTMLEditor::DoInlineTableEditingAction(const Element& aElement) {
           "CanHandleAndMaybeDispatchBeforeInputEvent(), failed");
       return EditorBase::ToGenericNSResult(rv);
     }
+    Element* nextCellElement = nullptr;
+    for (nsIContent* maybeNextCellElement = mInlineEditedCell->GetNextSibling();
+         maybeNextCellElement;
+         maybeNextCellElement = maybeNextCellElement->GetNextSibling()) {
+      if (HTMLEditUtils::IsTableCell(maybeNextCellElement)) {
+        nextCellElement = maybeNextCellElement->AsElement();
+        break;
+      }
+    }
     DebugOnly<nsresult> rvIgnored = InsertTableColumnsWithTransaction(
-        1, InsertPosition::eAfterSelectedCell);
+        nextCellElement
+            ? EditorDOMPoint(nextCellElement)
+            : EditorDOMPoint::AtEndOf(*mInlineEditedCell->GetParentElement()),
+        1);
     NS_WARNING_ASSERTION(
         NS_SUCCEEDED(rvIgnored),
-        "HTMLEditor::InsertTableColumnsWithTransaction(1, "
+        "HTMLEditor::InsertTableColumnsWithTransaction("
-        "InsertPosition::eAfterSelectedCell) failed, but ignored");
+        "EditorDOMPoint(nextCellElement), 1) failed, but ignored");
   } else if (anonclass.EqualsLiteral("mozTableAddRowBefore")) {
     AutoEditActionDataSetter editActionData(*this,
                                             EditAction::eInsertTableRowElement);
@@ -334,7 +350,6 @@ nsresult HTMLEditor::DoInlineTableEditingAction(const Element& aElement) {
     return NS_OK;
   }
 
-  // InsertTableRowsWithTransaction() might causes reframe.
   if (NS_WARN_IF(Destroyed())) {
     return NS_OK;
   }

editor/libeditor/HTMLTableEditor.cpp

@@ -328,25 +328,18 @@ NS_IMETHODIMP HTMLEditor::InsertTableCell(int32_t aNumberOfCellsToInsert,
     return EditorBase::ToGenericNSResult(rv);
   }
 
-  Result<RefPtr<Element>, nsresult> cellOrRowOrTableElementOrError =
+  Result<RefPtr<Element>, nsresult> cellElementOrError =
-      GetSelectedOrParentTableElement();
+      GetFirstSelectedCellElementInTable();
-  if (cellOrRowOrTableElementOrError.isErr()) {
+  if (cellElementOrError.isErr()) {
-    NS_WARNING("HTMLEditor::GetSelectedOrParentTableElement() failed");
+    NS_WARNING("HTMLEditor::GetFirstSelectedCellElementInTable() failed");
-    return EditorBase::ToGenericNSResult(
+    return EditorBase::ToGenericNSResult(cellElementOrError.unwrapErr());
-        cellOrRowOrTableElementOrError.unwrapErr());
   }
 
-  if (!cellOrRowOrTableElementOrError.inspect()) {
+  if (!cellElementOrError.inspect()) {
     return NS_OK;
   }
 
-  if (!HTMLEditUtils::GetClosestAncestorTableElement(
+  EditorDOMPoint pointToInsert(cellElementOrError.inspect());
-          *cellOrRowOrTableElementOrError.inspect())) {
-    NS_WARNING("There was no ancestor <table> element for the found cell");
-    return NS_ERROR_FAILURE;
-  }
-
-  EditorDOMPoint pointToInsert(cellOrRowOrTableElementOrError.inspect());
   if (!pointToInsert.IsSet()) {
     NS_WARNING("Found an orphan cell element");
     return NS_ERROR_FAILURE;
@@ -397,16 +390,11 @@ CreateElementResult HTMLEditor::InsertTableCellsWithTransaction(
 
   // Put caret into the cell before the first inserting cell, or the first
   // table cell in the row.
-  RefPtr<Element> cellToPutCaret;
+  RefPtr<Element> cellToPutCaret =
-  for (nsIContent* maybeCellToPutCaret =
+      aPointToInsert.IsEndOfContainer()
-           aPointToInsert.GetPreviousSiblingOfChild();
+          ? nullptr
-       maybeCellToPutCaret;
+          : HTMLEditUtils::GetPreviousTableCellElementSibling(
-       maybeCellToPutCaret = maybeCellToPutCaret->GetPreviousSibling()) {
+                *aPointToInsert.GetChild());
-    if (HTMLEditUtils::IsTableCell(maybeCellToPutCaret)) {
-      cellToPutCaret = maybeCellToPutCaret->AsElement();
-      break;
-    }
-  }
 
   RefPtr<Element> firstCellElement, lastCellElement;
   nsresult rv = [&]() MOZ_CAN_RUN_SCRIPT {
@@ -606,10 +594,30 @@ NS_IMETHODIMP HTMLEditor::InsertTableColumn(int32_t aNumberOfColumnsToInsert,
     return EditorBase::ToGenericNSResult(rv);
   }
 
-  rv = InsertTableColumnsWithTransaction(
+  Result<RefPtr<Element>, nsresult> cellElementOrError =
-      aNumberOfColumnsToInsert, aInsertAfterSelectedCell
+      GetFirstSelectedCellElementInTable();
-                                    ? InsertPosition::eAfterSelectedCell
+  if (cellElementOrError.isErr()) {
-                                    : InsertPosition::eBeforeSelectedCell);
+    NS_WARNING("HTMLEditor::GetFirstSelectedCellElementInTable() failed");
+    return EditorBase::ToGenericNSResult(cellElementOrError.unwrapErr());
+  }
+
+  if (!cellElementOrError.inspect()) {
+    return NS_OK;
+  }
+
+  EditorDOMPoint pointToInsert(cellElementOrError.inspect());
+  if (!pointToInsert.IsSet()) {
+    NS_WARNING("Found an orphan cell element");
+    return NS_ERROR_FAILURE;
+  }
+  if (aInsertAfterSelectedCell && !pointToInsert.IsEndOfContainer()) {
+    DebugOnly<bool> advanced = pointToInsert.AdvanceOffset();
+    NS_WARNING_ASSERTION(
+        advanced,
+        "Failed to set insertion point after current cell, but ignored");
+  }
+  rv = InsertTableColumnsWithTransaction(pointToInsert,
+                                         aNumberOfColumnsToInsert);
   NS_WARNING_ASSERTION(
       NS_SUCCEEDED(rv),
       "HTMLEditor::InsertTableColumnsWithTransaction() failed");
@@ -617,47 +625,98 @@ NS_IMETHODIMP HTMLEditor::InsertTableColumn(int32_t aNumberOfColumnsToInsert,
 }
 
 nsresult HTMLEditor::InsertTableColumnsWithTransaction(
-    int32_t aNumberOfColumnsToInsert, InsertPosition aInsertPosition) {
+    const EditorDOMPoint& aPointToInsert, int32_t aNumberOfColumnsToInsert) {
   MOZ_ASSERT(IsEditActionDataAvailable());
-  MOZ_ASSERT(aNumberOfColumnsToInsert >= 0);
+  MOZ_ASSERT(aPointToInsert.IsSetAndValid());
+  MOZ_ASSERT(aNumberOfColumnsToInsert > 0);
 
-  RefPtr<Element> table;
+  const RefPtr<PresShell> presShell = GetPresShell();
-  RefPtr<Element> curCell;
+  if (NS_WARN_IF(!presShell)) {
-  int32_t startRowIndex, startColIndex;
+    return NS_ERROR_FAILURE;
-  nsresult rv =
+  }
-      GetCellContext(getter_AddRefs(table), getter_AddRefs(curCell), nullptr,
+
-                     nullptr, &startRowIndex, &startColIndex);
+  if (!HTMLEditUtils::IsTableRow(aPointToInsert.GetContainer())) {
-  if (NS_FAILED(rv)) {
+    NS_WARNING("Tried to insert columns to non-<tr> element");
-    NS_WARNING("HTMLEditor::GetCellContext() failed");
+    return NS_ERROR_FAILURE;
-    return rv;
+  }
-  }
+
-  if (!table || !curCell) {
+  const RefPtr<Element> tableElement =
-    NS_WARNING(
+      HTMLEditUtils::GetClosestAncestorTableElement(
-        "HTMLEditor::GetCellContext() didn't return <table> and/or cell");
+          *aPointToInsert.ContainerAsElement());
-    // Don't fail if no cell found.
+  if (!tableElement) {
-    return NS_OK;
+    NS_WARNING("There was no ancestor <table> element");
-  }
-
-  // Get more data for current cell, we need rowspan value.
-  const auto cellDataAtSelection = CellData::AtIndexInTableElement(
-      *this, *table, startRowIndex, startColIndex);
-  if (NS_WARN_IF(cellDataAtSelection.FailedOrNotFound())) {
     return NS_ERROR_FAILURE;
   }
-  MOZ_ASSERT(curCell == cellDataAtSelection.mElement);
 
   const Result<TableSize, nsresult> tableSizeOrError =
-      TableSize::Create(*this, *table);
+      TableSize::Create(*this, *tableElement);
   if (NS_WARN_IF(tableSizeOrError.isErr())) {
     return tableSizeOrError.inspectErr();
   }
   const TableSize& tableSize = tableSizeOrError.inspect();
-  // Should not be empty since we've already found a cell.
+  if (NS_WARN_IF(tableSize.IsEmpty())) {
-  MOZ_ASSERT(!tableSize.IsEmpty());
+    return NS_ERROR_FAILURE;  // We cannot handle it in an empty table
+  }
+
+  // If aPointToInsert points non-cell element or end of the row, it means that
+  // the caller wants to insert column immediately after the last cell of
+  // the pointing cell element or in the raw.
+  const bool insertAfterPreviousCell = [&]() {
+    if (!aPointToInsert.IsEndOfContainer() &&
+        HTMLEditUtils::IsTableCell(aPointToInsert.GetChild())) {
+      return false;  // Insert before the cell element.
+    }
+    // There is a previous cell element, we should add a column after it.
+    Element* previousCellElement =
+        aPointToInsert.IsEndOfContainer()
+            ? HTMLEditUtils::GetLastTableCellElementChild(
+                  *aPointToInsert.ContainerAsElement())
+            : HTMLEditUtils::GetPreviousTableCellElementSibling(
+                  *aPointToInsert.GetChild());
+    return previousCellElement != nullptr;
+  }();
+
+  // Consider the column index in the table from given point and direction.
+  auto referenceColumnIndexOrError =
+      [&]() MOZ_CAN_RUN_SCRIPT -> Result<int32_t, nsresult> {
+    if (!insertAfterPreviousCell) {
+      if (aPointToInsert.IsEndOfContainer()) {
+        return tableSize.mColumnCount;  // Empty row, append columns to the end
+      }
+      // Insert columns immediately before current column.
+      const OwningNonNull<Element> tableCellElement =
+          *aPointToInsert.GetChild()->AsElement();
+      MOZ_ASSERT(HTMLEditUtils::IsTableCell(tableCellElement));
+      CellIndexes cellIndexes(*tableCellElement, presShell);
+      if (NS_WARN_IF(cellIndexes.isErr())) {
+        return Err(NS_ERROR_FAILURE);
+      }
+      return cellIndexes.mColumn;
+    }
+
+    // Otherwise, insert columns immediately after the previous column.
+    Element* previousCellElement =
+        aPointToInsert.IsEndOfContainer()
+            ? HTMLEditUtils::GetLastTableCellElementChild(
+                  *aPointToInsert.ContainerAsElement())
+            : HTMLEditUtils::GetPreviousTableCellElementSibling(
+                  *aPointToInsert.GetChild());
+    MOZ_ASSERT(previousCellElement);
+    CellIndexes cellIndexes(*previousCellElement, presShell);
+    if (NS_WARN_IF(cellIndexes.isErr())) {
+      return Err(NS_ERROR_FAILURE);
+    }
+    return cellIndexes.mColumn;
+  }();
+  if (MOZ_UNLIKELY(referenceColumnIndexOrError.isErr())) {
+    return referenceColumnIndexOrError.unwrapErr();
+  }
 
   AutoPlaceholderBatch treateAsOneTransaction(
       *this, ScrollSelectionIntoView::Yes, __FUNCTION__);
   // Prevent auto insertion of <br> element in new cell until we're done.
+  // XXX Why? We should put <br> element to every cell element before inserting
+  //     the cells into the tree.
   IgnoredErrorResult error;
   AutoEditSubActionNotifier startToHandleEditSubAction(
       *this, EditSubAction::eInsertNode, nsIEditor::eNext, error);
@@ -669,32 +728,6 @@ nsresult HTMLEditor::InsertTableColumnsWithTransaction(
       "HTMLEditor::OnStartToHandleTopLevelEditSubAction() failed, but ignored");
   error.SuppressException();
 
-  switch (aInsertPosition) {
-    case InsertPosition::eBeforeSelectedCell:
-      break;
-    case InsertPosition::eAfterSelectedCell:
-      // Use column after current cell.
-      startColIndex += cellDataAtSelection.mEffectiveColSpan;
-
-      // Detect when user is adding after a colspan=0 case.
-      // Assume they want to stop the "0" behavior and really add a new column.
-      // Thus we set the colspan to its true value.
-      if (!cellDataAtSelection.mColSpan) {
-        DebugOnly<nsresult> rvIgnored =
-            SetColSpan(cellDataAtSelection.mElement,
-                       cellDataAtSelection.mEffectiveColSpan);
-        NS_WARNING_ASSERTION(NS_SUCCEEDED(rvIgnored),
-                             "HTMLEditor::SetColSpan() failed, but ignored");
-      }
-      break;
-    default:
-      MOZ_ASSERT_UNREACHABLE("Invalid InsertPosition");
-  }
-
-  // We control selection resetting after the insert.
-  AutoSelectionSetterAfterTableEdit setCaret(
-      *this, table, cellDataAtSelection.mCurrent.mRow, startColIndex,
-      ePreviousRow, false);
   // Suppress Rules System selection munging.
   AutoTransactionsConserveSelection dontChangeSelection(*this);
 
@@ -703,36 +736,55 @@ nsresult HTMLEditor::InsertTableColumnsWithTransaction(
   // XXX As far as I've tested, NormalizeTableInternal() always fails to
   //     normalize non-rectangular table.  So, the following CellData will
   //     fail if the table is not rectangle.
-  if (startColIndex >= tableSize.mColumnCount) {
+  if (referenceColumnIndexOrError.inspect() >= tableSize.mColumnCount) {
-    DebugOnly<nsresult> rv = NormalizeTableInternal(*table);
+    DebugOnly<nsresult> rv = NormalizeTableInternal(*tableElement);
+    if (MOZ_UNLIKELY(Destroyed())) {
+      NS_WARNING(
+          "HTMLEditor::NormalizeTableInternal() caused destroying the editor");
+      return NS_ERROR_EDITOR_DESTROYED;
+    }
     NS_WARNING_ASSERTION(
         NS_SUCCEEDED(rv),
         "HTMLEditor::NormalizeTableInternal() failed, but ignored");
   }
 
-  RefPtr<Element> rowElement;
+  // First, we should collect all reference nodes to insert new table cells.
-  for (int32_t rowIndex = 0; rowIndex < tableSize.mRowCount; rowIndex++) {
+  AutoTArray<CellData, 32> arrayOfCellData;
-    if (startColIndex < tableSize.mColumnCount) {
+  {
-      // We are inserting before an existing column.
+    arrayOfCellData.SetCapacity(tableSize.mRowCount);
+    for (const int32_t rowIndex : IntegerRange(tableSize.mRowCount)) {
       const auto cellData = CellData::AtIndexInTableElement(
-          *this, *table, rowIndex, startColIndex);
+          *this, *tableElement, rowIndex,
+          referenceColumnIndexOrError.inspect());
       if (NS_WARN_IF(cellData.FailedOrNotFound())) {
         return NS_ERROR_FAILURE;
       }
+      arrayOfCellData.AppendElement(cellData);
+    }
+  }
 
+  // Note that checking whether the editor destroyed or not should be done
+  // after inserting all cell elements.  Otherwise, the table is left as
+  // not a rectangle.
+  auto cellElementToPutCaretOrError =
+      [&]() MOZ_CAN_RUN_SCRIPT -> Result<RefPtr<Element>, nsresult> {
+    // Block legacy mutation events for making this job simpler.
+    nsAutoScriptBlockerSuppressNodeRemoved blockToRunScript;
+    RefPtr<Element> cellElementToPutCaret;
+    for (const CellData& cellData : arrayOfCellData) {
       // Don't fail entire process if we fail to find a cell (may fail just in
       // particular rows with < adequate cells per row).
-      // XXX So, here wants to know whether the CellData actually failed above.
+      // XXX So, here wants to know whether the CellData actually failed
-      //     Fix this later.
+      //     above.  Fix this later.
       if (!cellData.mElement) {
         continue;
       }
 
-      if (cellData.IsSpannedFromOtherColumn()) {
+      if ((!insertAfterPreviousCell && cellData.IsSpannedFromOtherColumn()) ||
+          (insertAfterPreviousCell &&
+           cellData.IsNextColumnSpannedFromOtherColumn())) {
         // If we have a cell spanning this location, simply increase its
         // colspan to keep table rectangular.
-        // Note: we do nothing if colsspan=0, since it should automatically
-        // span the new column.
         if (cellData.mColSpan > 0) {
           DebugOnly<nsresult> rvIgnored = SetColSpan(
               cellData.mElement, cellData.mColSpan + aNumberOfColumnsToInsert);
@@ -742,70 +794,56 @@ nsresult HTMLEditor::InsertTableColumnsWithTransaction(
         continue;
       }
 
-      EditorDOMPoint pointToInsert(cellData.mElement);
+      EditorDOMPoint pointToInsert = [&]() {
-      if (MOZ_UNLIKELY(NS_WARN_IF(!pointToInsert.IsSet()))) {
+        if (!insertAfterPreviousCell) {
-        return NS_ERROR_EDITOR_UNEXPECTED_DOM_TREE;
+          // Insert before the reference cell.
+          return EditorDOMPoint(cellData.mElement);
         }
-      CreateElementResult insertCellElementResult =
+        if (!cellData.mElement->GetNextSibling()) {
+          // Insert after the reference cell, but nothing follows it, append
+          // to the end of the row.
+          return EditorDOMPoint::AtEndOf(*cellData.mElement->GetParentNode());
+        }
+        // Otherwise, returns immediately before the next sibling.  Note that
+        // the next sibling may not be a table cell element.  E.g., it may be
+        // a text node containing only white-spaces in most cases.
+        return EditorDOMPoint(cellData.mElement->GetNextSibling());
+      }();
+      if (NS_WARN_IF(!pointToInsert.IsInContentNode())) {
+        return Err(NS_ERROR_EDITOR_UNEXPECTED_DOM_TREE);
+      }
+      CreateElementResult insertCellElementsResult =
           InsertTableCellsWithTransaction(pointToInsert,
                                           aNumberOfColumnsToInsert);
-      if (insertCellElementResult.isErr()) {
+      if (insertCellElementsResult.isErr()) {
         NS_WARNING("HTMLEditor::InsertTableCellsWithTransaction() failed");
-        return insertCellElementResult.inspectErr();
+        return Err(insertCellElementsResult.unwrapErr());
       }
-      // We don't need to update selection here.
+      // We'll update selection later into the first inserted cell element in
-      insertCellElementResult.IgnoreCaretPointSuggestion();
+      // the current row.
-      continue;
+      insertCellElementsResult.IgnoreCaretPointSuggestion();
+      if (pointToInsert.ContainerAsElement() ==
+          aPointToInsert.ContainerAsElement()) {
+        cellElementToPutCaret = insertCellElementsResult.UnwrapNewNode();
+        MOZ_ASSERT(cellElementToPutCaret);
+        MOZ_ASSERT(HTMLEditUtils::IsTableCell(cellElementToPutCaret));
       }
-
-    // Get current row and append new cells after last cell in row
-    if (!rowIndex) {
-      Result<RefPtr<Element>, nsresult> rowElementOrError =
-          GetFirstTableRowElement(*table);
-      if (rowElementOrError.isErr()) {
-        NS_WARNING("HTMLEditor::GetFirstTableRowElement() failed");
-        return rowElementOrError.unwrapErr();
     }
-      if (!rowElementOrError.inspect()) {
+    return cellElementToPutCaret;
-        NS_WARNING("There was no table row");
+  }();
-        continue;
+  if (MOZ_UNLIKELY(cellElementToPutCaretOrError.isErr())) {
+    return NS_WARN_IF(Destroyed()) ? NS_ERROR_EDITOR_DESTROYED
+                                   : cellElementToPutCaretOrError.unwrapErr();
   }
-      rowElement = rowElementOrError.unwrap();
+  const RefPtr<Element> cellElementToPutCaret =
-    } else {
+      cellElementToPutCaretOrError.unwrap();
-      if (!rowElement) {
+  NS_WARNING_ASSERTION(
-        NS_WARNING("Have not found table row yet");
+      cellElementToPutCaret,
-        // XXX Looks like that when rowIndex is 0, startColIndex is always
+      "Didn't find the first inserted cell element in the specified row");
-        //     same as or larger than tableSize.mColumnCount.  Is it true?
+  if (MOZ_LIKELY(cellElementToPutCaret)) {
-        return NS_ERROR_FAILURE;
+    CollapseSelectionToDeepestNonTableFirstChild(cellElementToPutCaret);
   }
-      Result<RefPtr<Element>, nsresult> rowElementOrError =
+  return NS_WARN_IF(Destroyed()) ? NS_ERROR_EDITOR_DESTROYED : NS_OK;
-          GetNextTableRowElement(*rowElement);
-      if (rowElementOrError.isErr()) {
-        NS_WARNING("HTMLEditor::GetNextTableRowElement() failed");
-        return rowElementOrError.unwrapErr();
-      }
-      if (!rowElementOrError.inspect()) {
-        NS_WARNING(
-            "HTMLEditor::GetNextTableRowElement() didn't return <tr> element");
-        continue;
-      }
-      rowElement = rowElementOrError.unwrap();
-    }
-
-    EditorDOMPoint atEndOfRow = EditorDOMPoint::AtEndOf(*rowElement);
-    if (MOZ_UNLIKELY(NS_WARN_IF(!atEndOfRow.IsSet()))) {
-      return NS_ERROR_EDITOR_UNEXPECTED_DOM_TREE;
-    }
-    const CreateElementResult insertCellElementResult =
-        InsertTableCellsWithTransaction(atEndOfRow, aNumberOfColumnsToInsert);
-    if (insertCellElementResult.isErr()) {
-      NS_WARNING("HTMLEditor::InsertTableCellsWithTransaction() failed");
-      return insertCellElementResult.inspectErr();
-    }
-    // We don't need to update selection here.
-    insertCellElementResult.IgnoreCaretPointSuggestion();
-  }
-  return NS_OK;
 }
 
 NS_IMETHODIMP HTMLEditor::InsertTableRow(int32_t aNumberOfRowsToInsert,
@@ -4214,6 +4252,36 @@ Result<RefPtr<Element>, nsresult> HTMLEditor::GetSelectedOrParentTableElement(
   return cellElement;
 }
 
+Result<RefPtr<Element>, nsresult>
+HTMLEditor::GetFirstSelectedCellElementInTable() const {
+  Result<RefPtr<Element>, nsresult> cellOrRowOrTableElementOrError =
+      GetSelectedOrParentTableElement();
+  if (cellOrRowOrTableElementOrError.isErr()) {
+    NS_WARNING("HTMLEditor::GetSelectedOrParentTableElement() failed");
+    return cellOrRowOrTableElementOrError;
+  }
+
+  if (!cellOrRowOrTableElementOrError.inspect()) {
+    return cellOrRowOrTableElementOrError;
+  }
+
+  const RefPtr<Element>& element = cellOrRowOrTableElementOrError.inspect();
+  if (!HTMLEditUtils::IsTableCell(element)) {
+    return RefPtr<Element>();
+  }
+
+  if (!HTMLEditUtils::IsTableRow(element->GetParentNode())) {
+    NS_WARNING("There was no parent <tr> element for the found cell");
+    return RefPtr<Element>();
+  }
+
+  if (!HTMLEditUtils::GetClosestAncestorTableElement(*element)) {
+    NS_WARNING("There was no ancestor <table> element for the found cell");
+    return Err(NS_ERROR_FAILURE);
+  }
+  return cellOrRowOrTableElementOrError;
+}
+
 NS_IMETHODIMP HTMLEditor::GetSelectedCellsType(Element* aElement,
                                                uint32_t* aSelectionType) {
   if (NS_WARN_IF(!aSelectionType)) {

editor/libeditor/tests/test_nsITableEditor_insertTableColumn.html

@@ -122,6 +122,96 @@ SimpleTest.waitForFocus(() => {
        'No "input" event should be fired when nsITableEditor.insertTableColumn(1, true) causes exception due to no selection range');
   }
 
+  (function testInsertBeforeFirstColumn() {
+    selection.removeAllRanges();
+    editor.innerHTML =
+      "<table>" +
+        '<tr><td id="select">cell1-1</td><td>cell1-2</td><td>cell1-3</td></tr>' +
+        "<tr><td>cell2-1</td><td>cell2-2</td><td>cell2-3</td></tr>" +
+      "</table>";
+    editor.focus();
+    beforeInputEvents = [];
+    inputEvents = [];
+    selection.setBaseAndExtent(
+      document.getElementById("select").firstChild,
+      0,
+      document.getElementById("select").firstChild,
+      0
+    );
+    getTableEditor().insertTableColumn(1, false);
+    is(
+      editor.innerHTML,
+      "<table><tbody>" +
+        '<tr><td valign="top"><br></td><td id="select">cell1-1</td><td>cell1-2</td><td>cell1-3</td></tr>' +
+        '<tr><td valign="top"><br></td><td>cell2-1</td><td>cell2-2</td><td>cell2-3</td></tr>' +
+      "</tbody></table>",
+      "testInsertBeforeFirstColumn: nsITableEditor.insertTableColumn(1, false) should insert a column before the first column"
+    );
+    is(
+      beforeInputEvents.length,
+      1,
+      'testInsertBeforeFirstColumn: Only one "beforeinput" event should be fired'
+    );
+    checkInputEvent(
+      beforeInputEvents[0],
+      "when selection is collapsed in the first column (testInsertBeforeFirstColumn)"
+    );
+    is(
+      inputEvents.length,
+      1,
+      'testInsertBeforeFirstColumn: Only one "input" event should be fired'
+    );
+    checkInputEvent(
+      inputEvents[0],
+      "when selection is collapsed in the first column (testInsertBeforeFirstColumn)"
+    );
+  })();
+
+  (function testInsertAfterLastColumn() {
+    selection.removeAllRanges();
+    editor.innerHTML =
+      "<table>" +
+        '<tr><td>cell1-1</td><td>cell1-2</td><td id="select">cell1-3</td></tr>' +
+        "<tr><td>cell2-1</td><td>cell2-2</td><td>cell2-3</td></tr>" +
+      "</table>";
+    editor.focus();
+    beforeInputEvents = [];
+    inputEvents = [];
+    selection.setBaseAndExtent(
+      document.getElementById("select").firstChild,
+      0,
+      document.getElementById("select").firstChild,
+      0
+    );
+    getTableEditor().insertTableColumn(1, true);
+    is(
+      editor.innerHTML,
+      "<table><tbody>" +
+        '<tr><td>cell1-1</td><td>cell1-2</td><td id="select">cell1-3</td><td valign="top"><br></td></tr>' +
+        '<tr><td>cell2-1</td><td>cell2-2</td><td>cell2-3</td><td valign="top"><br></td></tr>' +
+      "</tbody></table>",
+      "testInsertAfterLastColumn: nsITableEditor.insertTableColumn(1, true) should insert a column after the last column"
+    );
+    is(
+      beforeInputEvents.length,
+      1,
+      'testInsertAfterLastColumn: Only one "beforeinput" event should be fired'
+    );
+    checkInputEvent(
+      beforeInputEvents[0],
+      "when selection is collapsed in the last column (testInsertAfterLastColumn)"
+    );
+    is(
+      inputEvents.length,
+      1,
+      'testInsertAfterLastColumn: One "input" event should be fired'
+    );
+    checkInputEvent(
+      inputEvents[0],
+      "when selection is collapsed in the last column (testInsertAfterLastColumn)"
+    );
+  })();
+
   selection.removeAllRanges();
   editor.innerHTML = "<table>" +
                        '<tr><td>cell1-1</td><td id="select">cell1-2</td><td>cell1-3</td></tr>' +
@@ -260,6 +350,186 @@ SimpleTest.waitForFocus(() => {
      'Only one "input" event should be fired when selection is collapsed in a cell which is col-spanning (after)');
   checkInputEvent(inputEvents[0], "when selection is collapsed in a cell which is col-spanning (after)");
 
+  (function testInsertBeforeFirstColumnFollowingTextNode() {
+    selection.removeAllRanges();
+    editor.innerHTML =
+      "<table>" +
+        '<tr> <td id="select">cell1-1</td><td>cell1-2</td><td>cell1-3</td> </tr>' +
+        "<tr> <td>cell2-1</td><td>cell2-2</td><td>cell2-3</td> </tr>" +
+      "</table>";
+    editor.focus();
+    beforeInputEvents = [];
+    inputEvents = [];
+    selection.setBaseAndExtent(
+      document.getElementById("select").firstChild,
+      0,
+      document.getElementById("select").firstChild,
+      0
+    );
+    getTableEditor().insertTableColumn(1, false);
+    is(
+      editor.innerHTML,
+      "<table><tbody>" +
+        '<tr> <td valign="top"><br></td><td id="select">cell1-1</td><td>cell1-2</td><td>cell1-3</td> </tr>' +
+        '<tr> <td valign="top"><br></td><td>cell2-1</td><td>cell2-2</td><td>cell2-3</td> </tr>' +
+      "</tbody></table>",
+      "testInsertBeforeFirstColumnFollowingTextNode: nsITableEditor.insertTableColumn(1, false) should insert a column before the first column"
+    );
+    is(
+      beforeInputEvents.length,
+      1,
+      'testInsertBeforeFirstColumnFollowingTextNode: Only one "beforeinput" event should be fired'
+    );
+    checkInputEvent(
+      beforeInputEvents[0],
+      "when selection is collapsed in the first column (testInsertBeforeFirstColumnFollowingTextNode)"
+    );
+    is(
+      inputEvents.length,
+      1,
+      'testInsertBeforeFirstColumnFollowingTextNode: Only one "input" event should be fired'
+    );
+    checkInputEvent(
+      inputEvents[0],
+      "when selection is collapsed in the first column (testInsertBeforeFirstColumnFollowingTextNode)"
+    );
+  })();
+
+  (function testInsertAfterLastColumnFollowedByTextNode() {
+    selection.removeAllRanges();
+    editor.innerHTML =
+      "<table>" +
+        '<tr> <td>cell1-1</td><td>cell1-2</td><td id="select">cell1-3</td> </tr>' +
+        "<tr> <td>cell2-1</td><td>cell2-2</td><td>cell2-3</td> </tr>" +
+      "</table>";
+    editor.focus();
+    beforeInputEvents = [];
+    inputEvents = [];
+    selection.setBaseAndExtent(
+      document.getElementById("select").firstChild,
+      0,
+      document.getElementById("select").firstChild,
+      0
+    );
+    getTableEditor().insertTableColumn(1, true);
+    is(
+      editor.innerHTML,
+      "<table><tbody>" +
+        '<tr> <td>cell1-1</td><td>cell1-2</td><td id="select">cell1-3</td><td valign="top"><br></td> </tr>' +
+        '<tr> <td>cell2-1</td><td>cell2-2</td><td>cell2-3</td><td valign="top"><br></td> </tr>' +
+      "</tbody></table>",
+      "testInsertAfterLastColumnFollowedByTextNode: nsITableEditor.insertTableColumn(1, true) should insert a column after the last column"
+    );
+    is(
+      beforeInputEvents.length,
+      1,
+      'testInsertAfterLastColumnFollowedByTextNode: Only one "beforeinput" event should be fired'
+    );
+    checkInputEvent(
+      beforeInputEvents[0],
+      "when selection is collapsed in the last column (testInsertAfterLastColumnFollowedByTextNode)"
+    );
+    is(
+      inputEvents.length,
+      1,
+      'testInsertAfterLastColumnFollowedByTextNode: One "input" event should be fired'
+    );
+    checkInputEvent(
+      inputEvents[0],
+      "when selection is collapsed in the last column (testInsertAfterLastColumnFollowedByTextNode)"
+    );
+  })();
+
+  (function testInsertBeforeColumnFollowingTextNode() {
+    selection.removeAllRanges();
+    editor.innerHTML =
+      "<table>" +
+        '<tr><td>cell1-1</td> <td id="select">cell1-2</td> <td>cell1-3</td></tr>' +
+        "<tr><td>cell2-1</td> <td>cell2-2</td> <td>cell2-3</td></tr>" +
+      "</table>";
+    editor.focus();
+    beforeInputEvents = [];
+    inputEvents = [];
+    selection.setBaseAndExtent(
+      document.getElementById("select").firstChild,
+      0,
+      document.getElementById("select").firstChild,
+      0
+    );
+    getTableEditor().insertTableColumn(1, false);
+    is(
+      editor.innerHTML,
+      "<table><tbody>" +
+        '<tr><td>cell1-1</td> <td valign="top"><br></td><td id="select">cell1-2</td> <td>cell1-3</td></tr>' +
+        '<tr><td>cell2-1</td> <td valign="top"><br></td><td>cell2-2</td> <td>cell2-3</td></tr>' +
+      "</tbody></table>",
+      "testInsertBeforeColumnFollowingTextNode: nsITableEditor.insertTableColumn(1, false) should insert a column before the first column"
+    );
+    is(
+      beforeInputEvents.length,
+      1,
+      'testInsertBeforeColumnFollowingTextNode: Only one "beforeinput" event should be fired'
+    );
+    checkInputEvent(
+      beforeInputEvents[0],
+      "when selection is collapsed in the column following a text node (testInsertBeforeColumnFollowingTextNode)"
+    );
+    is(
+      inputEvents.length,
+      1,
+      'testInsertBeforeColumnFollowingTextNode: Only one "input" event should be fired'
+    );
+    checkInputEvent(
+      inputEvents[0],
+      "when selection is collapsed in the column following a text node (testInsertBeforeColumnFollowingTextNode)"
+    );
+  })();
+
+  (function testInsertAfterColumnFollowedByTextNode() {
+    selection.removeAllRanges();
+    editor.innerHTML =
+      "<table>" +
+        '<tr><td>cell1-1</td> <td id="select">cell1-2</td> <td>cell1-3</td></tr>' +
+        "<tr><td>cell2-1</td> <td>cell2-2</td> <td>cell2-3</td></tr>" +
+      "</table>";
+    editor.focus();
+    beforeInputEvents = [];
+    inputEvents = [];
+    selection.setBaseAndExtent(
+      document.getElementById("select").firstChild,
+      0,
+      document.getElementById("select").firstChild,
+      0
+    );
+    getTableEditor().insertTableColumn(1, true);
+    is(
+      editor.innerHTML,
+      "<table><tbody>" +
+        '<tr><td>cell1-1</td> <td id="select">cell1-2</td><td valign="top"><br></td> <td>cell1-3</td></tr>' +
+        '<tr><td>cell2-1</td> <td>cell2-2</td><td valign="top"><br></td> <td>cell2-3</td></tr>' +
+      "</tbody></table>",
+      "testInsertAfterColumnFollowedByTextNode: nsITableEditor.insertTableColumn(1, true) should insert a column before the first column"
+    );
+    is(
+      beforeInputEvents.length,
+      1,
+      'testInsertAfterColumnFollowedByTextNode: Only one "beforeinput" event should be fired'
+    );
+    checkInputEvent(
+      beforeInputEvents[0],
+      "when selection is collapsed in the column followed by a text node (testInsertAfterColumnFollowedByTextNode)"
+    );
+    is(
+      inputEvents.length,
+      1,
+      'testInsertAfterColumnFollowedByTextNode: Only one "input" event should be fired'
+    );
+    checkInputEvent(
+      inputEvents[0],
+      "when selection is collapsed in the column followed by a text node (testInsertAfterColumnFollowedByTextNode)"
+    );
+  })();
+
   editor.removeEventListener("beforeinput", onBeforeInput);
   editor.removeEventListener("input", onInput);
 

editor/reftests/inline-table-editor-position-after-updating-table-size-from-input-event-listener-ref.html

@@ -5,9 +5,9 @@
 <title>Inline table editor should be positioned correctly even if modified the table from an input event listener</title>
 <script>
 addEventListener("load", async () => {
-  const cell = document.querySelector("td");
+  const cell = document.querySelector("td + td");
   document.body.focus();
-  getSelection().collapse(cell.firstChild, 0);
+  getSelection().collapse(cell, 0);
   document.execCommand("enableObjectResizing", false, "true");
   document.execCommand("enableInlineTableEditing", false, "true");
   requestAnimationFrame(