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 `<blockquote>` has already been moved to outside
of the editing host.
2. There is no container to insert new `<blockquote>`, 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 `<blockquote>` 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
This commit is contained in:
Masayuki Nakano 2023-06-15 08:15:38 +00:00
parent 21a2f56033
commit 9a4701f405
3 changed files with 76 additions and 26 deletions

View File

@ -5350,6 +5350,7 @@ nsresult HTMLEditor::HandleHTMLIndentAroundRanges(AutoRangeArray& aRanges,
}
}
// FIXME: Split ancestors when we consider to indent the range.
Result<EditorDOMPoint, nsresult> 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 <blockquote>, 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<CreateElementResult, nsresult> 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<Element> 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.

View File

@ -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 <typename EditorDOMPointType>
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<nsIContent>()
->template InclusiveAncestorsOfType<Element>()) {
if (!HTMLEditUtils::IsSimplyEditableNode(*containerElement)) {
return EditorDOMPoint();
}
if (HTMLEditUtils::CanNodeContain(*containerElement, aTagName)) {
return lastChild ? EditorDOMPoint(lastChild)
: aPoint.template To<EditorDOMPoint>();
}
if (containerElement == aAncestorLimit) {
return EditorDOMPoint();
}
lastChild = containerElement;
}
return EditorDOMPoint();
}
/**
* IsContainerNode() returns true if aContent is a container node.
*/

View File

@ -0,0 +1,18 @@
<!doctype html>
<html>
<head>
<meta charset="utf-8">
<script>
addEventListener("load", () => {
const samp = document.createElement("samp");
samp.innerText = "ABC";
samp.contentEditable = true;
document.documentElement.appendChild(samp);
getSelection().selectAllChildren(samp);
document.execCommand("indent");
});
</script>
</head>
<body>
</body>
</html>