From e6a6045ffb65077253a3237c94af7ccd11d89879 Mon Sep 17 00:00:00 2001 From: Sean Feng Date: Mon, 12 May 2025 20:40:53 +0000 Subject: [PATCH] Bug 1965664 - Fix some crashes in ContentSubtreeIterator r=jjaschke,dom-core by skipping the shadow hosts that ShadowDOM selection doesn't support at the moment. Differential Revision: https://phabricator.services.mozilla.com/D248917 --- dom/base/ContentIterator.cpp | 24 +++++++++++++++++++++--- dom/base/RangeUtils.cpp | 4 +++- dom/base/crashtests/1965664.html | 10 ++++++++++ dom/base/crashtests/crashtests.list | 1 + 4 files changed, 35 insertions(+), 4 deletions(-) create mode 100644 dom/base/crashtests/1965664.html diff --git a/dom/base/ContentIterator.cpp b/dom/base/ContentIterator.cpp index 5b6fa17bd2d7..b22ba7c31fac 100644 --- a/dom/base/ContentIterator.cpp +++ b/dom/base/ContentIterator.cpp @@ -664,6 +664,13 @@ nsIContent* ContentIteratorBase::GetNextSibling( aAllowCrossShadowBoundary == AllowRangeCrossShadowBoundary::Yes) { // Could have nested slots while (HTMLSlotElement* slot = aNode->AsContent()->GetAssignedSlot()) { + if (!ShadowDOMSelectionHelpers::GetShadowRoot( + slot->GetContainingShadowHost(), aAllowCrossShadowBoundary)) { + // The corresponding shadow host isn't supported + // by ContentSubtreeIterator, so let's skip this slot. + break; + } + // Next sibling of a slotted node should be the next slotted node auto currentIndex = slot->AssignedNodes().IndexOf(aNode); if (currentIndex < slot->AssignedNodes().Length() - 1) { @@ -726,6 +733,12 @@ nsIContent* ContentIteratorBase::GetPrevSibling( aAllowCrossShadowBoundary == AllowRangeCrossShadowBoundary::Yes) { // Could have nested slots. while (HTMLSlotElement* slot = aNode->AsContent()->GetAssignedSlot()) { + if (!ShadowDOMSelectionHelpers::GetShadowRoot( + slot->GetContainingShadowHost(), aAllowCrossShadowBoundary)) { + // The corresponding shadow host isn't supported + // by ContentSubtreeIterator, so let's skip this slot. + break; + } // prev sibling of a slotted node should be the prev slotted node auto currentIndex = slot->AssignedNodes().IndexOf(aNode); if (currentIndex > 0) { @@ -1026,15 +1039,19 @@ void ContentSubtreeIterator::CacheInclusiveAncestorsOfEndContainer() { break; } - const bool isDescendantInShadowTree = - IterAllowCrossShadowBoundary() && child->IsShadowRoot(); + // `ShadowDOMSelectionHelpers::GetShadowRoot` would return non-null shadow + // root if parent is a shadow host that we support cross boundary selection. + const bool isChildAShadowRootForSelection = + ShadowDOMSelectionHelpers::GetShadowRoot( + parent, mAllowCrossShadowBoundary) == child; info.mAncestor = parent->AsContent(); // mIsDescendantInShadowTree indicates that whether child is in the // shadow tree of parent or in the regular light DOM tree of parent. // So that later, when info.mAncestor is reached, we can decide whether // we should dive into the shadow tree. - info.mIsDescendantInShadowTree = isDescendantInShadowTree; + info.mIsDescendantInShadowTree = + IterAllowCrossShadowBoundary() && isChildAShadowRootForSelection; } } @@ -1255,6 +1272,7 @@ void ContentSubtreeIterator::Next() { ShadowRoot* root = ShadowDOMSelectionHelpers::GetShadowRoot( nextNode, mAllowCrossShadowBoundary); if (mInclusiveAncestorsOfEndContainer[i].mIsDescendantInShadowTree) { + MOZ_ASSERT(root); nextNode = root->GetFirstChild(); } else if (auto* slot = HTMLSlotElement::FromNode(nextNode); slot && IterAllowCrossShadowBoundary()) { diff --git a/dom/base/RangeUtils.cpp b/dom/base/RangeUtils.cpp index d1d7702ede2c..6a681dec33f2 100644 --- a/dom/base/RangeUtils.cpp +++ b/dom/base/RangeUtils.cpp @@ -365,7 +365,9 @@ nsINode* ShadowDOMSelectionHelpers::GetParentNodeInSameSelection( if (StaticPrefs::dom_shadowdom_selection_across_boundary_enabled() && aAllowCrossShadowBoundary == AllowRangeCrossShadowBoundary::Yes) { if (aNode.IsContent()) { - if (HTMLSlotElement* slot = aNode.AsContent()->GetAssignedSlot()) { + if (HTMLSlotElement* slot = aNode.AsContent()->GetAssignedSlot(); + slot && GetShadowRoot(slot->GetContainingShadowHost(), + aAllowCrossShadowBoundary)) { return slot; } } diff --git a/dom/base/crashtests/1965664.html b/dom/base/crashtests/1965664.html new file mode 100644 index 000000000000..2d77011381c4 --- /dev/null +++ b/dom/base/crashtests/1965664.html @@ -0,0 +1,10 @@ + + +

+ diff --git a/dom/base/crashtests/crashtests.list b/dom/base/crashtests/crashtests.list index aa23c710ac89..d922df76aa4c 100644 --- a/dom/base/crashtests/crashtests.list +++ b/dom/base/crashtests/crashtests.list @@ -283,3 +283,4 @@ load 1897248.html load 1907464.html asserts(0-1) load 1907228.html # see bug 1908485 for assert load 1922153.html +load 1965664.html