From 9a4701f405d6189ae2bb06104bf30a568b9a0ebd Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Thu, 15 Jun 2023 08:15:38 +0000 Subject: [PATCH] Bug 1756237 - Make `HTMLEditor::HandleHTMLIndentAroundRanges` validate DOM tree in each time of the loop r=m_kato There are 2 possible scenarios which are not handled by the method. 1. Moving content node to new `
` has already been moved to outside of the editing host. 2. There is no container to insert new `
`, e.g., in an inline editing host. In the case #1, we should ignore the ex-child node. In the case #2, we should abort it. Note that Chrome inserts `
` even if there is no proper container. However, such behavior is disagreed in interop-2023. Therefore, it's okay just to abort it for now. Depends on D180781 Differential Revision: https://phabricator.services.mozilla.com/D180782 --- editor/libeditor/HTMLEditSubActionHandler.cpp | 54 ++++++++++--------- editor/libeditor/HTMLEditUtils.h | 30 +++++++++++ ...t-in-inline-editing-host-outside-body.html | 18 +++++++ 3 files changed, 76 insertions(+), 26 deletions(-) create mode 100644 testing/web-platform/tests/editing/crashtests/indent-in-inline-editing-host-outside-body.html diff --git a/editor/libeditor/HTMLEditSubActionHandler.cpp b/editor/libeditor/HTMLEditSubActionHandler.cpp index 8e89d26d4643..52e60449a545 100644 --- a/editor/libeditor/HTMLEditSubActionHandler.cpp +++ b/editor/libeditor/HTMLEditSubActionHandler.cpp @@ -5350,6 +5350,7 @@ nsresult HTMLEditor::HandleHTMLIndentAroundRanges(AutoRangeArray& aRanges, } } + // FIXME: Split ancestors when we consider to indent the range. Result splitAtBRElementsResult = MaybeSplitElementsAtEveryBRElement(arrayOfContents, EditSubAction::eIndent); @@ -5375,7 +5376,18 @@ nsresult HTMLEditor::HandleHTMLIndentAroundRanges(AutoRangeArray& aRanges, return NS_ERROR_FAILURE; } + // If there is no element which can have
, abort. + if (NS_WARN_IF(!HTMLEditUtils::GetInsertionPointInInclusiveAncestor( + *nsGkAtoms::blockquote, pointToInsertBlockquoteElement, + &aEditingHost) + .IsSet())) { + return NS_ERROR_EDITOR_UNEXPECTED_DOM_TREE; + } + // Make sure we can put a block here. + // XXX Unfortunately, this calls + // MaybeSplitAncestorsForInsertWithTransaction() then, + // HTMLEditUtils::GetInsertionPointInInclusiveAncestor() is called again. Result createNewBlockquoteElementResult = InsertElementWithSplittingAncestorsWithTransaction( *nsGkAtoms::blockquote, pointToInsertBlockquoteElement, @@ -5458,6 +5470,11 @@ nsresult HTMLEditor::HandleHTMLIndentAroundRanges(AutoRangeArray& aRanges, continue; } + // If the content has been moved to different place, ignore it. + if (MOZ_UNLIKELY(!content->IsInclusiveDescendantOf(&aEditingHost))) { + continue; + } + if (HTMLEditUtils::IsAnyListElement(atContent.GetContainer())) { const RefPtr oldSubListElement = subListElement; // MOZ_KnownLive because 'arrayOfContents' is guaranteed to @@ -9257,37 +9274,22 @@ HTMLEditor::MaybeSplitAncestorsForInsertWithTransaction( // The point must be descendant of editing host. // XXX Isn't it a valid case if it points a direct child of aEditingHost? - if (aStartOfDeepestRightNode.GetContainer() != &aEditingHost && - !EditorUtils::IsDescendantOf(*aStartOfDeepestRightNode.GetContainer(), - aEditingHost)) { - NS_WARNING("aStartOfDeepestRightNode was not in editing host"); + if (NS_WARN_IF( + !aStartOfDeepestRightNode.GetContainer()->IsInclusiveDescendantOf( + &aEditingHost))) { return Err(NS_ERROR_INVALID_ARG); } // Look for a node that can legally contain the tag. - EditorDOMPoint pointToInsert(aStartOfDeepestRightNode); - for (; pointToInsert.IsSet(); - pointToInsert.Set(pointToInsert.GetContainer())) { - // We cannot split active editing host and its ancestor. So, there is - // no element to contain the specified element. - if (pointToInsert.GetChild() == &aEditingHost) { - NS_WARNING( - "HTMLEditor::MaybeSplitAncestorsForInsertWithTransaction() reached " - "editing host"); - return Err(NS_ERROR_FAILURE); - } - - if (HTMLEditUtils::CanNodeContain(*pointToInsert.GetContainer(), aTag)) { - // Found an ancestor node which can contain the element. - break; - } + const EditorDOMPoint pointToInsert = + HTMLEditUtils::GetInsertionPointInInclusiveAncestor( + aTag, aStartOfDeepestRightNode, &aEditingHost); + if (MOZ_UNLIKELY(!pointToInsert.IsSet())) { + NS_WARNING( + "HTMLEditor::MaybeSplitAncestorsForInsertWithTransaction() reached " + "editing host"); + return Err(NS_ERROR_FAILURE); } - - // If we got lost the editing host, we can do nothing. - if (NS_WARN_IF(!pointToInsert.IsSet())) { - return Err(NS_ERROR_EDITOR_UNEXPECTED_DOM_TREE); - } - // If the point itself can contain the tag, we don't need to split any // ancestor nodes. In this case, we should return the given split point // as is. diff --git a/editor/libeditor/HTMLEditUtils.h b/editor/libeditor/HTMLEditUtils.h index e07fa7909506..9b89e536a6b2 100644 --- a/editor/libeditor/HTMLEditUtils.h +++ b/editor/libeditor/HTMLEditUtils.h @@ -287,6 +287,36 @@ class HTMLEditUtils final { return false; } + /** + * Return a point which can insert a node whose name is aTagName scanning + * from aPoint to its ancestor points. + */ + template + static EditorDOMPoint GetInsertionPointInInclusiveAncestor( + nsAtom& aTagName, const EditorDOMPointType& aPoint, + const Element* aAncestorLimit = nullptr) { + if (MOZ_UNLIKELY(!aPoint.IsInContentNode())) { + return EditorDOMPoint(); + } + Element* lastChild = nullptr; + for (Element* containerElement : + aPoint.template ContainerAs() + ->template InclusiveAncestorsOfType()) { + if (!HTMLEditUtils::IsSimplyEditableNode(*containerElement)) { + return EditorDOMPoint(); + } + if (HTMLEditUtils::CanNodeContain(*containerElement, aTagName)) { + return lastChild ? EditorDOMPoint(lastChild) + : aPoint.template To(); + } + if (containerElement == aAncestorLimit) { + return EditorDOMPoint(); + } + lastChild = containerElement; + } + return EditorDOMPoint(); + } + /** * IsContainerNode() returns true if aContent is a container node. */ diff --git a/testing/web-platform/tests/editing/crashtests/indent-in-inline-editing-host-outside-body.html b/testing/web-platform/tests/editing/crashtests/indent-in-inline-editing-host-outside-body.html new file mode 100644 index 000000000000..d56a0fd248b9 --- /dev/null +++ b/testing/web-platform/tests/editing/crashtests/indent-in-inline-editing-host-outside-body.html @@ -0,0 +1,18 @@ + + + + + + + + +