Skip to content

Commit

Permalink
Make ShadowRoot notify mutation observers of "parent chain changed"
Browse files Browse the repository at this point in the history
Currently, `ShadowRoot::Unbind` does not notify the observers of the
notification.

With this change, `nsRange` starts removing itself from all `Selection`s if its
root becomes disconnected.  This behavior is different from Chrome, Chrome
allows the disconnected range in shadow as a selection range.  However, it
does not match with the behavior tested in
`move-selection-range-into-different-root.tentative.html` [1][2].  Therefore, I
think that removing the range from `Selection` is reasonable for now because
it's not reasonable to add a check to all selection range getters/users to check
whether the range is in the composed tree.

1. https://searchfox.org/mozilla-central/rev/b60cb73160843adb5a5a3ec8058e75a69b46acf7/testing/web-platform/tests/selection/move-selection-range-into-different-root.tentative.html
2. https://wpt.fyi/results/selection/move-selection-range-into-different-root.tentative.html?run_id=5070898326929408&run_id=5147803004698624&run_id=5098392828510208&run_id=5175063246012416

Differential Revision: https://phabricator.services.mozilla.com/D204218

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1883802
gecko-commit: dda1e753390ebd7e848058e896da8b74bb105068
gecko-reviewers: smaug
  • Loading branch information
masayuki-nakano authored and moz-wptsync-bot committed Mar 12, 2024
1 parent 92e4dc9 commit 757498e
Show file tree
Hide file tree
Showing 5 changed files with 297 additions and 0 deletions.
47 changes: 47 additions & 0 deletions dom/ranges/Range-in-shadow-after-the-shadow-removed.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<!doctype html>
<html>
<head>
<meta charset="utf-8">
<meta name="variant" content="?mode=closed">
<meta name="variant" content="?mode=open">
<title>Range in shadow after removing the shadow</title>
<script src=/resources/testharness.js></script>
<script src=/resources/testharnessreport.js></script>
<script>
"use strict";

addEventListener("load", () => {
const mode = (new URLSearchParams(document.location.search)).get("mode");
test(() => {
const host = document.createElement("div");
host.id = "host";
const root = host.attachShadow({mode});
root.innerHTML = '<div id="in-shadow">ABC</div>';
document.body.appendChild(host);
const range = document.createRange();
range.setStart(root.firstChild, 1);
host.remove();
assert_equals(range.startContainer, root.firstChild, "startContainer should not be changed");
assert_equals(range.startOffset, 1, "startOffset should not be changed");
}, "Range in shadow should stay in the shadow after the host is removed");

test(() => {
const wrapper = document.createElement("div");
wrapper.id = "wrapper";
const host = document.createElement("div");
host.id = "host";
const root = host.attachShadow({mode});
root.innerHTML = '<div id="in-shadow">ABC</div>';
wrapper.appendChild(host);
document.body.appendChild(wrapper);
const range = document.createRange();
range.setStart(root.firstChild, 1);
wrapper.remove();
assert_equals(range.startContainer, root.firstChild, "startContainer should not be changed");
assert_equals(range.startOffset, 1, "startOffset should not be changed");
}, "Range in shadow should stay in the shadow after the host parent is removed");
}, {once: true});
</script>
</head>
<body></body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<!doctype html>
<html class="reftest-wait">
<head>
<meta charset="utf-8">
<script>
document.addEventListener("DOMContentLoaded", () => {
const shadowRoot = b.attachShadow({mode: "closed"});
shadowRoot.textContent = "A";
getSelection().collapse(shadowRoot.firstChild, 1);

function moveCaretAndReplaceBodyWithAddress() {
getSelection().modify("move", "forward", "lineboundary");
document.documentElement.replaceChild(a, document.body);
}

requestAnimationFrame(() => {
requestAnimationFrame(moveCaretAndReplaceBodyWithAddress);
requestAnimationFrame(moveCaretAndReplaceBodyWithAddress);
requestAnimationFrame(
() => document.documentElement.removeAttribute("class")
);
});
}, {once: true});
</script>
</head>
<body>
<address id="a"></address>
<div id="b"></div>
</body>
</html>
69 changes: 69 additions & 0 deletions selection/selection-range-after-editinghost-removed.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
<!doctype html>
<html>
<head>
<meta charset="utf-8">
<title>Selection range in an editing host after the host is removed</title>
<script src=/resources/testharness.js></script>
<script src=/resources/testharnessreport.js></script>
<script>
"use strict";

