Skip to content

Commit

Permalink
Bug 1484121 - part 1: Create HTMLEditor::DeleteSelectedTableColumnsWi…
Browse files Browse the repository at this point in the history 
…thTransaction() for internal use of nsITableEditor::DeleteTableColumn() r=m_kato

nsITableEditor::DeleteTableColumn() is an XPCOM method but used internally.
So, it should be implemented with non-virtual method and internal users
should use it.

Note that this changes only one thing.  This moves
|AutoTopLevelEditSubActionNotifier maybeTopLevelEditSubAction(...| from
below DeleteTableElementAndChildrenWithTransaction() to above it.
I.e., DeleteTableElementAndChildrenWithTransaction() works under
EditSubAction::eDeleteNode as the top level sub-action now.  This is same
as DeleteSelectedTableRowsWithTransaction().  Therefore, the difference
with it when it removes <table> is now fixed.

Differential Revision: https://phabricator.services.mozilla.com/D5933
  • Loading branch information
@masayuki-nakano
masayuki-nakano committed Sep 19, 2018
1 parent edbee41 commit 8c3f8a1
Show file tree
Hide file tree
Showing 5 changed files with 118 additions and 69 deletions.
20 changes: 20 additions & 0 deletions editor/libeditor/HTMLEditor.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1553,6 +1553,26 @@ class HTMLEditor final : public TextEditor
int32_t aColSpan, bool aAfter, bool aIsHeader,
Element** aNewCell);

/**
* DeleteSelectedTableColumnsWithTransaction() removes cell elements which
* belong to same columns of selected cell elements.
* If only one cell element is selected or first selection range is
* in a cell, removes cell elements which belong to same column.
* If 2 or more cell elements are selected, removes cell elements which
* belong to any of all selected columns. In this case,
* aNumberOfColumnsToDelete is ignored.
* If there is no selection ranges, returns error.
* If selection is not in a cell element, this does not return error,
* just does nothing.
* WARNING: This does not remove <col> nor <colgroup> elements.
*
* @param aNumberOfColumnsToDelete Number of columns to remove. This is
* ignored if 2 ore more cells are
* selected.
*/
nsresult
DeleteSelectedTableColumnsWithTransaction(int32_t aNumberOfColumnsToDelete);

