Skip to content

Commit

Permalink
Bug 1359397 - Don't allow Selection in nodes not in the document; r=m…
Browse files Browse the repository at this point in the history
…asayuki

This matches the spec and Chrome, and seems to bring us closer to Edge
and WebKit as well.  It also matches our own behavior for addRange(),
which was changed in bug 1341137.

For collapse and selectAllChildren, we match the tests and browsers, but
the spec is incorrect at the time of this writing:
w3c/selection-api#86

The removeAllRanges test hadn't been updated for the spec change.

MozReview-Commit-ID: DTK8283k5IP
  • Loading branch information
ayg committed Aug 10, 2017
1 parent aca8f24 commit 54c6faf
Show file tree
Hide file tree
Showing 30 changed files with 54 additions and 104,735 deletions.
44 changes: 44 additions & 0 deletions dom/base/Selection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2481,6 +2481,21 @@ Selection::Collapse(nsINode& aContainer, uint32_t aOffset, ErrorResult& aRv)
return;
}

if (aContainer.NodeType() == nsIDOMNode::DOCUMENT_TYPE_NODE) {
aRv.Throw(NS_ERROR_DOM_INVALID_NODE_TYPE_ERR);
return;
}

if (aOffset > aContainer.Length()) {
aRv.Throw(NS_ERROR_DOM_INDEX_SIZE_ERR);
return;
}

if (!HasSameRoot(aContainer)) {
// Return with no error
return;
}

nsCOMPtr<nsINode> container = &aContainer;

RefPtr<nsFrameSelection> frameSelection = mFrameSelection;
Expand Down Expand Up @@ -2880,6 +2895,11 @@ Selection::Extend(nsINode& aContainer, uint32_t aOffset, ErrorResult& aRv)
return;
}

if (!HasSameRoot(aContainer)) {
// Return with no error
return;
}

nsresult res;
if (!IsValidSelectionPoint(mFrameSelection, &aContainer)) {
aRv.Throw(NS_ERROR_FAILURE);
Expand Down Expand Up @@ -3169,6 +3189,16 @@ Selection::SelectAllChildrenJS(nsINode& aNode, ErrorResult& aRv)
void
Selection::SelectAllChildren(nsINode& aNode, ErrorResult& aRv)
{
if (aNode.NodeType() == nsIDOMNode::DOCUMENT_TYPE_NODE) {
aRv.Throw(NS_ERROR_DOM_INVALID_NODE_TYPE_ERR);
return;
}

if (!HasSameRoot(aNode)) {
// Return with no error
return;
}

if (mFrameSelection) {
mFrameSelection->PostReason(nsISelectionListener::SELECTALL_REASON);
}
Expand Down Expand Up @@ -3971,6 +4001,12 @@ Selection::SetBaseAndExtent(nsINode& aAnchorNode, uint32_t aAnchorOffset,
return;
}

if (!HasSameRoot(aAnchorNode) ||
!HasSameRoot(aFocusNode)) {
// Return with no error
return;
}

SelectionBatcher batch(this);

int32_t relativePosition =
Expand Down Expand Up @@ -4211,3 +4247,11 @@ AutoHideSelectionChanges::AutoHideSelectionChanges(const nsFrameSelection* aFram
: AutoHideSelectionChanges(
aFrame ? aFrame->GetSelection(SelectionType::eNormal) : nullptr)
{}

bool
Selection::HasSameRoot(nsINode& aNode)
{
nsINode* root = aNode.SubtreeRoot();
nsIDocument* doc = GetParentObject();
return doc == root || (root && doc == root->GetComposedDoc());
}
3 changes: 3 additions & 0 deletions dom/base/Selection.h
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,9 @@ class Selection final : public nsISelectionPrivate,
// Note: DoAutoScroll might destroy arbitrary frames etc.
nsresult DoAutoScroll(nsIFrame *aFrame, nsPoint& aPoint);

// We are not allowed to be in nodes whose root is not our document
bool HasSameRoot(nsINode& aNode);

// XXX Please don't add additional uses of this method, it's only for
// XXX supporting broken code (bug 1245883) in the following classes:
friend class ::nsCopySupport;
Expand Down
2 changes: 0 additions & 2 deletions editor/libeditor/tests/test_bug537046.html
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@

/** Test for Bug 537046 **/

SimpleTest.expectAssertions(1);

SimpleTest.waitForExplicitFinish();
addLoadEvent(function() {
var ed = document.getElementById("editor");
Expand Down
1,956 changes: 0 additions & 1,956 deletions testing/web-platform/meta/selection/addRange-00.html.ini

Large diffs are not rendered by default.

1,956 changes: 0 additions & 1,956 deletions testing/web-platform/meta/selection/addRange-04.html.ini

Large diffs are not rendered by default.

5,282 changes: 0 additions & 5,282 deletions testing/web-platform/meta/selection/addRange-08.html.ini

This file was deleted.

3,618 changes: 0 additions & 3,618 deletions testing/web-platform/meta/selection/addRange-12.html.ini

Large diffs are not rendered by default.

2,787 changes: 0 additions & 2,787 deletions testing/web-platform/meta/selection/addRange-16.html.ini

Large diffs are not rendered by default.

3,618 changes: 0 additions & 3,618 deletions testing/web-platform/meta/selection/addRange-20.html.ini

Large diffs are not rendered by default.

3,618 changes: 0 additions & 3,618 deletions testing/web-platform/meta/selection/addRange-24.html.ini

Large diffs are not rendered by default.

1,956 changes: 0 additions & 1,956 deletions testing/web-platform/meta/selection/addRange-28.html.ini

Large diffs are not rendered by default.

2,787 changes: 0 additions & 2,787 deletions testing/web-platform/meta/selection/addRange-32.html.ini

Large diffs are not rendered by default.

1,956 changes: 0 additions & 1,956 deletions testing/web-platform/meta/selection/addRange-36.html.ini

Large diffs are not rendered by default.

5,282 changes: 0 additions & 5,282 deletions testing/web-platform/meta/selection/addRange-40.html.ini

This file was deleted.

5,282 changes: 0 additions & 5,282 deletions testing/web-platform/meta/selection/addRange-44.html.ini

This file was deleted.

5,282 changes: 0 additions & 5,282 deletions testing/web-platform/meta/selection/addRange-48.html.ini

This file was deleted.

5,282 changes: 0 additions & 5,282 deletions testing/web-platform/meta/selection/addRange-52.html.ini

This file was deleted.

2,642 changes: 0 additions & 2,642 deletions testing/web-platform/meta/selection/addRange-56.html.ini

This file was deleted.

Loading

0 comments on commit 54c6faf

Please sign in to comment.