From 1f8f4485a5a0cc12b770f4295e3b2db657639550 Mon Sep 17 00:00:00 2001 From: Mirko Brodesser Date: Thu, 14 May 2020 10:08:32 +0000 Subject: [PATCH] Bug 1635709: part 16) Replace `GetTableCellLocationFromRange` with `GetTableSelectionMode`. r=masayuki The location wasn't used from the caller of `GetTableCellLocationFromRange`. However, `GetTableCellLocationFromRange` included flushing frames, this is now done in `HTMLEditor::CellIndexes::Update`. Differential Revision: https://phabricator.services.mozilla.com/D75098 --- dom/base/Selection.cpp | 44 +------------------- editor/libeditor/HTMLEditor.h | 24 +++++++---- editor/libeditor/HTMLTableEditor.cpp | 62 ++++++++++++++++++---------- 3 files changed, 59 insertions(+), 71 deletions(-) diff --git a/dom/base/Selection.cpp b/dom/base/Selection.cpp index 831bb83e6294..8e10a3e690b9 100644 --- a/dom/base/Selection.cpp +++ b/dom/base/Selection.cpp @@ -456,45 +456,6 @@ static nsresult GetTableSelectionMode(const nsRange& aRange, return NS_OK; } -MOZ_CAN_RUN_SCRIPT static nsresult GetTableCellLocationFromRange( - const nsRange& aRange, TableSelectionMode* aSelectionType, int32_t* aRow, - int32_t* aCol) { - if (!aSelectionType || !aRow || !aCol) { - return NS_ERROR_NULL_POINTER; - } - - *aSelectionType = TableSelectionMode::None; - *aRow = 0; - *aCol = 0; - - nsresult result = GetTableSelectionMode(aRange, aSelectionType); - if (NS_FAILED(result)) return result; - - // Don't fail if range does not point to a single table cell, - // let aSelectionType tell user if we don't have a cell - if (*aSelectionType != TableSelectionMode::Cell) { - return NS_OK; - } - - // Get the child content (the cell) pointed to by starting node of range - // We do minimal checking since GetTableSelectionMode assures - // us that this really is a table cell - nsCOMPtr child = aRange.GetChildAtStartOffset(); - if (!child) return NS_ERROR_FAILURE; - - // GetCellLayout depends on current frame, we need flush frame to get - // nsITableCellLayout - if (RefPtr presShell = child->OwnerDoc()->GetPresShell()) { - presShell->FlushPendingNotifications(FlushType::Frames); - } - - // Note: This is a non-ref-counted pointer to the frame - nsITableCellLayout* cellLayout = nsFrameSelection::GetCellLayout(child); - if (!cellLayout) return NS_ERROR_FAILURE; - - return cellLayout->GetCellIndexes(*aRow, *aCol); -} - nsresult Selection::MaybeAddTableCellRange(nsRange& aRange, bool* aDidAddRange, int32_t* aOutIndex) { if (!aDidAddRange || !aOutIndex) { @@ -506,12 +467,9 @@ nsresult Selection::MaybeAddTableCellRange(nsRange& aRange, bool* aDidAddRange, if (!mFrameSelection) return NS_OK; - nsresult result; - // Get if we are adding a cell selection and the row, col of cell if we are - int32_t newRow, newCol; TableSelectionMode tableMode; - result = GetTableCellLocationFromRange(aRange, &tableMode, &newRow, &newCol); + nsresult result = GetTableSelectionMode(aRange, &tableMode); if (NS_FAILED(result)) return result; // If not adding a cell range, we are done here diff --git a/editor/libeditor/HTMLEditor.h b/editor/libeditor/HTMLEditor.h index ff6c20c4a8da..e203d6e035d3 100644 --- a/editor/libeditor/HTMLEditor.h +++ b/editor/libeditor/HTMLEditor.h @@ -3243,10 +3243,13 @@ class HTMLEditor final : public TextEditor, * @param aRv Returns error if layout information is not * available or given element is not a table cell. */ - CellIndexes(Element& aCellElement, ErrorResult& aRv) + // TODO: annotate with `MOZ_CAN_RUN_SCRIPT` instead. + MOZ_CAN_RUN_SCRIPT_BOUNDARY CellIndexes(Element& aCellElement, + PresShell* aPresShell, + ErrorResult& aRv) : mRow(-1), mColumn(-1) { MOZ_ASSERT(!aRv.Failed()); - Update(aCellElement, aRv); + Update(aCellElement, aPresShell, aRv); } /** @@ -3254,7 +3257,10 @@ class HTMLEditor final : public TextEditor, * * @param See above. */ - void Update(Element& aCellElement, ErrorResult& aRv); + // TODO: annotate with `MOZ_CAN_RUN_SCRIPT` instead. + MOZ_CAN_RUN_SCRIPT_BOUNDARY void Update(Element& aCellElement, + PresShell* aPresShell, + ErrorResult& aRv); /** * This constructor initializes mRowIndex and mColumnIndex with indexes of @@ -3266,8 +3272,10 @@ class HTMLEditor final : public TextEditor, * which contains anchor of Selection, or layout * information is not available. */ - CellIndexes(HTMLEditor& aHTMLEditor, Selection& aSelection, - ErrorResult& aRv) + // TODO: annotate with `MOZ_CAN_RUN_SCRIPT` instead. + MOZ_CAN_RUN_SCRIPT_BOUNDARY CellIndexes(HTMLEditor& aHTMLEditor, + Selection& aSelection, + ErrorResult& aRv) : mRow(-1), mColumn(-1) { Update(aHTMLEditor, aSelection, aRv); } @@ -3278,8 +3286,10 @@ class HTMLEditor final : public TextEditor, * * @param See above. */ - void Update(HTMLEditor& aHTMLEditor, Selection& aSelection, - ErrorResult& aRv); + // TODO: annotate with `MOZ_CAN_RUN_SCRIPT` instead. + MOZ_CAN_RUN_SCRIPT_BOUNDARY void Update(HTMLEditor& aHTMLEditor, + Selection& aSelection, + ErrorResult& aRv); bool operator==(const CellIndexes& aOther) const { return mRow == aOther.mRow && mColumn == aOther.mColumn; diff --git a/editor/libeditor/HTMLTableEditor.cpp b/editor/libeditor/HTMLTableEditor.cpp index ed7a96c2b39b..edf4fd2fe5b1 100644 --- a/editor/libeditor/HTMLTableEditor.cpp +++ b/editor/libeditor/HTMLTableEditor.cpp @@ -1127,7 +1127,8 @@ nsresult HTMLEditor::DeleteTableCellWithTransaction( // When 2 or more cells are selected, ignore aNumberOfCellsToRemove and // remove all selected cells. - CellIndexes firstCellIndexes(*firstSelectedCellElement, error); + const RefPtr presShell{GetPresShell()}; + CellIndexes firstCellIndexes(*firstSelectedCellElement, presShell, error); if (error.Failed()) { NS_WARNING("CellIndexes failed"); return error.StealNSResult(); @@ -1162,7 +1163,7 @@ nsresult HTMLEditor::DeleteTableCellWithTransaction( if (!cell) { break; } - CellIndexes nextSelectedCellIndexes(*cell, error); + CellIndexes nextSelectedCellIndexes(*cell, presShell, error); if (error.Failed()) { NS_WARNING("CellIndexes failed"); return error.StealNSResult(); @@ -1219,7 +1220,7 @@ nsresult HTMLEditor::DeleteTableCellWithTransaction( if (!cell) { break; } - CellIndexes nextSelectedCellIndexes(*cell, error); + CellIndexes nextSelectedCellIndexes(*cell, presShell, error); if (error.Failed()) { NS_WARNING("CellIndexes failed"); return error.StealNSResult(); @@ -1269,7 +1270,7 @@ nsresult HTMLEditor::DeleteTableCellWithTransaction( return NS_OK; } - CellIndexes nextCellIndexes(*nextCell, error); + CellIndexes nextCellIndexes(*nextCell, presShell, error); if (error.Failed()) { NS_WARNING("CellIndexes failed"); return error.StealNSResult(); @@ -1346,7 +1347,8 @@ nsresult HTMLEditor::DeleteTableCellContentsWithTransaction() { } if (firstSelectedCellElement) { - CellIndexes firstCellIndexes(*firstSelectedCellElement, error); + const RefPtr presShell{GetPresShell()}; + CellIndexes firstCellIndexes(*firstSelectedCellElement, presShell, error); if (error.Failed()) { NS_WARNING("CellIndexes failed"); return error.StealNSResult(); @@ -1458,7 +1460,8 @@ nsresult HTMLEditor::DeleteSelectedTableColumnsWithTransaction( MOZ_ASSERT(SelectionRefPtr()->RangeCount()); if (firstSelectedCellElement && SelectionRefPtr()->RangeCount() > 1) { - CellIndexes firstCellIndexes(*firstSelectedCellElement, error); + const RefPtr presShell{GetPresShell()}; + CellIndexes firstCellIndexes(*firstSelectedCellElement, presShell, error); if (error.Failed()) { NS_WARNING("CellIndexes failed"); return error.StealNSResult(); @@ -1488,9 +1491,10 @@ nsresult HTMLEditor::DeleteSelectedTableColumnsWithTransaction( // If 2 or more cells are selected, remove all columns which contain selected // cells. I.e., we ignore aNumberOfColumnsToDelete in this case. + const RefPtr presShell{GetPresShell()}; for (cell = firstSelectedCellElement; cell;) { if (cell != firstSelectedCellElement) { - CellIndexes cellIndexes(*cell, error); + CellIndexes cellIndexes(*cell, presShell, error); if (error.Failed()) { NS_WARNING("CellIndexes failed"); return error.StealNSResult(); @@ -1510,7 +1514,7 @@ nsresult HTMLEditor::DeleteSelectedTableColumnsWithTransaction( if (!cell) { break; } - CellIndexes cellIndexes(*cell, error); + CellIndexes cellIndexes(*cell, presShell, error); if (error.Failed()) { NS_WARNING("CellIndexes failed"); return error.StealNSResult(); @@ -1714,7 +1718,8 @@ nsresult HTMLEditor::DeleteSelectedTableRowsWithTransaction( if (firstSelectedCellElement && SelectionRefPtr()->RangeCount() > 1) { // Fetch indexes again - may be different for selected cells - CellIndexes firstCellIndexes(*firstSelectedCellElement, error); + const RefPtr presShell{GetPresShell()}; + CellIndexes firstCellIndexes(*firstSelectedCellElement, presShell, error); if (error.Failed()) { NS_WARNING("CellIndexes failed"); return error.StealNSResult(); @@ -1758,9 +1763,10 @@ nsresult HTMLEditor::DeleteSelectedTableRowsWithTransaction( // If 2 or more cells are selected, remove all rows which contain selected // cells. I.e., we ignore aNumberOfRowsToDelete in this case. + const RefPtr presShell{GetPresShell()}; for (cell = firstSelectedCellElement; cell;) { if (cell != firstSelectedCellElement) { - CellIndexes cellIndexes(*cell, error); + CellIndexes cellIndexes(*cell, presShell, error); if (error.Failed()) { NS_WARNING("CellIndexes failed"); return error.StealNSResult(); @@ -1780,7 +1786,7 @@ nsresult HTMLEditor::DeleteSelectedTableRowsWithTransaction( if (!cell) { break; } - CellIndexes cellIndexes(*cell, error); + CellIndexes cellIndexes(*cell, presShell, error); if (error.Failed()) { NS_WARNING("CellIndexes failed"); return error.StealNSResult(); @@ -3431,7 +3437,8 @@ NS_IMETHODIMP HTMLEditor::GetCellIndexes(Element* aCellElement, } ErrorResult error; - CellIndexes cellIndexes(*aCellElement, error); + const RefPtr presShell{GetPresShell()}; + CellIndexes cellIndexes(*aCellElement, presShell, error); if (error.Failed()) { NS_WARNING("CellIndexes failed"); return EditorBase::ToGenericNSResult(error.StealNSResult()); @@ -3456,16 +3463,26 @@ void HTMLEditor::CellIndexes::Update(HTMLEditor& aHTMLEditor, aRv.Throw(NS_ERROR_FAILURE); return; } - Update(*cellElement, aRv); + + RefPtr presShell{aHTMLEditor.GetPresShell()}; + Update(*cellElement, presShell, aRv); NS_WARNING_ASSERTION(!aRv.Failed(), "CellIndexes::Update() failed"); } -void HTMLEditor::CellIndexes::Update(Element& aCellElement, ErrorResult& aRv) { +void HTMLEditor::CellIndexes::Update(Element& aCellElement, + PresShell* aPresShell, ErrorResult& aRv) { MOZ_ASSERT(!aRv.Failed()); - // XXX If the table cell is created immediately before this call, e.g., - // using innerHTML, frames have not been created yet. In such case, - // shouldn't we flush pending layout? + // If the table cell is created immediately before this call, e.g., using + // innerHTML, frames have not been created yet. Hence, flush layout to create + // them. + if (NS_WARN_IF(!aPresShell)) { + aRv.Throw(NS_ERROR_INVALID_ARG); + return; + } + + aPresShell->FlushPendingNotifications(FlushType::Frames); + nsIFrame* frameOfCell = aCellElement.GetPrimaryFrame(); if (!frameOfCell) { NS_WARNING("There was no layout information of aCellElement"); @@ -3845,7 +3862,8 @@ nsresult HTMLEditor::GetCellContext(Element** aTable, Element** aCell, // Get the rest of the related data only if requested if (aRowIndex || aColumnIndex) { ErrorResult error; - CellIndexes cellIndexes(*cell, error); + const RefPtr presShell{GetPresShell()}; + CellIndexes cellIndexes(*cell, presShell, error); if (error.Failed()) { NS_WARNING("CellIndexes failed"); return error.StealNSResult(); @@ -4073,7 +4091,8 @@ void HTMLEditor::CellAndIndexes::Update(HTMLEditor& aHTMLEditor, return; } - mIndexes.Update(*mElement, aRv); + const RefPtr presShell{aHTMLEditor.GetPresShell()}; + mIndexes.Update(*mElement, presShell, aRv); NS_WARNING_ASSERTION(!aRv.Failed(), "CellIndexes::Update() failed"); } @@ -4345,11 +4364,12 @@ NS_IMETHODIMP HTMLEditor::GetSelectedCellsType(Element* aElement, // Store indexes of each row/col to avoid duplication of searches nsTArray indexArray; + const RefPtr presShell{GetPresShell()}; bool allCellsInRowAreSelected = false; bool allCellsInColAreSelected = false; IgnoredErrorResult ignoredError; while (selectedCell) { - CellIndexes selectedCellIndexes(*selectedCell, error); + CellIndexes selectedCellIndexes(*selectedCell, presShell, error); if (error.Failed()) { NS_WARNING("CellIndexes failed"); return EditorBase::ToGenericNSResult(error.StealNSResult()); @@ -4387,7 +4407,7 @@ NS_IMETHODIMP HTMLEditor::GetSelectedCellsType(Element* aElement, ignoredError.SuppressException(); while (selectedCell) { - CellIndexes selectedCellIndexes(*selectedCell, error); + CellIndexes selectedCellIndexes(*selectedCell, presShell, error); if (error.Failed()) { NS_WARNING("CellIndexes failed"); return EditorBase::ToGenericNSResult(error.StealNSResult());