addEventListener("load", () => {
const container = document.querySelector("div");
test(() => {
const editingHost = document.createElement("div");
editingHost.contentEditable = true;
editingHost.innerHTML = "ABC<br>";
container.appendChild(editingHost);
editingHost.focus();
getSelection().collapse(editingHost, 0);
editingHost.remove();
assert_equals(getSelection().focusNode, container, "focusNode should be the container");
assert_equals(getSelection().focusOffset, 0, "focusOffset should be 0");
}, "Selection range in an editing host should be collapsed where the host was after it's removed");

test(() => {
const wrapper = document.createElement("div");
wrapper.id = "wrapper";
const editingHost = document.createElement("div");
editingHost.contentEditable = true;
editingHost.innerHTML = "ABC<br>";
wrapper.appendChild(editingHost);
container.appendChild(wrapper);
editingHost.focus();
getSelection().collapse(editingHost, 0);
wrapper.remove();
assert_equals(getSelection().focusNode, container, "focusNode should be the container");
assert_equals(getSelection().focusOffset, 0, "focusOffset should be 0");
}, "Selection range in an editing host should be collapsed where the host was after its parent is removed");

test(() => {
const editingHost = document.createElement("div");
editingHost.contentEditable = true;
editingHost.innerHTML = "ABC<br>";
container.appendChild(editingHost);
editingHost.focus();
getSelection().collapse(editingHost, 0);
editingHost.replaceWith(editingHost);
assert_equals(getSelection().focusNode, container, "focusNode should be the container");
assert_equals(getSelection().focusOffset, 0, "focusOffset should be 0");
editingHost.remove();
}, "Selection range in an editing host should be collapsed where the host was after it's replaced with itself (.replaceWith)");

test(() => {
const editingHost = document.createElement("div");
editingHost.contentEditable = true;
editingHost.innerHTML = "ABC<br>";
container.appendChild(editingHost);
editingHost.focus();
getSelection().collapse(editingHost, 0);
container.replaceChild(editingHost, editingHost);
assert_equals(getSelection().focusNode, container, "focusNode should be the container");
assert_equals(getSelection().focusOffset, 0, "focusOffset should be 0");
editingHost.remove();
}, "Selection range in an editing host should be collapsed where the host was after it's replaced with itself (.replaceChild)");
}, {once: true});
</script>
</head>
<body><div id="container"></div></body>
</html>
55 changes: 55 additions & 0 deletions selection/selection-range-after-textcontrol-removed.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
<!doctype html>
<html>
<head>
<meta charset="utf-8">
<meta name="variant" content="?textControl=text">
<meta name="variant" content="?textControl=password">
<meta name="variant" content="?textControl=number">
<meta name="variant" content="?textControl=textarea">
<title>Selection range after text control is removed</title>
<script src=/resources/testharness.js></script>
<script src=/resources/testharnessreport.js></script>
<script>
"use strict";

/**
* Browsers may use internal Selection and/or Range to implement the selection
* in text controls. They should not be appear outside the host text control
* after the host is removed.
*/
addEventListener("load", () => {
const textControlType = (new URLSearchParams(document.location.search)).get("mode");
function createTextControlElement() {
const textControl =
document.createElement(textControlType == "textarea" ? "textarea" : "input");
if (textControlType != "textarea") {
textControl.type = textControlType;
}
return textControl;
}

test(() => {
const textControl = createTextControlElement();
document.body.appendChild(textControl);
textControl.select();
getSelection().removeAllRanges();
textControl.remove();
assert_equals(getSelection().rangeCount, 0);
}, "Removing the text control should not add selection range");

test(() => {
const wrapper = document.createElement("div");
wrapper.id = "wrapper";
const textControl = createTextControlElement();
wrapper.appendChild(textControl);
document.body.appendChild(wrapper);
textControl.select();
getSelection().removeAllRanges();
wrapper.remove();
assert_equals(getSelection().rangeCount, 0);
}, "Removing the text control parent should not add selection range");
}, {once: true});
</script>
</head>
<body></body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
<!doctype html>
<html>
<head>
<meta charset="utf-8">
<meta name="variant" content="?mode=closed">
<meta name="variant" content="?mode=open">
<title>Selection range in shadow after removing the shadow</title>
<script src=/resources/testharness.js></script>
<script src=/resources/testharnessreport.js></script>
<script>
"use strict";

