Skip to content

Commit

Permalink
Bug 1891783 - Fix two more bugs in ShadowDOM Selection r=jjaschke,sma…
Browse files Browse the repository at this point in the history 
…ug,dom-core

Bug #1: AbstractRange::(Mark|Unmark)Descendants should always use
the shadow tree of web-exposed shadow root, instead of using
light DOM elements of the host.

Bug #2: aRange could possibly create mCrossShadowBoundaryRange
first (due to boundaries are in different tree), and later
moves the boundaries to the same tree. When this happens, we
should remove mCrossShadowBoundaryRange and use the default
range to represent it.

Differential Revision: https://phabricator.services.mozilla.com/D207608
  • Loading branch information
@sefeng211
sefeng211 committed May 13, 2024
1 parent 3984610 commit 6ca9b95
Show file tree
Hide file tree
Showing 7 changed files with 243 additions and 60 deletions.
26 changes: 13 additions & 13 deletions dom/base/AbstractRange.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
// for the flattened children of aNode.
void UpdateDescendantsInFlattenedTree(const nsIContent& aNode,
bool aMarkDesendants) {
if (!aNode.IsElement() || aNode.IsHTMLElement(nsGkAtoms::slot)) {
if (!aNode.IsElement()) {
return;
}

Expand All @@ -119,14 +119,14 @@ void AbstractRange::MarkDescendants(const nsINode& aNode) {
// ancestor or a descendant of one, in which case all of our descendants have
// the bit set already.
if (!aNode.IsMaybeSelected()) {
// don't set the Descendant bit on |aNode| itself
nsINode* node = aNode.GetNextNode(&aNode);
if (!node) {
if (aNode.GetShadowRootForSelection()) {
UpdateDescendantsInFlattenedTree(*aNode.AsContent(), true);
}
// If aNode has a web-exposed shadow root, use this shadow tree and ignore
// the children of aNode.
if (aNode.GetShadowRootForSelection()) {
UpdateDescendantsInFlattenedTree(*aNode.AsContent(), true);
return;
}
// don't set the Descendant bit on |aNode| itself
nsINode* node = aNode.GetNextNode(&aNode);
while (node) {
node->SetDescendantOfClosestCommonInclusiveAncestorForRangeInSelection();
if (!node->IsClosestCommonInclusiveAncestorForRangeInSelection()) {
Expand All @@ -152,14 +152,14 @@ void AbstractRange::UnmarkDescendants(const nsINode& aNode) {
// common ancestor itself).
if (!aNode
.IsDescendantOfClosestCommonInclusiveAncestorForRangeInSelection()) {
// we know |aNode| doesn't have any bit set
nsINode* node = aNode.GetNextNode(&aNode);
if (!node) {
if (aNode.GetShadowRootForSelection()) {
UpdateDescendantsInFlattenedTree(*aNode.AsContent(), false);
}
// If aNode has a web-exposed shadow root, use this shadow tree and ignore
// the children of aNode.
if (aNode.GetShadowRootForSelection()) {
UpdateDescendantsInFlattenedTree(*aNode.AsContent(), false);
return;
}
// we know |aNode| doesn't have any bit set
nsINode* node = aNode.GetNextNode(&aNode);
while (node) {
node->ClearDescendantOfClosestCommonInclusiveAncestorForRangeInSelection();
if (!node->IsClosestCommonInclusiveAncestorForRangeInSelection()) {
Expand Down
108 changes: 68 additions & 40 deletions dom/base/nsRange.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,19 +106,19 @@ template nsresult nsRange::SetStartAndEnd(
template void nsRange::DoSetRange(const RangeBoundary& aStartBoundary,
const RangeBoundary& aEndBoundary,
nsINode* aRootNode, bool aNotInsertedYet,
CollapsePolicy aCollapsePolicy);
RangeBehaviour aRangeBehaviour);
template void nsRange::DoSetRange(const RangeBoundary& aStartBoundary,
const RawRangeBoundary& aEndBoundary,
nsINode* aRootNode, bool aNotInsertedYet,
CollapsePolicy aCollapsePolicy);
RangeBehaviour aRangeBehaviour);
template void nsRange::DoSetRange(const RawRangeBoundary& aStartBoundary,
const RangeBoundary& aEndBoundary,
nsINode* aRootNode, bool aNotInsertedYet,
CollapsePolicy aCollapsePolicy);
RangeBehaviour aRangeBehaviour);
template void nsRange::DoSetRange(const RawRangeBoundary& aStartBoundary,
const RawRangeBoundary& aEndBoundary,
nsINode* aRootNode, bool aNotInsertedYet,
CollapsePolicy aCollapsePolicy);
RangeBehaviour aRangeBehaviour);

template void nsRange::CreateOrUpdateCrossShadowBoundaryRangeIfNeeded(
const RangeBoundary& aStartBoundary, const RangeBoundary& aEndBoundary);
Expand Down Expand Up @@ -222,40 +222,59 @@ already_AddRefed<nsRange> nsRange::Create(
* aRange: The nsRange that aNewBoundary is being set to.
* aNewRoot: The shadow-including root of the container of aNewBoundary
* aNewBoundary: The new boundary
* aIsSetStart: true if ShouldCollapseBoundary is called by nsRange::SetStart,
* aIsSetStart: true if GetRangeBehaviour is called by nsRange::SetStart,
* false otherwise
* aAllowCrossShadowBoundary: Indicates whether the boundaries allowed to cross
* shadow boundary or not
*/
static CollapsePolicy ShouldCollapseBoundary(
static RangeBehaviour GetRangeBehaviour(
const nsRange* aRange, const nsINode* aNewRoot,
const RawRangeBoundary& aNewBoundary, const bool aIsSetStart,
AllowRangeCrossShadowBoundary aAllowCrossShadowBoundary) {
if (!aRange->IsPositioned()) {
return CollapsePolicy::DefaultRangeAndCrossShadowBoundaryRanges;
return RangeBehaviour::CollapseDefaultRangeAndCrossShadowBoundaryRanges;
}

MOZ_ASSERT(aRange->GetRoot());

if (aNewRoot != aRange->GetRoot()) {
// Boundaries are in different document (or not connected), so collapse
// the both the default range and the crossBoundaryRange range.
if (aNewRoot->GetComposedDoc() != aRange->GetRoot()->GetComposedDoc()) {
return CollapsePolicy::DefaultRangeAndCrossShadowBoundaryRanges;
return RangeBehaviour::CollapseDefaultRangeAndCrossShadowBoundaryRanges;
}

// Always collapse both ranges if the one of the roots is an UA widget
// regardless whether the boundaries are allowed to cross shadow boundary
// or not.
if (AbstractRange::IsRootUAWidget(aNewRoot) ||
AbstractRange::IsRootUAWidget(aRange->GetRoot())) {
return CollapsePolicy::DefaultRangeAndCrossShadowBoundaryRanges;
return RangeBehaviour::CollapseDefaultRangeAndCrossShadowBoundaryRanges;
}

if (const CrossShadowBoundaryRange* crossShadowBoundaryRange =
aRange->GetCrossShadowBoundaryRange()) {
// Check if the existing-other-side boundary in
// aRange::mCrossShadowBoundaryRange has the same root
// as aNewRoot. If this is the case, it means default range
// is good enough to represent this range, so that we can
// merge the cross-shadow-boundary range and the default range.
const RangeBoundary& otherSideExistingBoundary =
aIsSetStart ? crossShadowBoundaryRange->EndRef()
: crossShadowBoundaryRange->StartRef();
const nsINode* otherSideRoot =
RangeUtils::ComputeRootNode(otherSideExistingBoundary.Container());
if (aNewRoot == otherSideRoot) {
return RangeBehaviour::MergeDefaultRangeAndCrossShadowBoundaryRanges;
}
}

// Different root, but same document. So we only collapse the
// default range if boundaries are allowed to cross shadow boundary.
return aAllowCrossShadowBoundary == AllowRangeCrossShadowBoundary::Yes
? CollapsePolicy::DefaultRange
: CollapsePolicy::DefaultRangeAndCrossShadowBoundaryRanges;
? RangeBehaviour::CollapseDefaultRange
: RangeBehaviour::
CollapseDefaultRangeAndCrossShadowBoundaryRanges;
}

const RangeBoundary& otherSideExistingBoundary =
Expand All @@ -281,12 +300,12 @@ static CollapsePolicy ShouldCollapseBoundary(
// aNewBoundary intends to be the end.
//
// So no collapse for above cases.
return CollapsePolicy::No;
return RangeBehaviour::KeepDefaultRangeAndCrossShadowBoundaryRanges;
}

if (!aRange->MayCrossShadowBoundary() ||
aAllowCrossShadowBoundary == AllowRangeCrossShadowBoundary::No) {
return CollapsePolicy::DefaultRangeAndCrossShadowBoundaryRanges;
return RangeBehaviour::CollapseDefaultRangeAndCrossShadowBoundaryRanges;
}

const RangeBoundary& otherSideExistingCrossShadowBoundaryBoundary =
Expand All @@ -308,15 +327,15 @@ static CollapsePolicy ShouldCollapseBoundary(

// Valid to the cross boundary boundary.
if (withCrossShadowBoundaryOrder && *withCrossShadowBoundaryOrder != 1) {
return CollapsePolicy::DefaultRange;
return RangeBehaviour::CollapseDefaultRange;
}

// Not valid to both existing boundaries.
return CollapsePolicy::DefaultRangeAndCrossShadowBoundaryRanges;
return RangeBehaviour::CollapseDefaultRangeAndCrossShadowBoundaryRanges;
}

MOZ_ASSERT_UNREACHABLE();
return CollapsePolicy::DefaultRangeAndCrossShadowBoundaryRanges;
return RangeBehaviour::CollapseDefaultRangeAndCrossShadowBoundaryRanges;
}
/******************************************************
* nsISupports
Expand Down Expand Up @@ -1041,12 +1060,11 @@ void nsRange::AssertIfMismatchRootAndRangeBoundaries(
// Calling DoSetRange with either parent argument null will collapse
// the range to have both endpoints point to the other node
template <typename SPT, typename SRT, typename EPT, typename ERT>
void nsRange::DoSetRange(
const RangeBoundaryBase<SPT, SRT>& aStartBoundary,
const RangeBoundaryBase<EPT, ERT>& aEndBoundary, nsINode* aRootNode,
bool aNotInsertedYet /* = false */,
CollapsePolicy
aCollapsePolicy /* = DEFAULT_RANGE_AND_CROSS_BOUNDARY_RANGES */) {
void nsRange::
DoSetRange(const RangeBoundaryBase<SPT, SRT>& aStartBoundary,
const RangeBoundaryBase<EPT, ERT>& aEndBoundary,
nsINode* aRootNode,
bool aNotInsertedYet /* = false */, RangeBehaviour aRangeBehaviour /* = CollapseDefaultRangeAndCrossShadowBoundaryRanges */) {
mIsPositioned = aStartBoundary.IsSetAndValid() &&
aEndBoundary.IsSetAndValid() && aRootNode;
MOZ_ASSERT_IF(!mIsPositioned, !aStartBoundary.IsSet());
Expand Down Expand Up @@ -1075,8 +1093,8 @@ void nsRange::DoSetRange(
mStart.CopyFrom(aStartBoundary, RangeBoundaryIsMutationObserved::Yes);
mEnd.CopyFrom(aEndBoundary, RangeBoundaryIsMutationObserved::Yes);

if (aCollapsePolicy ==
CollapsePolicy::DefaultRangeAndCrossShadowBoundaryRanges) {
if (aRangeBehaviour ==
RangeBehaviour::CollapseDefaultRangeAndCrossShadowBoundaryRanges) {
ResetCrossShadowBoundaryRange();
}

Expand Down Expand Up @@ -1158,12 +1176,12 @@ void nsRange::SetStart(
return;
}

CollapsePolicy policy =
ShouldCollapseBoundary(this, newRoot, aPoint, true /* aIsSetStart= */,
aAllowCrossShadowBoundary);
RangeBehaviour behaviour =
GetRangeBehaviour(this, newRoot, aPoint, true /* aIsSetStart= */,
aAllowCrossShadowBoundary);

switch (policy) {
case CollapsePolicy::No:
switch (behaviour) {
case RangeBehaviour::KeepDefaultRangeAndCrossShadowBoundaryRanges:
// EndRef(..) may be same as mStart or not, depends on
// the value of mCrossShadowBoundaryRange->mEnd, We need to update
// mCrossShadowBoundaryRange and the default boundaries separately
Expand All @@ -1176,17 +1194,22 @@ void nsRange::SetStart(
ResetCrossShadowBoundaryRange();
}
}
DoSetRange(aPoint, mEnd, mRoot, false, policy);
DoSetRange(aPoint, mEnd, mRoot, false, behaviour);
break;
case CollapsePolicy::DefaultRangeAndCrossShadowBoundaryRanges:
DoSetRange(aPoint, aPoint, newRoot, false, policy);
case RangeBehaviour::CollapseDefaultRangeAndCrossShadowBoundaryRanges:
DoSetRange(aPoint, aPoint, newRoot, false, behaviour);
break;
case CollapsePolicy::DefaultRange:
case RangeBehaviour::CollapseDefaultRange:
MOZ_ASSERT(aAllowCrossShadowBoundary ==
AllowRangeCrossShadowBoundary::Yes);
CreateOrUpdateCrossShadowBoundaryRangeIfNeeded(
aPoint, MayCrossShadowBoundaryEndRef());
DoSetRange(aPoint, aPoint, newRoot, false, policy);
DoSetRange(aPoint, aPoint, newRoot, false, behaviour);
break;
case RangeBehaviour::MergeDefaultRangeAndCrossShadowBoundaryRanges:
DoSetRange(aPoint, MayCrossShadowBoundaryEndRef(), newRoot, false,
behaviour);
ResetCrossShadowBoundaryRange();
break;
default:
MOZ_ASSERT_UNREACHABLE();
Expand Down Expand Up @@ -1270,12 +1293,12 @@ void nsRange::SetEnd(const RawRangeBoundary& aPoint, ErrorResult& aRv,
return;
}

CollapsePolicy policy =
ShouldCollapseBoundary(this, newRoot, aPoint, false /* aIsStartStart */,
aAllowCrossShadowBoundary);
RangeBehaviour policy =
GetRangeBehaviour(this, newRoot, aPoint, false /* aIsStartStart */,
aAllowCrossShadowBoundary);

switch (policy) {
case CollapsePolicy::No:
case RangeBehaviour::KeepDefaultRangeAndCrossShadowBoundaryRanges:
// StartRef(..) may be same as mStart or not, depends on
// the value of mCrossShadowBoundaryRange->mStart, so we need to update
// mCrossShadowBoundaryRange and the default boundaries separately
Expand All @@ -1290,16 +1313,21 @@ void nsRange::SetEnd(const RawRangeBoundary& aPoint, ErrorResult& aRv,
}
DoSetRange(mStart, aPoint, mRoot, false, policy);
break;
case CollapsePolicy::DefaultRangeAndCrossShadowBoundaryRanges:
case RangeBehaviour::CollapseDefaultRangeAndCrossShadowBoundaryRanges:
DoSetRange(aPoint, aPoint, newRoot, false, policy);
break;
case CollapsePolicy::DefaultRange:
case RangeBehaviour::CollapseDefaultRange:
MOZ_ASSERT(aAllowCrossShadowBoundary ==
AllowRangeCrossShadowBoundary::Yes);
CreateOrUpdateCrossShadowBoundaryRangeIfNeeded(
MayCrossShadowBoundaryStartRef(), aPoint);
DoSetRange(aPoint, aPoint, newRoot, false, policy);
break;
case RangeBehaviour::MergeDefaultRangeAndCrossShadowBoundaryRanges:
DoSetRange(MayCrossShadowBoundaryStartRef(), aPoint, newRoot, false,
policy);
ResetCrossShadowBoundaryRange();
break;
default:
MOZ_ASSERT_UNREACHABLE();
}
Expand Down
22 changes: 15 additions & 7 deletions dom/base/nsRange.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,19 @@ class DOMRectList;
class InspectorFontFace;
class Selection;

enum class CollapsePolicy : uint8_t {
No, // Don't need to collapse
DefaultRange, // Collapse the default range
DefaultRangeAndCrossShadowBoundaryRanges // Collapse both the default range
// and the cross boundary range
enum class RangeBehaviour : uint8_t {
// Keep both ranges
KeepDefaultRangeAndCrossShadowBoundaryRanges,
// Merge both ranges; This is the case where the range boundaries was in
// different roots initially, and becoming in the same roots now. Since
// they start to be in the same root, using normal range is good enough
// to represent it
MergeDefaultRangeAndCrossShadowBoundaryRanges,
// Collapse the default range
CollapseDefaultRange,
// Collapse both the default range and the cross-shadow-boundary range
CollapseDefaultRangeAndCrossShadowBoundaryRanges

};
} // namespace dom
} // namespace mozilla
Expand Down Expand Up @@ -491,8 +499,8 @@ class nsRange final : public mozilla::dom::AbstractRange,
const mozilla::RangeBoundaryBase<SPT, SRT>& aStartBoundary,
const mozilla::RangeBoundaryBase<EPT, ERT>& aEndBoundary,
nsINode* aRootNode, bool aNotInsertedYet = false,
mozilla::dom::CollapsePolicy aCollapsePolicy = mozilla::dom::
CollapsePolicy::DefaultRangeAndCrossShadowBoundaryRanges);
mozilla::dom::RangeBehaviour aRangeBehaviour = mozilla::dom::
RangeBehaviour::CollapseDefaultRangeAndCrossShadowBoundaryRanges);

// Assume that this is guaranteed that this is held by the caller when
// this is used. (Note that we cannot use AutoRestore for mCalledByJS
Expand Down
6 changes: 6 additions & 0 deletions layout/generic/test/mochitest.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,12 @@ skip-if = ["release_or_beta"] # requires Selection.getComposedRanges to be enabl
["test_selection_cross_shadow_boundary_multi_ranges_backward_click.html"]
skip-if = ["release_or_beta"] # requires Selection.getComposedRanges to be enabled (Nightly only)

["test_selection_cross_shadow_boundary_forward_and_backward.html"]
skip-if = ["release_or_beta"] # requires Selection.getComposedRanges to be enabled (Nightly only)

["test_selection_cross_shadow_boundary_backward_nested_click.html"]
skip-if = ["release_or_beta"] # requires Selection.getComposedRanges to be enabled (Nightly only)

["test_selection_doubleclick.html"]

["test_selection_expanding.html"]
Expand Down
49 changes: 49 additions & 0 deletions layout/generic/test/test_selection_cross_shadow_boundary_backward_nested_click.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
<!DOCTYPE HTML>
<script src="/tests/SimpleTest/EventUtils.js"></script>
<script src="/tests/SimpleTest/SimpleTest.js"></script>
<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
<script>
SimpleTest.waitForExplicitFinish();
function run() {
const host = document.getElementById("host");
const inner = host.shadowRoot.getElementById("inner");
const innerRect = inner.getBoundingClientRect();

const innerHost = host.shadowRoot.getElementById("innerHost");
const nested = innerHost.shadowRoot.getElementById("nested");
const nestedRect = nested.getBoundingClientRect();

// Click the center of "NestedText"
synthesizeMouse(nested, nestedRect.width / 2, nestedRect.height / 2, { type: "mousedown" });
synthesizeMouse(nested, nestedRect.width / 2, nestedRect.height / 2, { type: "mouseup" });

// Click the center of "InnerText"
synthesizeMouse(inner, innerRect.width / 2, innerRect.height / 2, { type: "mousedown", shiftKey: true});
synthesizeMouse(inner, innerRect.width / 2, innerRect.height / 2, { type: "mouseup" , shiftKey: true});

// Above two clicks should select half of the content in "InnerText" and half of the content in "NestedText"
let sel = document.getSelection().getComposedRanges(host.shadowRoot, innerHost.shadowRoot)[0];

// forward selection
is(sel.startContainer, inner.firstChild, "startContainer is the InnerText");
is(sel.endContainer, nested.firstChild, "endContainer is the NestedText");

const collapsedRange = document.getSelection().getRangeAt(0);
is(collapsedRange.startContainer, inner.firstChild, "normal range's startContainer get collapsed to InnerText");
is(collapsedRange.endContainer, inner.firstChild, "normal range's endContainer get collapsed to InnerText");

SimpleTest.finish();
}
</script>
<body onload="SimpleTest.waitForFocus(run);">
<div id="host">
<template shadowrootmode="open">
<span id="inner">InnerText</span>
<div id="innerHost">
<template shadowrootmode="open">
<span id="nested">NestedText</span>
</template>
</div>
</template>
</div>
</body>
Loading

0 comments on commit 6ca9b95

Please sign in to comment.