diff --git a/components/script/dom/xpathresult.rs b/components/script/dom/xpathresult.rs index 85ac43c0a38..491b005ea02 100644 --- a/components/script/dom/xpathresult.rs +++ b/components/script/dom/xpathresult.rs @@ -6,11 +6,13 @@ use std::cell::{Cell, RefCell}; use dom_struct::dom_struct; use js::rust::HandleObject; +use script_bindings::codegen::GenericBindings::WindowBinding::WindowMethods; use crate::dom::bindings::codegen::Bindings::XPathResultBinding::{ XPathResultConstants, XPathResultMethods, }; use crate::dom::bindings::error::{Error, Fallible}; +use crate::dom::bindings::inheritance::Castable; use crate::dom::bindings::reflector::{Reflector, reflect_dom_object_with_proto}; use crate::dom::bindings::root::{Dom, DomRoot}; use crate::dom::bindings::str::DOMString; @@ -84,9 +86,11 @@ impl From for XPathResultValue { pub(crate) struct XPathResult { reflector_: Reflector, window: Dom, + /// The revision of the owner document when this result was created. When iterating over the + /// values in the result, this is used to invalidate the iterator when the document is modified. + version: Cell, result_type: Cell, value: RefCell, - iterator_invalid: Cell, iterator_pos: Cell, } @@ -112,13 +116,27 @@ impl XPathResult { XPathResult { reflector_: Reflector::new(), window: Dom::from_ref(window), + version: Cell::new( + window + .Document() + .upcast::() + .inclusive_descendants_version(), + ), result_type: Cell::new(inferred_result_type), - iterator_invalid: Cell::new(false), iterator_pos: Cell::new(0), value: RefCell::new(value), } } + fn document_changed_since_creation(&self) -> bool { + let current_document_version = self + .window + .Document() + .upcast::() + .inclusive_descendants_version(); + current_document_version != self.version.get() + } + /// NB: Blindly trusts `result_type` and constructs an object regardless of the contents /// of `value`. The exception is `XPathResultType::Any`, for which we look at the value /// to determine the type. @@ -140,7 +158,12 @@ impl XPathResult { pub(crate) fn reinitialize_with(&self, result_type: XPathResultType, value: XPathResultValue) { self.result_type.set(result_type); *self.value.borrow_mut() = value; - self.iterator_invalid.set(false); + self.version.set( + self.window + .Document() + .upcast::() + .inclusive_descendants_version(), + ); self.iterator_pos.set(0); } } @@ -183,48 +206,41 @@ impl XPathResultMethods for XPathResult { /// fn IterateNext(&self) -> Fallible>> { - // TODO(vlindhol): actually set `iterator_invalid` somewhere - if self.iterator_invalid.get() { - return Err(Error::Range( - "Invalidated iterator for XPathResult, the DOM has mutated.".to_string(), - )); + if !matches!( + self.result_type.get(), + XPathResultType::OrderedNodeIterator | XPathResultType::UnorderedNodeIterator + ) { + return Err(Error::Type("Result is not an iterator".into())); } - match (&*self.value.borrow(), self.result_type.get()) { - ( - XPathResultValue::Nodeset(nodes), - XPathResultType::OrderedNodeIterator | XPathResultType::UnorderedNodeIterator, - ) => { - let pos = self.iterator_pos.get(); - if pos >= nodes.len() { - Ok(None) - } else { - let node = nodes[pos].clone(); - self.iterator_pos.set(pos + 1); - Ok(Some(node)) - } - }, - _ => Err(Error::Type( + if self.document_changed_since_creation() { + return Err(Error::InvalidState); + } + + let XPathResultValue::Nodeset(nodes) = &*self.value.borrow() else { + return Err(Error::Type( "Can't iterate on XPathResult that is not a node-set".to_string(), - )), + )); + }; + + let position = self.iterator_pos.get(); + if position >= nodes.len() { + Ok(None) + } else { + let node = nodes[position].clone(); + self.iterator_pos.set(position + 1); + Ok(Some(node)) } } /// - fn GetInvalidIteratorState(&self) -> Fallible { - let is_iterator_invalid = self.iterator_invalid.get(); - if is_iterator_invalid || - matches!( - self.result_type.get(), - XPathResultType::OrderedNodeIterator | XPathResultType::UnorderedNodeIterator - ) - { - Ok(is_iterator_invalid) - } else { - Err(Error::Type( - "Can't iterate on XPathResult that is not a node-set".to_string(), - )) - } + fn InvalidIteratorState(&self) -> bool { + let is_iterable = matches!( + self.result_type.get(), + XPathResultType::OrderedNodeIterator | XPathResultType::UnorderedNodeIterator + ); + + is_iterable && self.document_changed_since_creation() } /// diff --git a/components/script_bindings/webidls/XPathResult.webidl b/components/script_bindings/webidls/XPathResult.webidl index ed0e9804ca4..4543449bdce 100644 --- a/components/script_bindings/webidls/XPathResult.webidl +++ b/components/script_bindings/webidls/XPathResult.webidl @@ -21,7 +21,7 @@ interface XPathResult { [Throws] readonly attribute DOMString stringValue; [Throws] readonly attribute boolean booleanValue; [Throws] readonly attribute Node? singleNodeValue; - [Throws] readonly attribute boolean invalidIteratorState; + readonly attribute boolean invalidIteratorState; [Throws] readonly attribute unsigned long snapshotLength; [Throws] Node? iterateNext(); diff --git a/tests/wpt/meta/MANIFEST.json b/tests/wpt/meta/MANIFEST.json index 334f4296c54..e7b579bd3fd 100644 --- a/tests/wpt/meta/MANIFEST.json +++ b/tests/wpt/meta/MANIFEST.json @@ -643317,6 +643317,13 @@ {} ] ], + "result-iterateNext.html": [ + "8c17f7ebc1c4292a0d8eb32b3d3503ca1e693eb9", + [ + null, + {} + ] + ], "text-html-attributes.html": [ "2157dcd2d68c5701b47ba907f7b1c3b2176f9239", [ diff --git a/tests/wpt/tests/domxpath/result-iterateNext.html b/tests/wpt/tests/domxpath/result-iterateNext.html new file mode 100644 index 00000000000..8c17f7ebc1c --- /dev/null +++ b/tests/wpt/tests/domxpath/result-iterateNext.html @@ -0,0 +1,100 @@ + + + + Invalidation of iterators over XPath results + + + + + + +
    +
  • +
  • +
+ + +