From 3397c35fa400ea9bd955e8b500afa9c54c97faff Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Thu, 6 Aug 2020 08:07:39 +0000 Subject: [PATCH] Bug 1656799 - part 3: Make `WhiteSpaceVisibilityKeeper::PrepareToDeleteRange()` not track `EditorDOMPoint`s r=m_kato It may be faster to use `AutoTrackDOMRange` directly. Therefore, current `WhiteSpaceVisibilityKeeper::PrepareToDeleteRange()` should be renamed to `WhiteSpaceVisibilityKeeper::PrepareToDeleteRangeAndTrackPoints()` and we should make `HTMLEditor::HandleDeleteNonCollapsedSelection()` track the range by itself. Depends on D85846 Differential Revision: https://phabricator.services.mozilla.com/D85847 --- editor/libeditor/EditorDOMPoint.h | 12 +++- editor/libeditor/HTMLEditSubActionHandler.cpp | 37 +++++------- editor/libeditor/WSRunObject.cpp | 59 ++++++++----------- editor/libeditor/WSRunObject.h | 34 ++++++++++- 4 files changed, 81 insertions(+), 61 deletions(-) diff --git a/editor/libeditor/EditorDOMPoint.h b/editor/libeditor/EditorDOMPoint.h index f3b6eb47e3af..b7e5c7c48c76 100644 --- a/editor/libeditor/EditorDOMPoint.h +++ b/editor/libeditor/EditorDOMPoint.h @@ -10,6 +10,7 @@ #include "mozilla/Attributes.h" #include "mozilla/Maybe.h" #include "mozilla/RangeBoundary.h" +#include "mozilla/dom/AbstractRange.h" #include "mozilla/dom/Element.h" #include "mozilla/dom/Text.h" #include "nsAtom.h" @@ -1018,8 +1019,8 @@ template class EditorDOMRangeBase final { public: EditorDOMRangeBase() = default; - template - explicit EditorDOMRangeBase(const PointType& aStart) + template + explicit EditorDOMRangeBase(const EditorDOMPointBase& aStart) : mStart(aStart), mEnd(aStart) { MOZ_ASSERT(!mStart.IsSet() || mStart.IsSetAndValid()); } @@ -1032,6 +1033,13 @@ class EditorDOMRangeBase final { MOZ_ASSERT_IF(mStart.IsSet() && mEnd.IsSet(), mStart.EqualsOrIsBefore(mEnd)); } + explicit EditorDOMRangeBase(const dom::AbstractRange& aRange) + : mStart(aRange.StartRef()), mEnd(aRange.EndRef()) { + MOZ_ASSERT_IF(mStart.IsSet(), mStart.IsSetAndValid()); + MOZ_ASSERT_IF(mEnd.IsSet(), mEnd.IsSetAndValid()); + MOZ_ASSERT_IF(mStart.IsSet() && mEnd.IsSet(), + mStart.EqualsOrIsBefore(mEnd)); + } template MOZ_NEVER_INLINE_DEBUG void SetStart(const PointType& aStart) { diff --git a/editor/libeditor/HTMLEditSubActionHandler.cpp b/editor/libeditor/HTMLEditSubActionHandler.cpp index 133da2eb365b..11d65f07fe36 100644 --- a/editor/libeditor/HTMLEditSubActionHandler.cpp +++ b/editor/libeditor/HTMLEditSubActionHandler.cpp @@ -2794,13 +2794,15 @@ EditActionResult HTMLEditor::HandleDeleteCollapsedSelectionAtVisibleChar( startToDelete = range->StartRef(); endToDelete = range->EndRef(); } - nsresult rv = WhiteSpaceVisibilityKeeper::PrepareToDeleteRange( + nsresult rv = WhiteSpaceVisibilityKeeper::PrepareToDeleteRangeAndTrackPoints( *this, &startToDelete, &endToDelete); if (NS_WARN_IF(Destroyed())) { return EditActionResult(NS_ERROR_EDITOR_DESTROYED); } if (NS_FAILED(rv)) { - NS_WARNING("WhiteSpaceVisibilityKeeper::PrepareToDeleteRange() failed"); + NS_WARNING( + "WhiteSpaceVisibilityKeeper::PrepareToDeleteRangeAndTrackPoints() " + "failed"); return EditActionResult(rv); } if (MaybeHasMutationEventListeners( @@ -3323,31 +3325,24 @@ EditActionResult HTMLEditor::HandleDeleteNonCollapsedSelection( // surrounding white-space in preparation to delete selection. if (!IsPlaintextEditor()) { AutoTransactionsConserveSelection dontChangeMySelection(*this); - EditorDOMPoint firstRangeStart(rangesToDelete.FirstRangeRef()->StartRef()); - EditorDOMPoint firstRangeEnd(rangesToDelete.FirstRangeRef()->EndRef()); + AutoTrackDOMRange firstRangeTracker(RangeUpdaterRef(), + &rangesToDelete.FirstRangeRef()); nsresult rv = WhiteSpaceVisibilityKeeper::PrepareToDeleteRange( - *this, &firstRangeStart, &firstRangeEnd); + *this, EditorDOMRange(rangesToDelete.FirstRangeRef())); if (NS_FAILED(rv)) { - NS_WARNING("WhiteSpaceVisibilityKeeper::PrepareToDeleteRange() failed"); + NS_WARNING( + "WhiteSpaceVisibilityKeeper::PrepareToDeleteRange() " + "failed"); return EditActionResult(rv); } - if (MaybeHasMutationEventListeners( - NS_EVENT_BITS_MUTATION_NODEREMOVED | - NS_EVENT_BITS_MUTATION_NODEREMOVEDFROMDOCUMENT | - NS_EVENT_BITS_MUTATION_ATTRMODIFIED | - NS_EVENT_BITS_MUTATION_CHARACTERDATAMODIFIED) && - (NS_WARN_IF(!firstRangeStart.IsSetAndValid()) || - NS_WARN_IF(!firstRangeEnd.IsSetAndValid()))) { - NS_WARNING("Mutation event listener changed the DOM tree"); + if (NS_WARN_IF( + !rangesToDelete.FirstRangeRef()->StartRef().IsSetAndValid()) || + NS_WARN_IF(!rangesToDelete.FirstRangeRef()->EndRef().IsSetAndValid())) { + NS_WARNING( + "WhiteSpaceVisibilityKeeper::PrepareToDeleteRange() made the firstr " + "range invalid"); return EditActionHandled(NS_ERROR_EDITOR_UNEXPECTED_DOM_TREE); } - rv = rangesToDelete.FirstRangeRef()->SetStartAndEnd( - firstRangeStart.ToRawRangeBoundary(), - firstRangeEnd.ToRawRangeBoundary()); - if (NS_FAILED(rv)) { - NS_WARNING("Failed to modify first range to delete"); - return EditActionResult(NS_ERROR_FAILURE); - } } // XXX This is odd. We do we simply use `DeleteRangesWithTransaction()` diff --git a/editor/libeditor/WSRunObject.cpp b/editor/libeditor/WSRunObject.cpp index 1d1b2225c532..9b15998396c0 100644 --- a/editor/libeditor/WSRunObject.cpp +++ b/editor/libeditor/WSRunObject.cpp @@ -63,29 +63,6 @@ template WSRunScanner::TextFragmentData::TextFragmentData( template WSRunScanner::TextFragmentData::TextFragmentData( const EditorDOMPointInText& aPoint, const Element* aEditingHost); -nsresult WhiteSpaceVisibilityKeeper::PrepareToDeleteRange( - HTMLEditor& aHTMLEditor, EditorDOMPoint* aStartPoint, - EditorDOMPoint* aEndPoint) { - MOZ_ASSERT(aStartPoint); - MOZ_ASSERT(aEndPoint); - - if (NS_WARN_IF(!aStartPoint->IsSet()) || NS_WARN_IF(!aEndPoint->IsSet())) { - return NS_ERROR_INVALID_ARG; - } - - AutoTrackDOMPoint trackerStart(aHTMLEditor.RangeUpdaterRef(), aStartPoint); - AutoTrackDOMPoint trackerEnd(aHTMLEditor.RangeUpdaterRef(), aEndPoint); - - nsresult rv = WhiteSpaceVisibilityKeeper:: - MakeSureToKeepVisibleStateOfWhiteSpacesAroundDeletingRange( - aHTMLEditor, EditorDOMRange(*aStartPoint, *aEndPoint)); - NS_WARNING_ASSERTION( - NS_SUCCEEDED(rv), - "WhiteSpaceVisibilityKeeper::" - "MakeSureToKeepVisibleStateOfWhiteSpacesAroundDeletingRange() failed"); - return rv; -} - nsresult WhiteSpaceVisibilityKeeper::PrepareToDeleteNode( HTMLEditor& aHTMLEditor, nsIContent* aContent) { if (NS_WARN_IF(!aContent)) { @@ -980,10 +957,13 @@ nsresult WhiteSpaceVisibilityKeeper::DeletePreviousWhiteSpace( EditorDOMPoint endToDelete = textFragmentDataAtDeletion.GetEndOfCollapsibleASCIIWhiteSpaces( atPreviousCharOfStart); - nsresult rv = WhiteSpaceVisibilityKeeper::PrepareToDeleteRange( - aHTMLEditor, &startToDelete, &endToDelete); + nsresult rv = + WhiteSpaceVisibilityKeeper::PrepareToDeleteRangeAndTrackPoints( + aHTMLEditor, &startToDelete, &endToDelete); if (NS_FAILED(rv)) { - NS_WARNING("WhiteSpaceVisibilityKeeper::PrepareToDeleteRange() failed"); + NS_WARNING( + "WhiteSpaceVisibilityKeeper::PrepareToDeleteRangeAndTrackPoints() " + "failed"); return rv; } @@ -1000,10 +980,13 @@ nsresult WhiteSpaceVisibilityKeeper::DeletePreviousWhiteSpace( if (atPreviousCharOfStart.IsCharNBSP()) { EditorDOMPoint startToDelete(atPreviousCharOfStart); EditorDOMPoint endToDelete(startToDelete.NextPoint()); - nsresult rv = WhiteSpaceVisibilityKeeper::PrepareToDeleteRange( - aHTMLEditor, &startToDelete, &endToDelete); + nsresult rv = + WhiteSpaceVisibilityKeeper::PrepareToDeleteRangeAndTrackPoints( + aHTMLEditor, &startToDelete, &endToDelete); if (NS_FAILED(rv)) { - NS_WARNING("WhiteSpaceVisibilityKeeper::PrepareToDeleteRange() failed"); + NS_WARNING( + "WhiteSpaceVisibilityKeeper::PrepareToDeleteRangeAndTrackPoints() " + "failed"); return rv; } @@ -1056,10 +1039,13 @@ nsresult WhiteSpaceVisibilityKeeper::DeleteInclusiveNextWhiteSpace( EditorDOMPoint endToDelete = textFragmentDataAtDeletion.GetEndOfCollapsibleASCIIWhiteSpaces( atNextCharOfStart); - nsresult rv = WhiteSpaceVisibilityKeeper::PrepareToDeleteRange( - aHTMLEditor, &startToDelete, &endToDelete); + nsresult rv = + WhiteSpaceVisibilityKeeper::PrepareToDeleteRangeAndTrackPoints( + aHTMLEditor, &startToDelete, &endToDelete); if (NS_FAILED(rv)) { - NS_WARNING("WhiteSpaceVisibilityKeeper::PrepareToDeleteRange() failed"); + NS_WARNING( + "WhiteSpaceVisibilityKeeper::PrepareToDeleteRangeAndTrackPoints() " + "failed"); return rv; } @@ -1076,10 +1062,13 @@ nsresult WhiteSpaceVisibilityKeeper::DeleteInclusiveNextWhiteSpace( if (atNextCharOfStart.IsCharNBSP()) { EditorDOMPoint startToDelete(atNextCharOfStart); EditorDOMPoint endToDelete(startToDelete.NextPoint()); - nsresult rv = WhiteSpaceVisibilityKeeper::PrepareToDeleteRange( - aHTMLEditor, &startToDelete, &endToDelete); + nsresult rv = + WhiteSpaceVisibilityKeeper::PrepareToDeleteRangeAndTrackPoints( + aHTMLEditor, &startToDelete, &endToDelete); if (NS_FAILED(rv)) { - NS_WARNING("WhiteSpaceVisibilityKeeper::PrepareToDeleteRange() failed"); + NS_WARNING( + "WhiteSpaceVisibilityKeeper::PrepareToDeleteRangeAndTrackPoints() " + "failed"); return rv; } diff --git a/editor/libeditor/WSRunObject.h b/editor/libeditor/WSRunObject.h index 553f3f70284f..d1955a4c4915 100644 --- a/editor/libeditor/WSRunObject.h +++ b/editor/libeditor/WSRunObject.h @@ -1192,9 +1192,37 @@ class WhiteSpaceVisibilityKeeper final { // and offsets are adjusted in response to any dom changes we make while // adjusting ws. // example of fixup: trailingws before aStartPoint needs to be removed. - MOZ_CAN_RUN_SCRIPT static nsresult PrepareToDeleteRange( - HTMLEditor& aHTMLEditor, EditorDOMPoint* aStartPoint, - EditorDOMPoint* aEndPoint); + [[nodiscard]] MOZ_CAN_RUN_SCRIPT static nsresult + PrepareToDeleteRangeAndTrackPoints(HTMLEditor& aHTMLEditor, + EditorDOMPoint* aStartPoint, + EditorDOMPoint* aEndPoint) { + MOZ_ASSERT(aStartPoint->IsSetAndValid()); + MOZ_ASSERT(aEndPoint->IsSetAndValid()); + AutoTrackDOMPoint trackerStart(aHTMLEditor.RangeUpdaterRef(), aStartPoint); + AutoTrackDOMPoint trackerEnd(aHTMLEditor.RangeUpdaterRef(), aEndPoint); + return WhiteSpaceVisibilityKeeper::PrepareToDeleteRange( + aHTMLEditor, EditorDOMRange(*aStartPoint, *aEndPoint)); + } + [[nodiscard]] MOZ_CAN_RUN_SCRIPT static nsresult PrepareToDeleteRange( + HTMLEditor& aHTMLEditor, const EditorDOMPoint& aStartPoint, + const EditorDOMPoint& aEndPoint) { + MOZ_ASSERT(aStartPoint.IsSetAndValid()); + MOZ_ASSERT(aEndPoint.IsSetAndValid()); + return WhiteSpaceVisibilityKeeper::PrepareToDeleteRange( + aHTMLEditor, EditorDOMRange(aStartPoint, aEndPoint)); + } + [[nodiscard]] MOZ_CAN_RUN_SCRIPT static nsresult PrepareToDeleteRange( + HTMLEditor& aHTMLEditor, const EditorDOMRange& aRange) { + MOZ_ASSERT(aRange.IsPositionedAndValid()); + nsresult rv = WhiteSpaceVisibilityKeeper:: + MakeSureToKeepVisibleStateOfWhiteSpacesAroundDeletingRange(aHTMLEditor, + aRange); + NS_WARNING_ASSERTION( + NS_SUCCEEDED(rv), + "WhiteSpaceVisibilityKeeper::" + "MakeSureToKeepVisibleStateOfWhiteSpacesAroundDeletingRange() failed"); + return rv; + } // PrepareToDeleteNode fixes up ws before and after aContent in preparation // for aContent to be deleted. Example of fixup: trailingws before