script: Use xpath ns resolver to resolve namespace prefixes (#39321)

The xpath resolver is a function provided by the user to resolve
namespace prefixes. Previously, we were ignoring the argument.

Testing: New web platform tests start to pass
Part of https://github.com/servo/servo/issues/34527

---------

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

components/script/dom/element.rs

@@ -1812,16 +1812,16 @@ impl Element {
         Ref::map(self.attrs.borrow(), |attrs| &**attrs)
     }
 
-    // Element branch of https://dom.spec.whatwg.org/#locate-a-namespace
+    /// Element branch of <https://dom.spec.whatwg.org/#locate-a-namespace>
     pub(crate) fn locate_namespace(&self, prefix: Option<DOMString>) -> Namespace {
         let namespace_prefix = prefix.clone().map(|s| Prefix::from(&*s));
 
-        // "1. If prefix is "xml", then return the XML namespace."
+        // Step 1. If prefix is "xml", then return the XML namespace.
         if namespace_prefix == Some(namespace_prefix!("xml")) {
             return ns!(xml);
         }
 
-        // "2. If prefix is "xmlns", then return the XMLNS namespace."
+        // Step 2. If prefix is "xmlns", then return the XMLNS namespace.
         if namespace_prefix == Some(namespace_prefix!("xmlns")) {
             return ns!(xmlns);
         }
@@ -1833,21 +1833,20 @@ impl Element {
             .inclusive_ancestors(ShadowIncluding::No)
             .filter_map(DomRoot::downcast::<Self>);
 
-        // "5. If its parent element is null, then return null."
+        // Step 5. If its parent element is null, then return null.
-        // "6. Return the result of running locate a namespace on its parent element using prefix."
+        // Step 6. Return the result of running locate a namespace on its parent element using prefix.
         for element in inclusive_ancestor_elements {
-            // "3. If its namespace is non-null and its namespace prefix is prefix, then return
+            // Step 3. If its namespace is non-null and its namespace prefix is prefix, then return namespace.
-            // namespace."
             if element.namespace() != &ns!() &&
                 element.prefix().as_ref().map(|p| &**p) == prefix.as_deref()
             {
                 return element.namespace().clone();
             }
 
-            // "4. If it has an attribute whose namespace is the XMLNS namespace, namespace prefix
+            // Step 4. If it has an attribute whose namespace is the XMLNS namespace, namespace prefix
             // is "xmlns", and local name is prefix, or if prefix is null and it has an attribute
             // whose namespace is the XMLNS namespace, namespace prefix is null, and local name is
-            // "xmlns", then return its value if it is not the empty string, and null otherwise."
+            // "xmlns", then return its value if it is not the empty string, and null otherwise.
             let attr = Ref::filter_map(self.attrs(), |attrs| {
                 attrs.iter().find(|attr| {
                     if attr.namespace() != &ns!(xmlns) {
@@ -3090,18 +3089,18 @@ impl Element {
 
 #[allow(non_snake_case)]
 impl ElementMethods<crate::DomTypeHolder> for Element {
-    // https://dom.spec.whatwg.org/#dom-element-namespaceuri
+    /// <https://dom.spec.whatwg.org/#dom-element-namespaceuri>
     fn GetNamespaceURI(&self) -> Option<DOMString> {
         Node::namespace_to_string(self.namespace.clone())
     }
 
-    // https://dom.spec.whatwg.org/#dom-element-localname
+    /// <https://dom.spec.whatwg.org/#dom-element-localname>
     fn LocalName(&self) -> DOMString {
         // FIXME(ajeffrey): Convert directly from LocalName to DOMString
         DOMString::from(&*self.local_name)
     }
 
-    // https://dom.spec.whatwg.org/#dom-element-prefix
+    /// <https://dom.spec.whatwg.org/#dom-element-prefix>
     fn GetPrefix(&self) -> Option<DOMString> {
         self.prefix.borrow().as_ref().map(|p| DOMString::from(&**p))
     }

components/script/dom/node.rs

@@ -3988,13 +3988,10 @@ impl NodeMethods<crate::DomTypeHolder> for Node {
 
     /// <https://dom.spec.whatwg.org/#dom-node-lookupnamespaceuri>
     fn LookupNamespaceURI(&self, prefix: Option<DOMString>) -> Option<DOMString> {
-        // Step 1.
+        // Step 1. If prefix is the empty string, then set it to null.
-        let prefix = match prefix {
+        let prefix = prefix.filter(|prefix| !prefix.is_empty());
-            Some(ref p) if p.is_empty() => None,
-            pre => pre,
-        };
 
-        // Step 2.
+        // Step 2. Return the result of running locate a namespace for this using prefix.
         Node::namespace_to_string(Node::locate_namespace(self, prefix))
     }
 

components/script/dom/xpathevaluator.rs

@@ -9,7 +9,6 @@ use js::rust::HandleObject;
 
 use super::bindings::error::Error;
 use crate::dom::bindings::codegen::Bindings::XPathEvaluatorBinding::XPathEvaluatorMethods;
-use crate::dom::bindings::codegen::Bindings::XPathExpressionBinding::XPathExpression_Binding::XPathExpressionMethods;
 use crate::dom::bindings::codegen::Bindings::XPathNSResolverBinding::XPathNSResolver;
 use crate::dom::bindings::error::Fallible;
 use crate::dom::bindings::reflector::{DomGlobal, Reflector, reflect_dom_object_with_proto};
@@ -91,7 +90,7 @@ impl XPathEvaluatorMethods<crate::DomTypeHolder> for XPathEvaluator {
         &self,
         expression_str: DOMString,
         context_node: &Node,
-        _resolver: Option<Rc<XPathNSResolver>>,
+        resolver: Option<Rc<XPathNSResolver>>,
         result_type: u16,
         result: Option<&XPathResult>,
         can_gc: CanGc,
@@ -101,12 +100,6 @@ impl XPathEvaluatorMethods<crate::DomTypeHolder> for XPathEvaluator {
         let parsed_expression =
             crate::xpath::parse(&expression_str).map_err(|_| Error::Syntax(None))?;
         let expression = XPathExpression::new(window, None, can_gc, parsed_expression);
-        XPathExpressionMethods::<crate::DomTypeHolder>::Evaluate(
+        expression.evaluate_internal(context_node, result_type, result, resolver, can_gc)
-            &*expression,
-            context_node,
-            result_type,
-            result,
-            can_gc,
-        )
     }
 }

components/script/dom/xpathexpression.rs

@@ -2,10 +2,13 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
+use std::rc::Rc;
+
 use dom_struct::dom_struct;
 use js::rust::HandleObject;
 
 use crate::dom::bindings::codegen::Bindings::XPathExpressionBinding::XPathExpressionMethods;
+use crate::dom::bindings::codegen::Bindings::XPathNSResolverBinding::XPathNSResolver;
 use crate::dom::bindings::error::{Error, Fallible};
 use crate::dom::bindings::reflector::{DomGlobal, Reflector, reflect_dom_object_with_proto};
 use crate::dom::bindings::root::{Dom, DomRoot};
@@ -45,15 +48,13 @@ impl XPathExpression {
             can_gc,
         )
     }
-}
 
-impl XPathExpressionMethods<crate::DomTypeHolder> for XPathExpression {
+    pub(crate) fn evaluate_internal(
-    /// <https://dom.spec.whatwg.org/#dom-xpathexpression-evaluate>
-    fn Evaluate(
         &self,
         context_node: &Node,
         result_type_num: u16,
         _result: Option<&XPathResult>,
+        resolver: Option<Rc<XPathNSResolver>>,
         can_gc: CanGc,
     ) -> Fallible<DomRoot<XPathResult>> {
         let result_type = XPathResultType::try_from(result_type_num)
@@ -62,8 +63,7 @@ impl XPathExpressionMethods<crate::DomTypeHolder> for XPathExpression {
         let global = self.global();
         let window = global.as_window();
 
-        let result_value = evaluate_parsed_xpath(&self.parsed_expression, context_node)
+        let result_value = evaluate_parsed_xpath(&self.parsed_expression, context_node, resolver)?;
-            .map_err(|_e| Error::Operation)?;
 
         // TODO(vlindhol): support putting results into mutable `_result` as per the spec
         Ok(XPathResult::new(
@@ -75,3 +75,16 @@ impl XPathExpressionMethods<crate::DomTypeHolder> for XPathExpression {
         ))
     }
 }
+
+impl XPathExpressionMethods<crate::DomTypeHolder> for XPathExpression {
+    /// <https://dom.spec.whatwg.org/#dom-xpathexpression-evaluate>
+    fn Evaluate(
+        &self,
+        context_node: &Node,
+        result_type_num: u16,
+        result: Option<&XPathResult>,
+        can_gc: CanGc,
+    ) -> Fallible<DomRoot<XPathResult>> {
+        self.evaluate_internal(context_node, result_type_num, result, None, can_gc)
+    }
+}

components/script/xpath/context.rs

@@ -2,26 +2,33 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
+use std::fmt;
 use std::iter::Enumerate;
+use std::rc::Rc;
 use std::vec::IntoIter;
 
+use script_bindings::error::Fallible;
+use script_bindings::script_runtime::CanGc;
 use script_bindings::str::DOMString;
 
 use super::Node;
+use crate::dom::bindings::callback::ExceptionHandling;
 use crate::dom::bindings::codegen::Bindings::NodeBinding::NodeMethods;
+use crate::dom::bindings::codegen::Bindings::XPathNSResolverBinding::XPathNSResolver;
 use crate::dom::bindings::root::DomRoot;
 
 /// The context during evaluation of an XPath expression.
-#[derive(Debug)]
 pub(crate) struct EvaluationCtx {
-    /// Where we started at
+    /// Where we started at.
     pub(crate) starting_node: DomRoot<Node>,
-    /// The "current" node in the evaluation
+    /// The "current" node in the evaluation.
     pub(crate) context_node: DomRoot<Node>,
-    /// Details needed for evaluating a predicate list
+    /// Details needed for evaluating a predicate list.
     pub(crate) predicate_ctx: Option<PredicateCtx>,
-    /// The nodes we're currently matching against
+    /// The nodes we're currently matching against.
     pub(crate) predicate_nodes: Option<Vec<DomRoot<Node>>>,
+    /// A list of known namespace prefixes.
+    pub(crate) resolver: Option<Rc<XPathNSResolver>>,
 }
 
 #[derive(Clone, Copy, Debug)]
@@ -32,12 +39,13 @@ pub(crate) struct PredicateCtx {
 
 impl EvaluationCtx {
     /// Prepares the context used while evaluating the XPath expression
-    pub(crate) fn new(context_node: &Node) -> EvaluationCtx {
+    pub(crate) fn new(context_node: &Node, resolver: Option<Rc<XPathNSResolver>>) -> EvaluationCtx {
         EvaluationCtx {
             starting_node: DomRoot::from_ref(context_node),
             context_node: DomRoot::from_ref(context_node),
             predicate_ctx: None,
             predicate_nodes: None,
+            resolver,
         }
     }
 
@@ -48,6 +56,7 @@ impl EvaluationCtx {
             context_node: DomRoot::from_ref(node),
             predicate_ctx: self.predicate_ctx,
             predicate_nodes: self.predicate_nodes.clone(),
+            resolver: self.resolver.clone(),
         }
     }
 
@@ -57,6 +66,7 @@ impl EvaluationCtx {
             context_node: self.context_node.clone(),
             predicate_ctx: None,
             predicate_nodes: Some(nodes.into_iter().map(DomRoot::from_ref).collect()),
+            resolver: self.resolver.clone(),
         }
     }
 
@@ -74,9 +84,26 @@ impl EvaluationCtx {
     }
 
     /// Resolve a namespace prefix using the context node's document
-    pub(crate) fn resolve_namespace(&self, prefix: Option<&str>) -> Option<DOMString> {
+    pub(crate) fn resolve_namespace(
-        self.context_node
+        &self,
-            .LookupNamespaceURI(prefix.map(DOMString::from))
+        prefix: Option<&str>,
+        can_gc: CanGc,
+    ) -> Fallible<Option<DOMString>> {
+        // First check if the prefix is known by our resolver function
+        if let Some(resolver) = self.resolver.as_ref() {
+            if let Some(namespace_uri) = resolver.LookupNamespaceURI__(
+                prefix.map(DOMString::from),
+                ExceptionHandling::Rethrow,
+                can_gc,
+            )? {
+                return Ok(Some(namespace_uri));
+            }
+        }
+
+        // Then, see if it's defined on the context node
+        Ok(self
+            .context_node
+            .LookupNamespaceURI(prefix.map(DOMString::from)))
     }
 }
 
@@ -100,6 +127,19 @@ impl Iterator for EvalNodesetIter<'_> {
                 index: idx + 1,
                 size: self.size,
             }),
+            resolver: self.ctx.resolver.clone(),
         })
     }
 }
+
+impl fmt::Debug for EvaluationCtx {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.debug_struct("EvaluationCtx")
+            .field("starting_node", &self.starting_node)
+            .field("context_node", &self.context_node)
+            .field("predicate_ctx", &self.predicate_ctx)
+            .field("predicate_nodes", &self.predicate_nodes)
+            .field("resolver", &"<callback function>")
+            .finish()
+    }
+}

components/script/xpath/eval.rs

@@ -5,6 +5,7 @@
 use std::fmt;
 
 use html5ever::{LocalName, Namespace, Prefix, QualName, local_name, namespace_prefix, ns};
+use script_bindings::script_runtime::CanGc;
 
 use super::parser::{
     AdditiveOp, Axis, EqualityOp, Expr, FilterExpr, KindTest, Literal, MultiplicativeOp, NodeTest,
@@ -15,6 +16,7 @@ use super::{EvaluationCtx, Value};
 use crate::dom::attr::Attr;
 use crate::dom::bindings::codegen::Bindings::NodeBinding::NodeMethods;
 use crate::dom::bindings::domname::namespace_from_domstring;
+use crate::dom::bindings::error::Error as JsError;
 use crate::dom::bindings::inheritance::{Castable, CharacterDataTypeId, NodeTypeId};
 use crate::dom::bindings::root::DomRoot;
 use crate::dom::bindings::str::DOMString;
@@ -24,16 +26,30 @@ use crate::dom::node::{Node, ShadowIncluding};
 use crate::dom::processinginstruction::ProcessingInstruction;
 use crate::xpath::context::PredicateCtx;
 
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug)]
 pub(crate) enum Error {
     NotANodeset,
     InvalidPath,
-    UnknownFunction { name: QualName },
+    UnknownFunction {
-    UnknownVariable { name: QualName },
+        name: QualName,
-    UnknownNamespace { prefix: String },
+    },
-    InvalidQName { qname: ParserQualName },
+    UnknownVariable {
-    FunctionEvaluation { fname: String },
+        name: QualName,
-    Internal { msg: String },
+    },
+    UnknownNamespace {
+        prefix: String,
+    },
+    InvalidQName {
+        qname: ParserQualName,
+    },
+    FunctionEvaluation {
+        fname: String,
+    },
+    Internal {
+        msg: String,
+    },
+    /// A JS exception that needs to be propagated to the caller.
+    JsException(JsError),
 }
 
 impl std::fmt::Display for Error {
@@ -55,6 +71,9 @@ impl std::fmt::Display for Error {
             Error::Internal { msg } => {
                 write!(f, "internal error: {}", msg)
             },
+            Error::JsException(exception) => {
+                write!(f, "JS exception: {:?}", exception)
+            },
         }
     }
 }
@@ -360,27 +379,22 @@ fn validate_and_extract(
     }
 }
 
-pub(crate) struct QualNameConverter<'a> {
+pub(crate) fn convert_parsed_qname_to_qualified_name(
-    qname: &'a ParserQualName,
+    qname: &ParserQualName,
-    context: &'a EvaluationCtx,
+    context: &EvaluationCtx,
-}
+    can_gc: CanGc,
+) -> Result<QualName, Error> {
+    let qname_as_str = qname.to_string();
+    let namespace = context
+        .resolve_namespace(qname.prefix.as_deref(), can_gc)
+        .map_err(Error::JsException)?;
 
-impl<'a> TryFrom<QualNameConverter<'a>> for QualName {
+    if let Ok((ns, prefix, local)) = validate_and_extract(namespace, &qname_as_str) {
-    type Error = Error;
+        Ok(QualName { prefix, ns, local })
-
+    } else {
-    fn try_from(converter: QualNameConverter<'a>) -> Result<Self, Self::Error> {
+        Err(Error::InvalidQName {
-        let qname_as_str = converter.qname.to_string();
+            qname: qname.clone(),
-        let namespace = converter
+        })
-            .context
-            .resolve_namespace(converter.qname.prefix.as_deref());
-
-        if let Ok((ns, prefix, local)) = validate_and_extract(namespace, &qname_as_str) {
-            Ok(QualName { prefix, ns, local })
-        } else {
-            Err(Error::InvalidQName {
-                qname: converter.qname.clone(),
-            })
-        }
     }
 }
 
@@ -434,11 +448,16 @@ pub(crate) fn element_name_test(
     }
 }
 
-fn apply_node_test(context: &EvaluationCtx, test: &NodeTest, node: &Node) -> Result<bool, Error> {
+fn apply_node_test(
+    context: &EvaluationCtx,
+    test: &NodeTest,
+    node: &Node,
+    can_gc: CanGc,
+) -> Result<bool, Error> {
     let result = match test {
         NodeTest::Name(qname) => {
             // Convert the unvalidated "parser QualName" into the proper QualName structure
-            let wanted_name: QualName = QualNameConverter { qname, context }.try_into()?;
+            let wanted_name = convert_parsed_qname_to_qualified_name(qname, context, can_gc)?;
             match node.type_id() {
                 NodeTypeId::Element(_) => {
                     let element = node.downcast::<Element>().unwrap();
@@ -563,7 +582,9 @@ impl Evaluatable for StepExpr {
                 let filtered_nodes: Vec<DomRoot<Node>> = nodes
                     .into_iter()
                     .map(|node| {
-                        apply_node_test(context, &axis_step.node_test, &node)
+                        // FIXME: propagate this can_gc up further. This likely requires removing the "Evaluate"
+                        // trait or changing the signature of "evaluate". The trait is not really necessary anyways.
+                        apply_node_test(context, &axis_step.node_test, &node, CanGc::note())
                             .map(|matches| matches.then_some(node))
                     })
                     .collect::<Result<Vec<_>, _>>()?
@@ -613,6 +634,7 @@ impl Evaluatable for PredicateListExpr {
                         context_node: node.clone(),
                         predicate_nodes: context.predicate_nodes.clone(),
                         predicate_ctx: Some(PredicateCtx { index: i + 1, size }),
+                        resolver: context.resolver.clone(),
                     };
 
                     let eval_result = predicate_expr.expr.evaluate(&predicate_ctx);

components/script/xpath/mod.rs

@@ -2,13 +2,16 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
+use std::rc::Rc;
+
 use context::EvaluationCtx;
-use eval::Evaluatable;
+use eval::{Error as EvaluationError, Evaluatable};
 pub(crate) use eval_value::{NodesetHelpers, Value};
-use parser::OwnedParserError;
 pub(crate) use parser::{Expr, parse as parse_impl};
 
 use super::dom::node::Node;
+use crate::dom::bindings::codegen::Bindings::XPathNSResolverBinding::XPathNSResolver;
+use crate::dom::bindings::error::{Error as JsError, Error, Fallible};
 
 mod context;
 #[allow(dead_code)]
@@ -19,58 +22,41 @@ mod eval_value;
 #[allow(dead_code)]
 mod parser;
 
-/// The failure modes of executing an XPath.
-#[derive(Debug, PartialEq)]
-pub(crate) enum Error {
-    /// The XPath was syntactically invalid
-    Parsing { source: OwnedParserError },
-    /// The XPath could not be executed
-    Evaluating { source: eval::Error },
-}
-
-impl std::fmt::Display for Error {
-    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
-        match self {
-            Error::Parsing { source } => write!(f, "Unable to parse XPath: {}", source),
-            Error::Evaluating { source } => write!(f, "Unable to evaluate XPath: {}", source),
-        }
-    }
-}
-
-impl std::error::Error for Error {
-    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
-        match self {
-            Error::Parsing { source } => Some(source),
-            Error::Evaluating { source } => Some(source),
-        }
-    }
-}
-
 /// Parse an XPath expression from a string
-pub(crate) fn parse(xpath: &str) -> Result<Expr, Error> {
+pub(crate) fn parse(xpath: &str) -> Fallible<Expr> {
     match parse_impl(xpath) {
         Ok(expr) => {
-            debug!("Parsed XPath: {:?}", expr);
+            debug!("Parsed XPath: {expr:?}");
             Ok(expr)
         },
-        Err(e) => {
+        Err(error) => {
-            debug!("Unable to parse XPath: {}", e);
+            debug!("Unable to parse XPath: {error}");
-            Err(Error::Parsing { source: e })
+            Err(Error::Operation)
         },
     }
 }
 
 /// Evaluate an already-parsed XPath expression
-pub(crate) fn evaluate_parsed_xpath(expr: &Expr, context_node: &Node) -> Result<Value, Error> {
+pub(crate) fn evaluate_parsed_xpath(
-    let context = EvaluationCtx::new(context_node);
+    expr: &Expr,
+    context_node: &Node,
+    resolver: Option<Rc<XPathNSResolver>>,
+) -> Fallible<Value> {
+    let context = EvaluationCtx::new(context_node, resolver);
     match expr.evaluate(&context) {
-        Ok(v) => {
+        Ok(value) => {
-            debug!("Evaluated XPath: {:?}", v);
+            debug!("Evaluated XPath: {value:?}");
-            Ok(v)
+            Ok(value)
         },
-        Err(e) => {
+        Err(error) => {
-            debug!("Unable to evaluate XPath: {}", e);
+            debug!("Unable to evaluate XPath: {error}");
-            Err(Error::Evaluating { source: e })
+
+            let error = match error {
+                EvaluationError::JsException(exception) => exception,
+                _ => JsError::Operation,
+            };
+
+            Err(error)
         },
     }
 }

tests/wpt/meta/domxpath/text-html-attributes.html.ini

@@ -11,9 +11,6 @@
   [Select HTML element with non-ascii attribute 3]
     expected: FAIL
 
-  [Select SVG element based on mixed case attribute]
-    expected: FAIL
-
   [Select both HTML and SVG elements based on mixed case attribute]
     expected: FAIL
 
@@ -22,6 +19,3 @@
 
   [Select SVG element with non-ascii attribute 2]
     expected: FAIL
-
-  [svg element with XLink attribute]
-    expected: FAIL

tests/wpt/meta/domxpath/text-html-elements.html.ini

@@ -1,19 +1,7 @@
 [text-html-elements.html]
-  [HTML elements namespace prefix]
-    expected: FAIL
-
-  [HTML elements mixed use of prefix]
-    expected: FAIL
-
-  [SVG elements namespace prefix]
-    expected: FAIL
-
   [HTML elements mixed case]
     expected: FAIL
 
-  [SVG elements mixed case selector]
-    expected: FAIL
-
   [Non-ascii HTML element]
     expected: FAIL