mirror of
https://github.com/servo/servo.git
synced 2025-07-29 18:20:24 +01:00
Don't drain ranges across shadow boundaries (#37281)
The [live range pre remove steps](https://dom.spec.whatwg.org/#live-range-pre-remove-steps) state that: > For each [live range](https://dom.spec.whatwg.org/#concept-live-range) whose [start node](https://dom.spec.whatwg.org/#concept-range-start-node) is an [inclusive descendant](https://dom.spec.whatwg.org/#concept-tree-inclusive-descendant) of node, set its [start](https://dom.spec.whatwg.org/#concept-range-start) to (parent, index). Elements in a shadow tree are not inclusive descendants of their hosts - meaning we should not bubble ranges across shadow boundaries. Includes a small fix to `Node::ranges_is_empty` which makes servo do less work on DOM mutations, as well as some changes to `range.rs` to match the spec better. I made these changes during debugging and they don't feel like they're worth their own PR. Testing: Covered by WPT --------- Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
This commit is contained in:
parent
836316c844
commit
430f65584d
3 changed files with 80 additions and 55 deletions
|
@ -752,10 +752,9 @@ impl Node {
|
||||||
}
|
}
|
||||||
|
|
||||||
pub(crate) fn ranges_is_empty(&self) -> bool {
|
pub(crate) fn ranges_is_empty(&self) -> bool {
|
||||||
match self.rare_data().as_ref() {
|
self.rare_data()
|
||||||
Some(data) => data.ranges.is_empty(),
|
.as_ref()
|
||||||
None => false,
|
.is_none_or(|data| data.ranges.is_empty())
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
#[inline]
|
#[inline]
|
||||||
|
@ -2665,7 +2664,9 @@ impl Node {
|
||||||
fn remove(node: &Node, parent: &Node, suppress_observers: SuppressObserver, can_gc: CanGc) {
|
fn remove(node: &Node, parent: &Node, suppress_observers: SuppressObserver, can_gc: CanGc) {
|
||||||
parent.owner_doc().add_script_and_layout_blocker();
|
parent.owner_doc().add_script_and_layout_blocker();
|
||||||
|
|
||||||
// Step 2.
|
// Step 1. Let parent be node’s parent.
|
||||||
|
// Step 2. Assert: parent is non-null.
|
||||||
|
// NOTE: We get parent as an argument instead
|
||||||
assert!(
|
assert!(
|
||||||
node.GetParentNode()
|
node.GetParentNode()
|
||||||
.is_some_and(|node_parent| &*node_parent == parent)
|
.is_some_and(|node_parent| &*node_parent == parent)
|
||||||
|
@ -2677,11 +2678,21 @@ impl Node {
|
||||||
if parent.ranges_is_empty() {
|
if parent.ranges_is_empty() {
|
||||||
None
|
None
|
||||||
} else {
|
} else {
|
||||||
// Step 1.
|
// Step 1. Let parent be node’s parent.
|
||||||
|
// Step 2. Assert: parent is not null.
|
||||||
|
// NOTE: We already have the parent.
|
||||||
|
|
||||||
|
// Step 3. Let index be node’s index.
|
||||||
let index = node.index();
|
let index = node.index();
|
||||||
// Steps 2-3 are handled in Node::unbind_from_tree.
|
|
||||||
// Steps 4-5.
|
// Steps 4-5 are handled in Node::unbind_from_tree.
|
||||||
|
|
||||||
|
// Step 6. For each live range whose start node is parent and start offset is greater than index,
|
||||||
|
// decrease its start offset by 1.
|
||||||
|
// Step 7. For each live range whose end node is parent and end offset is greater than index,
|
||||||
|
// decrease its end offset by 1.
|
||||||
parent.ranges().decrease_above(parent, index, 1);
|
parent.ranges().decrease_above(parent, index, 1);
|
||||||
|
|
||||||
// Parent had ranges, we needed the index, let's keep track of
|
// Parent had ranges, we needed the index, let's keep track of
|
||||||
// it to avoid computing it for other ranges when calling
|
// it to avoid computing it for other ranges when calling
|
||||||
// unbind_from_tree recursively.
|
// unbind_from_tree recursively.
|
||||||
|
@ -3925,7 +3936,12 @@ impl VirtualMethods for Node {
|
||||||
/// <https://dom.spec.whatwg.org/#concept-node-remove>
|
/// <https://dom.spec.whatwg.org/#concept-node-remove>
|
||||||
fn unbind_from_tree(&self, context: &UnbindContext, can_gc: CanGc) {
|
fn unbind_from_tree(&self, context: &UnbindContext, can_gc: CanGc) {
|
||||||
self.super_type().unwrap().unbind_from_tree(context, can_gc);
|
self.super_type().unwrap().unbind_from_tree(context, can_gc);
|
||||||
if !self.ranges_is_empty() {
|
|
||||||
|
// Ranges should only drain to the parent from inclusive non-shadow
|
||||||
|
// including descendants. If we're in a shadow tree at this point then the
|
||||||
|
// unbind operation happened further up in the tree and we should not
|
||||||
|
// drain any ranges.
|
||||||
|
if !self.is_in_a_shadow_tree() && !self.ranges_is_empty() {
|
||||||
self.ranges().drain_to_parent(context, self);
|
self.ranges().drain_to_parent(context, self);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -344,6 +344,58 @@ impl Range {
|
||||||
.chain(iter::once(end_clone))
|
.chain(iter::once(end_clone))
|
||||||
.flat_map(move |node| node.content_boxes(can_gc))
|
.flat_map(move |node| node.content_boxes(can_gc))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <https://dom.spec.whatwg.org/#concept-range-bp-set>
|
||||||
|
#[allow(clippy::neg_cmp_op_on_partial_ord)]
|
||||||
|
fn set_the_start_or_end(
|
||||||
|
&self,
|
||||||
|
node: &Node,
|
||||||
|
offset: u32,
|
||||||
|
start_or_end: StartOrEnd,
|
||||||
|
) -> ErrorResult {
|
||||||
|
// Step 1. If node is a doctype, then throw an "InvalidNodeTypeError" DOMException.
|
||||||
|
if node.is_doctype() {
|
||||||
|
return Err(Error::InvalidNodeType);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Step 2. If offset is greater than node’s length, then throw an "IndexSizeError" DOMException.
|
||||||
|
if offset > node.len() {
|
||||||
|
return Err(Error::IndexSize);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Step 3. Let bp be the boundary point (node, offset).
|
||||||
|
// NOTE: We don't need this part.
|
||||||
|
|
||||||
|
match start_or_end {
|
||||||
|
// If these steps were invoked as "set the start"
|
||||||
|
StartOrEnd::Start => {
|
||||||
|
// Step 4.1 If range’s root is not equal to node’s root, or if bp is after the range’s end,
|
||||||
|
// set range’s end to bp.
|
||||||
|
// Step 4.2 Set range’s start to bp.
|
||||||
|
self.set_start(node, offset);
|
||||||
|
if !(self.start() <= self.end()) {
|
||||||
|
self.set_end(node, offset);
|
||||||
|
}
|
||||||
|
},
|
||||||
|
// If these steps were invoked as "set the end"
|
||||||
|
StartOrEnd::End => {
|
||||||
|
// Step 4.1 If range’s root is not equal to node’s root, or if bp is before the range’s start,
|
||||||
|
// set range’s start to bp.
|
||||||
|
// Step 4.2 Set range’s end to bp.
|
||||||
|
self.set_end(node, offset);
|
||||||
|
if !(self.end() >= self.start()) {
|
||||||
|
self.set_start(node, offset);
|
||||||
|
}
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
enum StartOrEnd {
|
||||||
|
Start,
|
||||||
|
End,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl RangeMethods<crate::DomTypeHolder> for Range {
|
impl RangeMethods<crate::DomTypeHolder> for Range {
|
||||||
|
@ -365,43 +417,13 @@ impl RangeMethods<crate::DomTypeHolder> for Range {
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <https://dom.spec.whatwg.org/#dom-range-setstart>
|
/// <https://dom.spec.whatwg.org/#dom-range-setstart>
|
||||||
#[allow(clippy::neg_cmp_op_on_partial_ord)]
|
|
||||||
fn SetStart(&self, node: &Node, offset: u32) -> ErrorResult {
|
fn SetStart(&self, node: &Node, offset: u32) -> ErrorResult {
|
||||||
if node.is_doctype() {
|
self.set_the_start_or_end(node, offset, StartOrEnd::Start)
|
||||||
// Step 1.
|
|
||||||
Err(Error::InvalidNodeType)
|
|
||||||
} else if offset > node.len() {
|
|
||||||
// Step 2.
|
|
||||||
Err(Error::IndexSize)
|
|
||||||
} else {
|
|
||||||
// Step 3.
|
|
||||||
self.set_start(node, offset);
|
|
||||||
if !(self.start() <= self.end()) {
|
|
||||||
// Step 4.
|
|
||||||
self.set_end(node, offset);
|
|
||||||
}
|
|
||||||
Ok(())
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <https://dom.spec.whatwg.org/#dom-range-setend>
|
/// <https://dom.spec.whatwg.org/#dom-range-setend>
|
||||||
#[allow(clippy::neg_cmp_op_on_partial_ord)]
|
|
||||||
fn SetEnd(&self, node: &Node, offset: u32) -> ErrorResult {
|
fn SetEnd(&self, node: &Node, offset: u32) -> ErrorResult {
|
||||||
if node.is_doctype() {
|
self.set_the_start_or_end(node, offset, StartOrEnd::End)
|
||||||
// Step 1.
|
|
||||||
Err(Error::InvalidNodeType)
|
|
||||||
} else if offset > node.len() {
|
|
||||||
// Step 2.
|
|
||||||
Err(Error::IndexSize)
|
|
||||||
} else {
|
|
||||||
// Step 3.
|
|
||||||
self.set_end(node, offset);
|
|
||||||
if !(self.end() >= self.start()) {
|
|
||||||
// Step 4.
|
|
||||||
self.set_start(node, offset);
|
|
||||||
}
|
|
||||||
Ok(())
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <https://dom.spec.whatwg.org/#dom-range-setstartbefore>
|
/// <https://dom.spec.whatwg.org/#dom-range-setstartbefore>
|
||||||
|
@ -1204,6 +1226,7 @@ impl WeakRangeVec {
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Used for steps 2-3. when removing a node.
|
/// Used for steps 2-3. when removing a node.
|
||||||
|
///
|
||||||
/// <https://dom.spec.whatwg.org/#concept-node-remove>
|
/// <https://dom.spec.whatwg.org/#concept-node-remove>
|
||||||
pub(crate) fn drain_to_parent(&self, context: &UnbindContext, child: &Node) {
|
pub(crate) fn drain_to_parent(&self, context: &UnbindContext, child: &Node) {
|
||||||
if self.is_empty() {
|
if self.is_empty() {
|
||||||
|
|
|
@ -1,14 +0,0 @@
|
||||||
[Range-in-shadow-after-the-shadow-removed.html?mode=closed]
|
|
||||||
[Range in shadow should stay in the shadow after the host is removed]
|
|
||||||
expected: FAIL
|
|
||||||
|
|
||||||
[Range in shadow should stay in the shadow after the host parent is removed]
|
|
||||||
expected: FAIL
|
|
||||||
|
|
||||||
|
|
||||||
[Range-in-shadow-after-the-shadow-removed.html?mode=open]
|
|
||||||
[Range in shadow should stay in the shadow after the host is removed]
|
|
||||||
expected: FAIL
|
|
||||||
|
|
||||||
[Range in shadow should stay in the shadow after the host parent is removed]
|
|
||||||
expected: FAIL
|
|
Loading…
Add table
Add a link
Reference in a new issue