/**
* DeleteSelectedTableRowsWithTransaction() removes <tr> elements.
* If only one cell element is selected or first selection range is
Expand Down
4 changes: 3 additions & 1 deletion editor/libeditor/HTMLInlineTableEditor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,9 @@ HTMLEditor::DoInlineTableEditingAction(const Element& aElement)
} else if (anonclass.EqualsLiteral("mozTableAddRowAfter")) {
InsertTableRow(1, true);
} else if (anonclass.EqualsLiteral("mozTableRemoveColumn")) {
DeleteTableColumn(1);
DebugOnly<nsresult> rv = DeleteSelectedTableColumnsWithTransaction(1);
NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
"Failed to delete the selected table column");
hideUI = (colCount == 1);
} else if (anonclass.EqualsLiteral("mozTableRemoveRow")) {
DebugOnly<nsresult> rv = DeleteSelectedTableRowsWithTransaction(1);
Expand Down
119 changes: 68 additions & 51 deletions editor/libeditor/HTMLTableEditor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1082,7 +1082,19 @@ HTMLEditor::DeleteCellContents(Element* aCell)
}

NS_IMETHODIMP
HTMLEditor::DeleteTableColumn(int32_t aNumber)
HTMLEditor::DeleteTableColumn(int32_t aNumberOfColumnsToDelete)
{
nsresult rv =
DeleteSelectedTableColumnsWithTransaction(aNumberOfColumnsToDelete);
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
return NS_OK;
}

nsresult
HTMLEditor::DeleteSelectedTableColumnsWithTransaction(
int32_t aNumberOfColumnsToDelete)
{
RefPtr<Selection> selection;
RefPtr<Element> table;
Expand All @@ -1096,9 +1108,9 @@ HTMLEditor::DeleteTableColumn(int32_t aNumber)
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
if (NS_WARN_IF(!table) || NS_WARN_IF(!cell)) {
if (NS_WARN_IF(!selection) || NS_WARN_IF(!table) || NS_WARN_IF(!cell)) {
// Don't fail if no cell found.
return NS_SUCCESS_EDITOR_ELEMENT_NOT_FOUND;
return NS_OK;
}

ErrorResult error;
Expand All @@ -1109,25 +1121,20 @@ HTMLEditor::DeleteTableColumn(int32_t aNumber)

AutoPlaceholderBatch beginBatching(this);

// Prevent rules testing until we're done
AutoTopLevelEditSubActionNotifier maybeTopLevelEditSubAction(
*this, EditSubAction::eDeleteNode,
nsIEditor::eNext);

// Shortcut the case of deleting all columns in table
if (!startColIndex && aNumber >= tableSize.mColumnCount) {
if (NS_WARN_IF(!selection)) {
return NS_ERROR_FAILURE;
}
if (!startColIndex && aNumberOfColumnsToDelete >= tableSize.mColumnCount) {
rv = DeleteTableElementAndChildrenWithTransaction(*selection, *table);
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
return NS_OK;
}

// Check for counts too high
aNumber = std::min(aNumber, (tableSize.mColumnCount - startColIndex));

// Prevent rules testing until we're done
AutoTopLevelEditSubActionNotifier maybeTopLevelEditSubAction(
*this, EditSubAction::eDeleteNode,
nsIEditor::eNext);

// Test if deletion is controlled by selected cells
RefPtr<Element> firstSelectedCellElement =
Expand All @@ -1136,59 +1143,69 @@ HTMLEditor::DeleteTableColumn(int32_t aNumber)
return error.StealNSResult();
}

uint32_t rangeCount = selection->RangeCount();
MOZ_ASSERT(selection->RangeCount());

if (firstSelectedCellElement && rangeCount > 1) {
if (firstSelectedCellElement && selection->RangeCount() > 1) {
CellIndexes firstCellIndexes(*firstSelectedCellElement, error);
if (NS_WARN_IF(error.Failed())) {
return error.StealNSResult();
}
startRowIndex = firstCellIndexes.mRow;
startColIndex = firstCellIndexes.mColumn;
}
//We control selection resetting after the insert...

// We control selection resetting after the insert...
AutoSelectionSetterAfterTableEdit setCaret(*this, table, startRowIndex,
startColIndex, ePreviousRow,
false);

if (firstSelectedCellElement && rangeCount > 1) {
// Use selected cells to determine what rows to delete
cell = firstSelectedCellElement;
// If 2 or more cells are not selected, removing columns starting from
// a column which contains first selection range.
if (!firstSelectedCellElement || selection->RangeCount() == 1) {
int32_t columnCountToRemove =
std::min(aNumberOfColumnsToDelete,
tableSize.mColumnCount - startColIndex);
for (int32_t i = 0; i < columnCountToRemove; i++) {
rv = DeleteColumn(table, startColIndex);
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
}
return NS_OK;
}

while (cell) {
if (cell != firstSelectedCellElement) {
CellIndexes cellIndexes(*cell, error);
if (NS_WARN_IF(error.Failed())) {
return error.StealNSResult();
}
startRowIndex = cellIndexes.mRow;
startColIndex = cellIndexes.mColumn;
// If 2 or more cells are selected, remove all columns which contain selected
// cells. I.e., we ignore aNumberOfColumnsToDelete in this case.
for (cell = firstSelectedCellElement; cell;) {
if (cell != firstSelectedCellElement) {
CellIndexes cellIndexes(*cell, error);
if (NS_WARN_IF(error.Failed())) {
return error.StealNSResult();
}
// Find the next cell in a different column
// to continue after we delete this column
int32_t nextCol = startColIndex;
while (nextCol == startColIndex) {
cell = GetNextSelectedTableCellElement(*selection, error);
if (NS_WARN_IF(error.Failed())) {
return error.StealNSResult();
}
if (!cell) {
break;
}
CellIndexes cellIndexes(*cell, error);
if (NS_WARN_IF(error.Failed())) {
return error.StealNSResult();
}
startRowIndex = cellIndexes.mRow;
nextCol = cellIndexes.mColumn;
startRowIndex = cellIndexes.mRow;
startColIndex = cellIndexes.mColumn;
}
// Find the next cell in a different column
// to continue after we delete this column
int32_t nextCol = startColIndex;
while (nextCol == startColIndex) {
cell = GetNextSelectedTableCellElement(*selection, error);
if (NS_WARN_IF(error.Failed())) {
return error.StealNSResult();
}
rv = DeleteColumn(table, startColIndex);
NS_ENSURE_SUCCESS(rv, rv);
if (!cell) {
break;
}
CellIndexes cellIndexes(*cell, error);
if (NS_WARN_IF(error.Failed())) {
return error.StealNSResult();
}
startRowIndex = cellIndexes.mRow;
nextCol = cellIndexes.mColumn;
}
} else {
for (int32_t i = 0; i < aNumber; i++) {
rv = DeleteColumn(table, startColIndex);
NS_ENSURE_SUCCESS(rv, rv);
rv = DeleteColumn(table, startColIndex);
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
}
return NS_OK;
Expand Down
24 changes: 8 additions & 16 deletions editor/libeditor/tests/test_nsITableEditor_deleteTableColumn.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,7 @@
range.selectNode(document.getElementById("select"));
selection.addRange(range);
getTableEditor().deleteTableColumn(2);
// XXX I don't know why <br> element is inserted different from deleteTableRow().
is(editor.innerHTML, "<br>",
is(editor.innerHTML, "",
"nsITableEditor.deleteTableColumn(2) should delete the <table> since there is only 2 columns");

selection.removeAllRanges();
Expand All @@ -74,8 +73,7 @@
range.selectNode(document.getElementById("select"));
selection.addRange(range);
getTableEditor().deleteTableColumn(3);
// XXX I don't know why <br> element is inserted different from deleteTableRow().
is(editor.innerHTML, "<br>",
is(editor.innerHTML, "",
"nsITableEditor.deleteTableColumn(3) should delete the <table> when argument is larger than actual number of columns");

selection.removeAllRanges();
Expand All @@ -95,8 +93,7 @@
range.selectNode(document.getElementById("select"));
selection.addRange(range);
getTableEditor().deleteTableColumn(3);
// XXX I don't know why <br> element is inserted different from deleteTableRow().
is(editor.innerHTML, "<br>",
is(editor.innerHTML, "",
"nsITableEditor.deleteTableColumn(3) should delete the <table> since the argument equals actual number of columns");

// Similar to selected a cell, when selection is in a cell, the cell should
Expand Down Expand Up @@ -131,8 +128,7 @@
range.selectNode(document.getElementById("select").firstChild);
selection.addRange(range);
getTableEditor().deleteTableColumn(2);
// XXX I don't know why <br> element is inserted different from deleteTableRow().
is(editor.innerHTML, "<br>",
is(editor.innerHTML, "",
"nsITableEditor.deleteTableColumn(2) should delete the <table> since there is only 2 columns");

selection.removeAllRanges();
Expand All @@ -143,8 +139,7 @@
range.selectNode(document.getElementById("select").firstChild);
selection.addRange(range);
getTableEditor().deleteTableColumn(3);
// XXX I don't know why <br> element is inserted different from deleteTableRow().
is(editor.innerHTML, "<br>",
is(editor.innerHTML, "",
"nsITableEditor.deleteTableColumn(3) should delete the <table> when argument is larger than actual number of columns");

selection.removeAllRanges();
Expand All @@ -166,8 +161,7 @@
range.selectNode(document.getElementById("select").firstChild);
selection.addRange(range);
getTableEditor().deleteTableColumn(3);
// XXX I don't know why <br> element is inserted different from deleteTableRow().
is(editor.innerHTML, "<br>",
is(editor.innerHTML, "",
"nsITableEditor.deleteTableColumn(3) should delete the <table> since the argument equals actual number of columns");

// The argument should be ignored when 2 or more cells are selected.
Expand Down Expand Up @@ -195,8 +189,7 @@
range.selectNode(document.getElementById("select2"));
selection.addRange(range);
getTableEditor().deleteTableColumn(2);
// XXX I don't know why <br> element is inserted different from deleteTableRow().
is(editor.innerHTML, "<br>",
is(editor.innerHTML, "",
"nsITableEditor.deleteTableColumn(2) should delete the <table> since 2 is number of columns of the <table>");

selection.removeAllRanges();
Expand All @@ -209,8 +202,7 @@
range.selectNode(document.getElementById("select2"));
selection.addRange(range);
getTableEditor().deleteTableColumn(2);
// XXX I don't know why <br> element is inserted different from deleteTableRow().
is(editor.innerHTML, "<br>",
is(editor.innerHTML, "",
"nsITableEditor.deleteTableColumn(2) should delete the <table> since 2 is number of columns of the <table>");

selection.removeAllRanges();
Expand Down
20 changes: 19 additions & 1 deletion editor/nsITableEditor.idl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,25 @@ interface nsITableEditor : nsISupports
* have to be contiguous.
*/
void deleteTableCell(in long aNumber);
void deleteTableColumn(in long aNumber);

/**
* deleteTableColumn() removes cell elements which belong to same columns
* of selected cell elements.
* If only one cell element is selected or first selection range is
* in a cell, removes cell elements which belong to same column.
* If 2 or more cell elements are selected, removes cell elements which
* belong to any of all selected columns. In this case,
* aNumberOfColumnsToDelete is ignored.
* If there is no selection ranges, throws exception.
* If selection is not in a cell element, just does nothing without
* throwing exception.
* WARNING: This does not remove <col> nor <colgroup> elements.
*
* @param aNumberOfColumnsToDelete Number of columns to remove. This is
* ignored if 2 ore more cells are
* selected.
*/
void deleteTableColumn(in long aNumberOfColumnsToDelete);

/**
* deleteTableRow() removes <tr> elements.
Expand Down

0 comments on commit 8c3f8a1

Please sign in to comment.