addEventListener("load", () => {
const mode = (new URLSearchParams(document.location.search)).get("mode");
const container = document.querySelector("div");
test(() => {
const host = document.createElement("div");
host.id = "host";
const root = host.attachShadow({mode});
root.innerHTML = '<div id="in-shadow">ABC</div>';
container.appendChild(host);
getSelection().collapse(root.firstChild.firstChild, 2);
const range = getSelection().getRangeAt(0);
host.remove();
// The range should be kept in the shadow because it's referred by JS as
// a range.
assert_equals(range.startContainer, root.firstChild.firstChild, "startContainer should not be changed");
assert_equals(range.startOffset, 2, "startOffset should not be changed");
// It may be reasonable to just remove the range in removed shadow.
// This matches with move-selection-range-into-different-root.tentative.html
assert_equals(getSelection().rangeCount, 0, "Selection should not have the range in removed shadow");
}, "Selection range in shadow should not be as a selection range after the host is removed");

test(() => {
const wrapper = document.createElement("div");
wrapper.id = "wrapper";
const host = document.createElement("div");
host.id = "host";
const root = host.attachShadow({mode});
root.innerHTML = '<div id="in-shadow">ABC</div>';
wrapper.appendChild(host);
container.appendChild(wrapper);
getSelection().collapse(root.firstChild.firstChild, 2);
const range = getSelection().getRangeAt(0);
wrapper.remove();
// The range should be kept in the shadow because it's referred by JS as
// a range.
assert_equals(range.startContainer, root.firstChild.firstChild, "startContainer should not be changed");
assert_equals(range.startOffset, 2, "startOffset should not be changed");
// It may be reasonable to just remove the range in removed shadow.
// This matches with move-selection-range-into-different-root.tentative.html
assert_equals(getSelection().rangeCount, 0, "Selection should not have the range in removed shadow");
}, "Selection range in shadow should not be as a selection range after the host parent is removed");

test(() => {
const host = document.createElement("div");
host.id = "host";
const root = host.attachShadow({mode});
root.innerHTML = '<div id="in-shadow">ABC</div>';
container.appendChild(host);
getSelection().collapse(root.firstChild.firstChild, 2);
const range = getSelection().getRangeAt(0);
host.replaceWith(host);
// The range should be kept in the shadow because it's referred by JS as
// a range.
assert_equals(range.startContainer, root.firstChild.firstChild, "startContainer should not be changed");
assert_equals(range.startOffset, 2, "startOffset should not be changed");
// It may be reasonable to just remove the range in removed shadow.
// This matches with move-selection-range-into-different-root.tentative.html
assert_equals(getSelection().rangeCount, 0, "Selection should not have the range in removed shadow");
host.remove();
}, "Selection range in shadow should not be as a selection range after the host is replaced with itself (.replaceWith)");

test(() => {
const host = document.createElement("div");
host.id = "host";
const root = host.attachShadow({mode});
root.innerHTML = '<div id="in-shadow">ABC</div>';
container.appendChild(host);
getSelection().collapse(root.firstChild.firstChild, 2);
const range = getSelection().getRangeAt(0);
container.replaceChild(host, host);
// The range should be kept in the shadow because it's referred by JS as
// a range.
assert_equals(range.startContainer, root.firstChild.firstChild, "startContainer should not be changed");
assert_equals(range.startOffset, 2, "startOffset should not be changed");
// It may be reasonable to just remove the range in removed shadow.
// This matches with move-selection-range-into-different-root.tentative.html
assert_equals(getSelection().rangeCount, 0, "Selection should not have the range in removed shadow");
host.remove();
}, "Selection range in shadow should not be as a selection range after the host is replaced with itself (.replaceChild");
}, {once: true});
</script>
</head>
<body><div id="container"></div></body>
</html>

0 comments on commit 757498e

Please sign in to comment.