From 197de3d0a2629bdad4c984e53ef8fcc4caf822ab Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Wed, 1 Nov 2023 01:32:40 +0000 Subject: [PATCH] Bug 1858794 - Make `AutoMoveOneLineHandler` track the new line range while moving content nodes r=m_kato It moves nodes in a range to new place one by one. At this time, the moved position range is not tracked. Therefore, if the DOM tree is unexpectedly changed by `HTMLEditor` itself, the range gets broken. E.g., in this case, deleting empty inline element causes the range after the source position is broken. Differential Revision: https://phabricator.services.mozilla.com/D192180 --- editor/libeditor/HTMLEditorDeleteHandler.cpp | 13 +++++- .../delete-at-text-following-svg.html | 41 +++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 testing/web-platform/tests/editing/crashtests/delete-at-text-following-svg.html diff --git a/editor/libeditor/HTMLEditorDeleteHandler.cpp b/editor/libeditor/HTMLEditorDeleteHandler.cpp index 5d3cb601489e..80493fa86c41 100644 --- a/editor/libeditor/HTMLEditorDeleteHandler.cpp +++ b/editor/libeditor/HTMLEditorDeleteHandler.cpp @@ -5399,6 +5399,8 @@ Result HTMLEditor::AutoMoveOneLineHandler::Run( for (const OwningNonNull& content : arrayOfContents) { { AutoEditorDOMRangeChildrenInvalidator lockOffsets(movedContentRange); + AutoTrackDOMRange trackMovedContentRange(aHTMLEditor.RangeUpdaterRef(), + &movedContentRange); // If the content is a block element, move all children of it to the // new container, and then, remove the (probably) empty block element. if (HTMLEditUtils::IsBlockElement( @@ -5413,7 +5415,6 @@ Result HTMLEditor::AutoMoveOneLineHandler::Run( return moveChildrenResult; } moveContentsInLineResult |= moveChildrenResult.inspect(); - moveContentsInLineResult.MarkAsHandled(); // MOZ_KnownLive due to bug 1620312 nsresult rv = aHTMLEditor.DeleteNodeWithTransaction(MOZ_KnownLive(content)); @@ -5428,6 +5429,7 @@ Result HTMLEditor::AutoMoveOneLineHandler::Run( // If the moving content is a comment node or an empty inline node, we // don't want it to appear in the dist paragraph. else if (content->IsComment() || + (content->IsText() && !content->AsText()->TextDataLength()) || HTMLEditUtils::IsEmptyInlineContainer( content, {EmptyCheckOption::TreatSingleBRElementAsVisible, @@ -5461,10 +5463,16 @@ Result HTMLEditor::AutoMoveOneLineHandler::Run( moveContentsInLineResult |= moveNodeOrChildrenResult.inspect(); } } + moveContentsInLineResult.MarkAsHandled(); + if (NS_WARN_IF(!movedContentRange.IsPositioned())) { + return Err(NS_ERROR_EDITOR_UNEXPECTED_DOM_TREE); + } // For backward compatibility, we should move contents to end of the // container if the instance is created without specific insertion point. if (ForceMoveToEndOfContainer()) { pointToInsert = NextInsertionPointRef(); + MOZ_ASSERT(pointToInsert.IsSet()); + MOZ_ASSERT(movedContentRange.StartRef().EqualsOrIsBefore(pointToInsert)); movedContentRange.SetEnd(pointToInsert); } // And also if pointToInsert has been made invalid with removing preceding @@ -5482,6 +5490,9 @@ Result HTMLEditor::AutoMoveOneLineHandler::Run( pointToInsert = NextInsertionPointRef(); if (!aHTMLEditor.MayHaveMutationEventListeners() || movedContentRange.EndRef().IsBefore(pointToInsert)) { + MOZ_ASSERT(pointToInsert.IsSet()); + MOZ_ASSERT( + movedContentRange.StartRef().EqualsOrIsBefore(pointToInsert)); movedContentRange.SetEnd(pointToInsert); } } diff --git a/testing/web-platform/tests/editing/crashtests/delete-at-text-following-svg.html b/testing/web-platform/tests/editing/crashtests/delete-at-text-following-svg.html new file mode 100644 index 000000000000..cc2040a2a346 --- /dev/null +++ b/testing/web-platform/tests/editing/crashtests/delete-at-text-following-svg.html @@ -0,0 +1,41 @@ + + + + + + + + + + +AA +
  • +
  • + +