Simplify rust-selectors API for attribute selectors

components/script/dom/element.rs

@@ -86,9 +86,10 @@ use net_traits::request::CorsSettings;
 use ref_filter_map::ref_filter_map;
 use script_layout_interface::message::ReflowQueryType;
 use script_thread::Runnable;
+use selectors::attr::AttrSelectorOperation;
 use selectors::matching::{ElementSelectorFlags, MatchingContext, MatchingMode, matches_selector_list};
 use selectors::matching::{HAS_EDGE_CHILD_SELECTOR, HAS_SLOW_SELECTOR, HAS_SLOW_SELECTOR_LATER_SIBLINGS};
-use selectors::parser::{AttrSelector, NamespaceConstraint};
+use selectors::parser::NamespaceConstraint;
 use servo_atoms::Atom;
 use std::ascii::AsciiExt;
 use std::borrow::Cow;
@@ -288,7 +289,7 @@ pub trait RawLayoutElementHelpers {
                                       -> Option<&'a AttrValue>;
     unsafe fn get_attr_val_for_layout<'a>(&'a self, namespace: &Namespace, name: &LocalName)
                                       -> Option<&'a str>;
-    unsafe fn get_attr_vals_for_layout<'a>(&'a self, name: &LocalName) -> Vec<&'a str>;
+    unsafe fn get_attr_vals_for_layout<'a>(&'a self, name: &LocalName) -> Vec<&'a AttrValue>;
 }
 
 #[inline]
@@ -314,6 +315,7 @@ impl RawLayoutElementHelpers for Element {
         })
     }
 
+    #[inline]
     unsafe fn get_attr_val_for_layout<'a>(&'a self, namespace: &Namespace, name: &LocalName)
                                           -> Option<&'a str> {
         get_attr_for_layout(self, namespace, name).map(|attr| {
@@ -322,12 +324,12 @@ impl RawLayoutElementHelpers for Element {
     }
 
     #[inline]
-    unsafe fn get_attr_vals_for_layout<'a>(&'a self, name: &LocalName) -> Vec<&'a str> {
+    unsafe fn get_attr_vals_for_layout<'a>(&'a self, name: &LocalName) -> Vec<&'a AttrValue> {
         let attrs = self.attrs.borrow_for_layout();
         attrs.iter().filter_map(|attr| {
             let attr = attr.to_layout();
             if *name == attr.local_name_atom_forever() {
-              Some(attr.value_ref_forever())
+              Some(attr.value_forever())
             } else {
               None
             }
@@ -2352,37 +2354,9 @@ impl VirtualMethods for Element {
     }
 }
 
-impl<'a> ::selectors::MatchAttrGeneric for Root<Element> {
+impl<'a> ::selectors::Element for Root<Element> {
     type Impl = SelectorImpl;
 
-    fn match_attr<F>(&self, attr: &AttrSelector<SelectorImpl>, test: F) -> bool
-        where F: Fn(&str) -> bool
-    {
-        use ::selectors::Element;
-        let local_name = {
-            if self.is_html_element_in_html_document() {
-                &attr.lower_name
-            } else {
-                &attr.name
-            }
-        };
-        match attr.namespace {
-            NamespaceConstraint::Specific(ref ns) => {
-                self.get_attribute(&ns.url, local_name)
-                    .map_or(false, |attr| {
-                        test(&attr.value())
-                    })
-            },
-            NamespaceConstraint::Any => {
-                self.attrs.borrow().iter().any(|attr| {
-                    attr.local_name() == local_name && test(&attr.value())
-                })
-            }
-        }
-    }
-}
-
-impl<'a> ::selectors::Element for Root<Element> {
     fn parent_element(&self) -> Option<Root<Element>> {
         self.upcast::<Node>().GetParentElement()
     }
@@ -2412,6 +2386,25 @@ impl<'a> ::selectors::Element for Root<Element> {
         self.node.following_siblings().filter_map(Root::downcast).next()
     }
 
+    fn attr_matches(&self,
+                    ns: &NamespaceConstraint<SelectorImpl>,
+                    local_name: &LocalName,
+                    operation: &AttrSelectorOperation<SelectorImpl>)
+                    -> bool {
+        match *ns {
+            NamespaceConstraint::Specific(ref ns) => {
+                self.get_attribute(&ns.url, local_name)
+                    .map_or(false, |attr| attr.value().eval_selector(operation))
+            }
+            NamespaceConstraint::Any => {
+                self.attrs.borrow().iter().any(|attr| {
+                    attr.local_name() == local_name &&
+                    attr.value().eval_selector(operation)
+                })
+            }
+        }
+    }
+
     fn is_root(&self) -> bool {
         match self.node.GetParentNode() {
             None => false,

components/script/layout_wrapper.rs

@@ -50,8 +50,9 @@ use script_layout_interface::{HTMLCanvasData, LayoutNodeType, SVGSVGData, Truste
 use script_layout_interface::{OpaqueStyleAndLayoutData, PartialPersistentLayoutData};
 use script_layout_interface::wrapper_traits::{DangerousThreadSafeLayoutNode, GetLayoutData, LayoutNode};
 use script_layout_interface::wrapper_traits::{PseudoElementType, ThreadSafeLayoutElement, ThreadSafeLayoutNode};
+use selectors::attr::AttrSelectorOperation;
 use selectors::matching::{ElementSelectorFlags, MatchingContext};
-use selectors::parser::{AttrSelector, NamespaceConstraint};
+use selectors::parser::NamespaceConstraint;
 use servo_atoms::Atom;
 use servo_url::ServoUrl;
 use std::fmt;
@@ -509,6 +510,13 @@ impl<'le> ServoLayoutElement<'le> {
         }
     }
 
+    #[inline]
+    fn get_attr_enum(&self, namespace: &Namespace, name: &LocalName) -> Option<&AttrValue> {
+        unsafe {
+            (*self.element.unsafe_get()).get_attr_for_layout(namespace, name)
+        }
+    }
+
     #[inline]
     fn get_attr(&self, namespace: &Namespace, name: &LocalName) -> Option<&str> {
         unsafe {
@@ -558,32 +566,9 @@ fn as_element<'le>(node: LayoutJS<Node>) -> Option<ServoLayoutElement<'le>> {
     node.downcast().map(ServoLayoutElement::from_layout_js)
 }
 
-impl<'le> ::selectors::MatchAttrGeneric for ServoLayoutElement<'le> {
+impl<'le> ::selectors::Element for ServoLayoutElement<'le> {
     type Impl = SelectorImpl;
 
-    fn match_attr<F>(&self, attr: &AttrSelector<SelectorImpl>, test: F) -> bool
-    where F: Fn(&str) -> bool {
-        use ::selectors::Element;
-        let name = if self.is_html_element_in_html_document() {
-            &attr.lower_name
-        } else {
-            &attr.name
-        };
-        match attr.namespace {
-            NamespaceConstraint::Specific(ref ns) => {
-                self.get_attr(&ns.url, name).map_or(false, |attr| test(attr))
-            },
-            NamespaceConstraint::Any => {
-                let attrs = unsafe {
-                    (*self.element.unsafe_get()).get_attr_vals_for_layout(name)
-                };
-                attrs.iter().any(|attr| test(*attr))
-            }
-        }
-    }
-}
-
-impl<'le> ::selectors::Element for ServoLayoutElement<'le> {
     fn parent_element(&self) -> Option<ServoLayoutElement<'le>> {
         unsafe {
             self.element.upcast().parent_node_ref().and_then(as_element)
@@ -620,6 +605,25 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> {
         None
     }
 
+    fn attr_matches(&self,
+                    ns: &NamespaceConstraint<SelectorImpl>,
+                    local_name: &LocalName,
+                    operation: &AttrSelectorOperation<SelectorImpl>)
+                    -> bool {
+        match *ns {
+            NamespaceConstraint::Specific(ref ns) => {
+                self.get_attr_enum(&ns.url, local_name)
+                    .map_or(false, |value| value.eval_selector(operation))
+            }
+            NamespaceConstraint::Any => {
+                let values = unsafe {
+                    (*self.element.unsafe_get()).get_attr_vals_for_layout(local_name)
+                };
+                values.iter().any(|value| value.eval_selector(operation))
+            }
+        }
+    }
+
     fn is_root(&self) -> bool {
         match self.as_node().parent_node() {
             None => false,
@@ -1075,6 +1079,10 @@ impl<'le> ThreadSafeLayoutElement for ServoThreadSafeLayoutElement<'le> {
         self.element
     }
 
+    fn get_attr_enum(&self, namespace: &Namespace, name: &LocalName) -> Option<&AttrValue> {
+        self.element.get_attr_enum(namespace, name)
+    }
+
     fn get_attr<'a>(&'a self, namespace: &Namespace, name: &LocalName) -> Option<&'a str> {
         self.element.get_attr(namespace, name)
     }
@@ -1096,25 +1104,9 @@ impl<'le> ThreadSafeLayoutElement for ServoThreadSafeLayoutElement<'le> {
 ///
 /// Note that the element implementation is needed only for selector matching,
 /// not for inheritance (styles are inherited appropiately).
-impl<'le> ::selectors::MatchAttrGeneric for ServoThreadSafeLayoutElement<'le> {
+impl<'le> ::selectors::Element for ServoThreadSafeLayoutElement<'le> {
     type Impl = SelectorImpl;
 
-    fn match_attr<F>(&self, attr: &AttrSelector<SelectorImpl>, test: F) -> bool
-        where F: Fn(&str) -> bool {
-        match attr.namespace {
-            NamespaceConstraint::Specific(ref ns) => {
-                self.get_attr(&ns.url, &attr.name).map_or(false, |attr| test(attr))
-            },
-            NamespaceConstraint::Any => {
-                unsafe {
-                    (*self.element.element.unsafe_get()).get_attr_vals_for_layout(&attr.name).iter()
-                        .any(|attr| test(*attr))
-                }
-            }
-        }
-    }
-}
-impl<'le> ::selectors::Element for ServoThreadSafeLayoutElement<'le> {
     fn parent_element(&self) -> Option<Self> {
         warn!("ServoThreadSafeLayoutElement::parent_element called");
         None
@@ -1166,6 +1158,25 @@ impl<'le> ::selectors::Element for ServoThreadSafeLayoutElement<'le> {
         false
     }
 
+    fn attr_matches(&self,
+                    ns: &NamespaceConstraint<SelectorImpl>,
+                    local_name: &LocalName,
+                    operation: &AttrSelectorOperation<SelectorImpl>)
+                    -> bool {
+        match *ns {
+            NamespaceConstraint::Specific(ref ns) => {
+                self.get_attr_enum(&ns.url, local_name)
+                    .map_or(false, |value| value.eval_selector(operation))
+            }
+            NamespaceConstraint::Any => {
+                let values = unsafe {
+                    (*self.element.element.unsafe_get()).get_attr_vals_for_layout(local_name)
+                };
+                values.iter().any(|v| v.eval_selector(operation))
+            }
+        }
+    }
+
     fn match_non_ts_pseudo_class<F>(&self,
                                     _: &NonTSPseudoClass,
                                     _: &mut MatchingContext,

components/script_layout_interface/wrapper_traits.rs

@@ -15,6 +15,7 @@ use msg::constellation_msg::{BrowsingContextId, PipelineId};
 use range::Range;
 use servo_url::ServoUrl;
 use std::fmt::Debug;
+use style::attr::AttrValue;
 use style::computed_values::display;
 use style::context::SharedStyleContext;
 use style::data::ElementData;
@@ -335,6 +336,8 @@ pub trait ThreadSafeLayoutElement: Clone + Copy + Sized + Debug +
     #[inline]
     fn get_attr(&self, namespace: &Namespace, name: &LocalName) -> Option<&str>;
 
+    fn get_attr_enum(&self, namespace: &Namespace, name: &LocalName) -> Option<&AttrValue>;
+
     fn get_style_data(&self) -> Option<&AtomicRefCell<ElementData>>;
 
     #[inline]

components/selectors/attr.rs

@@ -0,0 +1,114 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+use parser::SelectorImpl;
+use std::ascii::AsciiExt;
+
+pub enum AttrSelectorOperation<'a, Impl: SelectorImpl> where Impl::AttrValue: 'a {
+    Exists,
+    WithValue {
+        operator: AttrSelectorOperator,
+        case_sensitivity: CaseSensitivity,
+        expected_value: &'a Impl::AttrValue,
+    }
+}
+
+impl<'a, Impl: SelectorImpl> AttrSelectorOperation<'a, Impl> {
+    pub fn eval_str(&self, element_attr_value: &str) -> bool
+    where Impl::AttrValue: AsRef<str> {
+        match *self {
+            AttrSelectorOperation::Exists => true,
+            AttrSelectorOperation::WithValue { operator, case_sensitivity, expected_value } => {
+                operator.eval_str(element_attr_value, expected_value.as_ref(), case_sensitivity)
+            }
+        }
+    }
+}
+
+#[derive(Eq, PartialEq, Clone, Copy)]
+pub enum AttrSelectorOperator {
+    Equal,  // =
+    Includes,  // ~=
+    DashMatch,  // |=
+    Prefix,  // ^=
+    Substring,  // *=
+    Suffix,  // $=
+}
+
+impl AttrSelectorOperator {
+    pub fn eval_str(self, element_attr_value: &str, attr_selector_value: &str,
+                    case_sensitivity: CaseSensitivity) -> bool {
+        let e = element_attr_value.as_bytes();
+        let s = attr_selector_value.as_bytes();
+        let case = case_sensitivity;
+        match self {
+            AttrSelectorOperator::Equal => {
+                case.eq(e, s)
+            }
+            AttrSelectorOperator::Prefix => {
+                e.len() >= s.len() && case.eq(&e[..s.len()], s)
+            }
+            AttrSelectorOperator::Suffix => {
+                e.len() >= s.len() && case.eq(&e[(e.len() - s.len())..], s)
+            }
+            AttrSelectorOperator::Substring => {
+                case.contains(element_attr_value, attr_selector_value)
+            }
+            AttrSelectorOperator::Includes => {
+                element_attr_value.split(SELECTOR_WHITESPACE)
+                                  .any(|part| case.eq(part.as_bytes(), s))
+            }
+            AttrSelectorOperator::DashMatch => {
+                case.eq(e, s) || (
+                    e.get(s.len()) == Some(&b'-') &&
+                    case.eq(&e[..s.len()], s)
+                )
+            }
+        }
+    }
+}
+
+/// The definition of whitespace per CSS Selectors Level 3 § 4.
+pub static SELECTOR_WHITESPACE: &'static [char] = &[' ', '\t', '\n', '\r', '\x0C'];
+
+#[derive(Eq, PartialEq, Clone, Copy, Debug)]
+pub enum CaseSensitivity {
+    CaseSensitive,  // Selectors spec says language-defined, but HTML says sensitive.
+    AsciiCaseInsensitive,
+}
+
+impl CaseSensitivity {
+    pub fn eq(self, a: &[u8], b: &[u8]) -> bool {
+        match self {
+            CaseSensitivity::CaseSensitive => a == b,
+            CaseSensitivity::AsciiCaseInsensitive => a.eq_ignore_ascii_case(b)
+        }
+    }
+
+    pub fn contains(self, haystack: &str, needle: &str) -> bool {
+        match self {
+            CaseSensitivity::CaseSensitive => haystack.contains(needle),
+            CaseSensitivity::AsciiCaseInsensitive => {
+                if let Some((&n_first_byte, n_rest)) = needle.as_bytes().split_first() {
+                    haystack.bytes().enumerate().any(|(i, byte)| {
+                        if !byte.eq_ignore_ascii_case(&n_first_byte) {
+                            return false
+                        }
+                        let after_this_byte = &haystack.as_bytes()[i + 1..];
+                        match after_this_byte.get(..n_rest.len()) {
+                            None => false,
+                            Some(haystack_slice) => {
+                                haystack_slice.eq_ignore_ascii_case(n_rest)
+                            }
+                        }
+                    })
+                } else {
+                    // any_str.contains("") == true,
+                    // though these cases should be handled with *NeverMatches and never go here.
+                    true
+                }
+            }
+        }
+    }
+}

components/selectors/lib.rs

@@ -11,6 +11,7 @@ extern crate precomputed_hash;
 extern crate smallvec;
 
 pub mod arcslice;
+pub mod attr;
 pub mod bloom;
 pub mod matching;
 pub mod parser;
@@ -21,4 +22,3 @@ pub mod visitor;
 
 pub use parser::{SelectorImpl, Parser, SelectorList};
 pub use tree::Element;
-pub use tree::{MatchAttr, MatchAttrGeneric};

components/selectors/matching.rs

@@ -1,9 +1,11 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+use attr::{AttrSelectorOperation, AttrSelectorOperator, CaseSensitivity};
 use bloom::BloomFilter;
-use parser::{CaseSensitivity, Combinator, ComplexSelector, Component, LocalName};
+use parser::{Combinator, ComplexSelector, Component, LocalName, NamespaceConstraint};
-use parser::{Selector, SelectorInner, SelectorIter, SelectorImpl};
+use parser::{Selector, SelectorInner, SelectorIter, SelectorImpl, AttrSelector};
 use std::borrow::Borrow;
 use tree::Element;
 
@@ -410,28 +412,56 @@ fn matches_simple_selector<E, F>(
             element.has_class(class)
         }
         Component::AttrExists(ref attr) => {
-            element.match_attr_has(attr)
+            let (ns, n) = unpack_attr_selector(element, attr);
+            element.attr_matches(ns, n, &AttrSelectorOperation::Exists)
         }
         Component::AttrEqual(ref attr, ref value, case_sensitivity) => {
-            match case_sensitivity {
+            let (ns, n) = unpack_attr_selector(element, attr);
-                CaseSensitivity::CaseSensitive => element.match_attr_equals(attr, value),
+            element.attr_matches(ns, n, &AttrSelectorOperation::WithValue {
-                CaseSensitivity::AsciiCaseInsensitive => element.match_attr_equals_ignore_ascii_case(attr, value),
+                operator: AttrSelectorOperator::Equal,
-            }
+                case_sensitivity: case_sensitivity,
+                expected_value: value
+            })
         }
         Component::AttrIncludes(ref attr, ref value) => {
-            element.match_attr_includes(attr, value)
+            let (ns, n) = unpack_attr_selector(element, attr);
+            element.attr_matches(ns, n, &AttrSelectorOperation::WithValue {
+                operator: AttrSelectorOperator::Includes,
+                case_sensitivity: CaseSensitivity::CaseSensitive,
+                expected_value: value
+            })
         }
         Component::AttrDashMatch(ref attr, ref value) => {
-            element.match_attr_dash(attr, value)
+            let (ns, n) = unpack_attr_selector(element, attr);
+            element.attr_matches(ns, n, &AttrSelectorOperation::WithValue {
+                operator: AttrSelectorOperator::DashMatch,
+                case_sensitivity: CaseSensitivity::CaseSensitive,
+                expected_value: value
+            })
         }
         Component::AttrPrefixMatch(ref attr, ref value) => {
-            element.match_attr_prefix(attr, value)
+            let (ns, n) = unpack_attr_selector(element, attr);
+            element.attr_matches(ns, n, &AttrSelectorOperation::WithValue {
+                operator: AttrSelectorOperator::Prefix,
+                case_sensitivity: CaseSensitivity::CaseSensitive,
+                expected_value: value
+            })
         }
         Component::AttrSubstringMatch(ref attr, ref value) => {
-            element.match_attr_substring(attr, value)
+            let (ns, n) = unpack_attr_selector(element, attr);
+            element.attr_matches(ns, n, &AttrSelectorOperation::WithValue {
+                operator: AttrSelectorOperator::Substring,
+                case_sensitivity: CaseSensitivity::CaseSensitive,
+                expected_value: value
+            })
         }
         Component::AttrSuffixMatch(ref attr, ref value) => {
-            element.match_attr_suffix(attr, value)
+            let (ns, n) = unpack_attr_selector(element, attr);
+            element.attr_matches(ns, n, &AttrSelectorOperation::WithValue {
+                operator: AttrSelectorOperator::Suffix,
+                case_sensitivity: CaseSensitivity::CaseSensitive,
+                expected_value: value
+            })
         }
         Component::AttrIncludesNeverMatch(..) |
         Component::AttrPrefixNeverMatch(..) |
@@ -489,6 +519,18 @@ fn matches_simple_selector<E, F>(
     }
 }
 
+fn unpack_attr_selector<'a, E>(element: &E, attr: &'a AttrSelector<E::Impl>)
+                               -> (&'a NamespaceConstraint<E::Impl>,
+                                   &'a <E::Impl as SelectorImpl>::LocalName)
+where E: Element {
+    let name = if element.is_html_element_in_html_document() {
+        &attr.lower_name
+    } else {
+        &attr.name
+    };
+    (&attr.namespace, name)
+}
+
 #[inline]
 fn matches_generic_nth_child<E, F>(element: &E,
                                    a: i32,

components/selectors/parser.rs

@@ -3,6 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use arcslice::ArcSlice;
+use attr::{CaseSensitivity, SELECTOR_WHITESPACE};
 use cssparser::{Token, Parser as CssParser, parse_nth, ToCss, serialize_identifier, CssStringWriter};
 use precomputed_hash::PrecomputedHash;
 use smallvec::SmallVec;
@@ -13,7 +14,6 @@ use std::fmt::{self, Display, Debug, Write};
 use std::iter::Rev;
 use std::ops::Add;
 use std::slice;
-use tree::SELECTOR_WHITESPACE;
 use visitor::SelectorVisitor;
 
 /// A trait that represents a pseudo-element.
@@ -560,7 +560,6 @@ pub enum Component<Impl: SelectorImpl> {
     OnlyOfType,
     NonTSPseudoClass(Impl::NonTSPseudoClass),
     PseudoElement(Impl::PseudoElement),
-    // ...
 }
 
 impl<Impl: SelectorImpl> Component<Impl> {
@@ -605,13 +604,6 @@ impl<Impl: SelectorImpl> Component<Impl> {
     }
 }
 
-#[derive(Eq, PartialEq, Clone, Copy, Debug)]
-pub enum CaseSensitivity {
-    CaseSensitive,  // Selectors spec says language-defined, but HTML says sensitive.
-    AsciiCaseInsensitive,
-}
-
-
 #[derive(Eq, PartialEq, Clone)]
 pub struct LocalName<Impl: SelectorImpl> {
     pub name: Impl::LocalName,

components/selectors/tree.rs

@@ -5,122 +5,13 @@
 //! Traits that nodes must implement. Breaks the otherwise-cyclic dependency between layout and
 //! style.
 
+use attr::AttrSelectorOperation;
 use matching::{ElementSelectorFlags, MatchingContext};
-use parser::{AttrSelector, SelectorImpl};
+use parser::{NamespaceConstraint, SelectorImpl};
-use std::ascii::AsciiExt;
 
-/// The definition of whitespace per CSS Selectors Level 3 § 4.
+pub trait Element: Sized {
-pub static SELECTOR_WHITESPACE: &'static [char] = &[' ', '\t', '\n', '\r', '\x0C'];
-
-// Attribute matching routines. Consumers with simple implementations can implement
-// MatchAttrGeneric instead.
-pub trait MatchAttr {
     type Impl: SelectorImpl;
 
-    fn match_attr_has(
-        &self,
-        attr: &AttrSelector<Self::Impl>) -> bool;
-
-    fn match_attr_equals(
-        &self,
-        attr: &AttrSelector<Self::Impl>,
-        value: &<Self::Impl as SelectorImpl>::AttrValue) -> bool;
-
-    fn match_attr_equals_ignore_ascii_case(
-        &self,
-        attr: &AttrSelector<Self::Impl>,
-        value: &<Self::Impl as SelectorImpl>::AttrValue) -> bool;
-
-    fn match_attr_includes(
-        &self,
-        attr: &AttrSelector<Self::Impl>,
-        value: &<Self::Impl as SelectorImpl>::AttrValue) -> bool;
-
-    fn match_attr_dash(
-        &self,
-        attr: &AttrSelector<Self::Impl>,
-        value: &<Self::Impl as SelectorImpl>::AttrValue) -> bool;
-
-    fn match_attr_prefix(
-        &self,
-        attr: &AttrSelector<Self::Impl>,
-        value: &<Self::Impl as SelectorImpl>::AttrValue) -> bool;
-
-    fn match_attr_substring(
-        &self,
-        attr: &AttrSelector<Self::Impl>,
-        value: &<Self::Impl as SelectorImpl>::AttrValue) -> bool;
-
-    fn match_attr_suffix(
-        &self,
-        attr: &AttrSelector<Self::Impl>,
-        value: &<Self::Impl as SelectorImpl>::AttrValue) -> bool;
-}
-
-pub trait MatchAttrGeneric {
-    type Impl: SelectorImpl;
-    fn match_attr<F>(&self, attr: &AttrSelector<Self::Impl>, test: F) -> bool where F: Fn(&str) -> bool;
-}
-
-impl<T> MatchAttr for T where T: MatchAttrGeneric, T::Impl: SelectorImpl<AttrValue = String> {
-    type Impl = T::Impl;
-
-    fn match_attr_has(&self, attr: &AttrSelector<Self::Impl>) -> bool {
-        self.match_attr(attr, |_| true)
-    }
-
-    fn match_attr_equals(&self, attr: &AttrSelector<Self::Impl>, value: &String) -> bool {
-        self.match_attr(attr, |v| v == value)
-    }
-
-    fn match_attr_equals_ignore_ascii_case(&self, attr: &AttrSelector<Self::Impl>,
-                                           value: &String) -> bool {
-        self.match_attr(attr, |v| v.eq_ignore_ascii_case(value))
-    }
-
-    fn match_attr_includes(&self, attr: &AttrSelector<Self::Impl>, value: &String) -> bool {
-        self.match_attr(attr, |attr_value| {
-            attr_value.split(SELECTOR_WHITESPACE).any(|v| v == value)
-        })
-    }
-
-    fn match_attr_dash(&self, attr: &AttrSelector<Self::Impl>, value: &String) -> bool {
-        self.match_attr(attr, |attr_value| {
-            // The attribute must start with the pattern.
-            if !attr_value.starts_with(value) {
-                return false
-            }
-
-            // If the strings are the same, we're done.
-            if attr_value.len() == value.len() {
-                return true
-            }
-
-            // The attribute is long than the pattern, so the next character must be '-'.
-            attr_value.as_bytes()[value.len()] == '-' as u8
-        })
-    }
-
-    fn match_attr_prefix(&self, attr: &AttrSelector<Self::Impl>, value: &String) -> bool {
-        self.match_attr(attr, |attr_value| {
-            attr_value.starts_with(value)
-        })
-    }
-
-    fn match_attr_substring(&self, attr: &AttrSelector<Self::Impl>, value: &String) -> bool {
-        self.match_attr(attr, |attr_value| {
-            attr_value.contains(value)
-        })
-    }
-
-    fn match_attr_suffix(&self, attr: &AttrSelector<Self::Impl>, value: &String) -> bool {
-        self.match_attr(attr, |attr_value| {
-            attr_value.ends_with(value)
-        })
-    }
-}
-
-pub trait Element: MatchAttr + Sized {
     fn parent_element(&self) -> Option<Self>;
 
     /// The parent of a given pseudo-element, after matching a pseudo-element
@@ -147,6 +38,12 @@ pub trait Element: MatchAttr + Sized {
     fn get_local_name(&self) -> &<Self::Impl as SelectorImpl>::BorrowedLocalName;
     fn get_namespace(&self) -> &<Self::Impl as SelectorImpl>::BorrowedNamespaceUrl;
 
+    fn attr_matches(&self,
+                    ns: &NamespaceConstraint<Self::Impl>,
+                    local_name: &<Self::Impl as SelectorImpl>::LocalName,
+                    operation: &AttrSelectorOperation<Self::Impl>)
+                    -> bool;
+
     fn match_non_ts_pseudo_class<F>(&self,
                                     pc: &<Self::Impl as SelectorImpl>::NonTSPseudoClass,
                                     context: &mut MatchingContext,

components/style/attr.rs

@@ -12,6 +12,8 @@ use cssparser::{self, Color, RGBA};
 use euclid::num::Zero;
 use num_traits::ToPrimitive;
 use properties::PropertyDeclarationBlock;
+use selector_parser::SelectorImpl;
+use selectors::attr::AttrSelectorOperation;
 use servo_url::ServoUrl;
 use shared_lock::Locked;
 use std::ascii::AsciiExt;
@@ -349,6 +351,12 @@ impl AttrValue {
             panic!("Uint not found");
         }
     }
+
+    pub fn eval_selector(&self, selector: &AttrSelectorOperation<SelectorImpl>) -> bool {
+        // FIXME(SimonSapin) this can be more efficient by matching on `(self, selector)` variants
+        // and doing Atom comparisons instead of string comparisons where possible.
+        selector.eval_str(self)
+    }
 }
 
 impl ::std::ops::Deref for AttrValue {

components/style/gecko/snapshot.rs

@@ -8,14 +8,15 @@
 use dom::TElement;
 use element_state::ElementState;
 use gecko::snapshot_helpers;
-use gecko::wrapper::{AttrSelectorHelpers, GeckoElement};
+use gecko::wrapper::{NamespaceConstraintHelpers, GeckoElement};
 use gecko_bindings::bindings;
 use gecko_bindings::structs::ServoElementSnapshot;
 use gecko_bindings::structs::ServoElementSnapshotFlags as Flags;
 use gecko_bindings::structs::ServoElementSnapshotTable;
 use restyle_hints::ElementSnapshot;
 use selector_parser::SelectorImpl;
-use selectors::parser::AttrSelector;
+use selectors::attr::{AttrSelectorOperation, AttrSelectorOperator, CaseSensitivity};
+use selectors::parser::NamespaceConstraint;
 use string_cache::Atom;
 
 /// A snapshot of a Gecko element.
@@ -47,11 +48,6 @@ impl SnapshotMap {
 }
 
 impl GeckoElementSnapshot {
-    #[inline]
-    fn is_html_element_in_html_document(&self) -> bool {
-        self.mIsHTMLElementInHTMLDocument
-    }
-
     #[inline]
     fn has_any(&self, flags: Flags) -> bool {
         (self.mContains as u8 & flags as u8) != 0
@@ -60,76 +56,67 @@ impl GeckoElementSnapshot {
     fn as_ptr(&self) -> *const Self {
         self
     }
-}
 
-impl ::selectors::MatchAttr for GeckoElementSnapshot {
+    /// selectors::Element::attr_matches
-    type Impl = SelectorImpl;
+    pub fn attr_matches(&self,
-
+                        ns: &NamespaceConstraint<SelectorImpl>,
-    fn match_attr_has(&self, attr: &AttrSelector<SelectorImpl>) -> bool {
+                        local_name: &Atom,
+                        operation: &AttrSelectorOperation<SelectorImpl>)
+                        -> bool {
         unsafe {
-            bindings::Gecko_SnapshotHasAttr(self,
+            match *operation {
-                                            attr.ns_or_null(),
+                AttrSelectorOperation::Exists => {
-                                            attr.select_name(self.is_html_element_in_html_document()))
+                    bindings:: Gecko_SnapshotHasAttr(self,
-        }
+                                                     ns.atom_or_null(),
-    }
+                                                     local_name.as_ptr())
-
+                }
-    fn match_attr_equals(&self, attr: &AttrSelector<SelectorImpl>, value: &Atom) -> bool {
+                AttrSelectorOperation::WithValue { operator, case_sensitivity, expected_value } => {
-        unsafe {
+                    let ignore_case = match case_sensitivity {
-            bindings::Gecko_SnapshotAttrEquals(self,
+                        CaseSensitivity::CaseSensitive => false,
-                                               attr.ns_or_null(),
+                        CaseSensitivity::AsciiCaseInsensitive => true,
-                                               attr.select_name(self.is_html_element_in_html_document()),
+                    };
-                                               value.as_ptr(),
+                    // FIXME: case sensitivity for operators other than Equal
-                                               /* ignoreCase = */ false)
+                    match operator {
-        }
+                        AttrSelectorOperator::Equal => bindings::Gecko_SnapshotAttrEquals(
-    }
+                            self,
-
+                            ns.atom_or_null(),
-    fn match_attr_equals_ignore_ascii_case(&self, attr: &AttrSelector<SelectorImpl>, value: &Atom) -> bool {
+                            local_name.as_ptr(),
-        unsafe {
+                            expected_value.as_ptr(),
-            bindings::Gecko_SnapshotAttrEquals(self,
+                            ignore_case
-                                               attr.ns_or_null(),
+                        ),
-                                               attr.select_name(self.is_html_element_in_html_document()),
+                        AttrSelectorOperator::Includes => bindings::Gecko_SnapshotAttrIncludes(
-                                               value.as_ptr(),
+                            self,
-                                               /* ignoreCase = */ true)
+                            ns.atom_or_null(),
-        }
+                            local_name.as_ptr(),
-    }
+                            expected_value.as_ptr(),
-    fn match_attr_includes(&self, attr: &AttrSelector<SelectorImpl>, value: &Atom) -> bool {
+                        ),
-        unsafe {
+                        AttrSelectorOperator::DashMatch => bindings::Gecko_SnapshotAttrDashEquals(
-            bindings::Gecko_SnapshotAttrIncludes(self,
+                            self,
-                                                 attr.ns_or_null(),
+                            ns.atom_or_null(),
-                                                 attr.select_name(self.is_html_element_in_html_document()),
+                            local_name.as_ptr(),
-                                                 value.as_ptr())
+                            expected_value.as_ptr(),
-        }
+                        ),
-    }
+                        AttrSelectorOperator::Prefix => bindings::Gecko_SnapshotAttrHasPrefix(
-    fn match_attr_dash(&self, attr: &AttrSelector<SelectorImpl>, value: &Atom) -> bool {
+                            self,
-        unsafe {
+                            ns.atom_or_null(),
-            bindings::Gecko_SnapshotAttrDashEquals(self,
+                            local_name.as_ptr(),
-                                                   attr.ns_or_null(),
+                            expected_value.as_ptr(),
-                                                   attr.select_name(self.is_html_element_in_html_document()),
+                        ),
-                                                   value.as_ptr())
+                        AttrSelectorOperator::Suffix => bindings::Gecko_SnapshotAttrHasSuffix(
-        }
+                            self,
-    }
+                            ns.atom_or_null(),
-    fn match_attr_prefix(&self, attr: &AttrSelector<SelectorImpl>, value: &Atom) -> bool {
+                            local_name.as_ptr(),
-        unsafe {
+                            expected_value.as_ptr(),
-            bindings::Gecko_SnapshotAttrHasPrefix(self,
+                        ),
-                                                  attr.ns_or_null(),
+                        AttrSelectorOperator::Substring => bindings::Gecko_SnapshotAttrHasSubstring(
-                                                  attr.select_name(self.is_html_element_in_html_document()),
+                            self,
-                                                  value.as_ptr())
+                            ns.atom_or_null(),
-        }
+                            local_name.as_ptr(),
-    }
+                            expected_value.as_ptr(),
-    fn match_attr_substring(&self, attr: &AttrSelector<SelectorImpl>, value: &Atom) -> bool {
+                        ),
-        unsafe {
+                    }
-            bindings::Gecko_SnapshotAttrHasSubstring(self,
+                }
-                                                     attr.ns_or_null(),
+            }
-                                                     attr.select_name(self.is_html_element_in_html_document()),
-                                                     value.as_ptr())
-        }
-    }
-    fn match_attr_suffix(&self, attr: &AttrSelector<SelectorImpl>, value: &Atom) -> bool {
-        unsafe {
-            bindings::Gecko_SnapshotAttrHasSuffix(self,
-                                                  attr.ns_or_null(),
-                                                  attr.select_name(self.is_html_element_in_html_document()),
-                                                  value.as_ptr())
         }
     }
 }

components/style/gecko/wrapper.rs

@@ -64,8 +64,9 @@ use properties::style_structs::Font;
 use rule_tree::CascadeLevel as ServoCascadeLevel;
 use selector_parser::ElementExt;
 use selectors::Element;
+use selectors::attr::{AttrSelectorOperation, AttrSelectorOperator, CaseSensitivity};
 use selectors::matching::{ElementSelectorFlags, MatchingContext, MatchingMode};
-use selectors::parser::{AttrSelector, NamespaceConstraint};
+use selectors::parser::NamespaceConstraint;
 use shared_lock::Locked;
 use sink::Push;
 use std::cell::RefCell;
@@ -1082,6 +1083,8 @@ impl<'le> PresentationalHintsSynthesizer for GeckoElement<'le> {
 }
 
 impl<'le> ::selectors::Element for GeckoElement<'le> {
+    type Impl = SelectorImpl;
+
     fn parent_element(&self) -> Option<Self> {
         let parent_node = self.as_node().parent_node();
         parent_node.and_then(|n| n.as_element())
@@ -1136,6 +1139,68 @@ impl<'le> ::selectors::Element for GeckoElement<'le> {
         None
     }
 
+    fn attr_matches(&self,
+                    ns: &NamespaceConstraint<SelectorImpl>,
+                    local_name: &Atom,
+                    operation: &AttrSelectorOperation<SelectorImpl>)
+                    -> bool {
+        unsafe {
+            match *operation {
+                AttrSelectorOperation::Exists => {
+                    bindings::Gecko_HasAttr(self.0,
+                                            ns.atom_or_null(),
+                                            local_name.as_ptr())
+                }
+                AttrSelectorOperation::WithValue { operator, case_sensitivity, expected_value } => {
+                    let ignore_case = match case_sensitivity {
+                        CaseSensitivity::CaseSensitive => false,
+                        CaseSensitivity::AsciiCaseInsensitive => true,
+                    };
+                    // FIXME: case sensitivity for operators other than Equal
+                    match operator {
+                        AttrSelectorOperator::Equal => bindings::Gecko_AttrEquals(
+                            self.0,
+                            ns.atom_or_null(),
+                            local_name.as_ptr(),
+                            expected_value.as_ptr(),
+                            ignore_case
+                        ),
+                        AttrSelectorOperator::Includes => bindings::Gecko_AttrIncludes(
+                            self.0,
+                            ns.atom_or_null(),
+                            local_name.as_ptr(),
+                            expected_value.as_ptr(),
+                        ),
+                        AttrSelectorOperator::DashMatch => bindings::Gecko_AttrDashEquals(
+                            self.0,
+                            ns.atom_or_null(),
+                            local_name.as_ptr(),
+                            expected_value.as_ptr(),
+                        ),
+                        AttrSelectorOperator::Prefix => bindings::Gecko_AttrHasPrefix(
+                            self.0,
+                            ns.atom_or_null(),
+                            local_name.as_ptr(),
+                            expected_value.as_ptr(),
+                        ),
+                        AttrSelectorOperator::Suffix => bindings::Gecko_AttrHasSuffix(
+                            self.0,
+                            ns.atom_or_null(),
+                            local_name.as_ptr(),
+                            expected_value.as_ptr(),
+                        ),
+                        AttrSelectorOperator::Substring => bindings::Gecko_AttrHasSubstring(
+                            self.0,
+                            ns.atom_or_null(),
+                            local_name.as_ptr(),
+                            expected_value.as_ptr(),
+                        ),
+                    }
+                }
+            }
+        }
+    }
+
     fn is_root(&self) -> bool {
         unsafe {
             Gecko_IsRootElement(self.0)
@@ -1343,99 +1408,18 @@ impl<'le> ::selectors::Element for GeckoElement<'le> {
 }
 
 /// A few helpers to help with attribute selectors and snapshotting.
-pub trait AttrSelectorHelpers {
+pub trait NamespaceConstraintHelpers {
     /// Returns the namespace of the selector, or null otherwise.
-    fn ns_or_null(&self) -> *mut nsIAtom;
+    fn atom_or_null(&self) -> *mut nsIAtom;
-    /// Returns the proper selector name depending on whether the requesting
-    /// element is an HTML element in an HTML document or not.
-    fn select_name(&self, is_html_element_in_html_document: bool) -> *mut nsIAtom;
 }
 
-impl AttrSelectorHelpers for AttrSelector<SelectorImpl> {
+impl NamespaceConstraintHelpers for NamespaceConstraint<SelectorImpl> {
-    fn ns_or_null(&self) -> *mut nsIAtom {
+    fn atom_or_null(&self) -> *mut nsIAtom {
-        match self.namespace {
+        match *self {
             NamespaceConstraint::Any => ptr::null_mut(),
             NamespaceConstraint::Specific(ref ns) => ns.url.0.as_ptr(),
         }
     }
-
-    fn select_name(&self, is_html_element_in_html_document: bool) -> *mut nsIAtom {
-        if is_html_element_in_html_document {
-            self.lower_name.as_ptr()
-        } else {
-            self.name.as_ptr()
-        }
-    }
-}
-
-impl<'le> ::selectors::MatchAttr for GeckoElement<'le> {
-    type Impl = SelectorImpl;
-
-    fn match_attr_has(&self, attr: &AttrSelector<Self::Impl>) -> bool {
-        unsafe {
-            bindings::Gecko_HasAttr(self.0,
-                                    attr.ns_or_null(),
-                                    attr.select_name(self.is_html_element_in_html_document()))
-        }
-    }
-    fn match_attr_equals(&self, attr: &AttrSelector<Self::Impl>, value: &Atom) -> bool {
-        unsafe {
-            bindings::Gecko_AttrEquals(self.0,
-                                       attr.ns_or_null(),
-                                       attr.select_name(self.is_html_element_in_html_document()),
-                                       value.as_ptr(),
-                                       /* ignoreCase = */ false)
-        }
-    }
-    fn match_attr_equals_ignore_ascii_case(&self, attr: &AttrSelector<Self::Impl>, value: &Atom) -> bool {
-        unsafe {
-            bindings::Gecko_AttrEquals(self.0,
-                                       attr.ns_or_null(),
-                                       attr.select_name(self.is_html_element_in_html_document()),
-                                       value.as_ptr(),
-                                       /* ignoreCase = */ true)
-        }
-    }
-    fn match_attr_includes(&self, attr: &AttrSelector<Self::Impl>, value: &Atom) -> bool {
-        unsafe {
-            bindings::Gecko_AttrIncludes(self.0,
-                                         attr.ns_or_null(),
-                                         attr.select_name(self.is_html_element_in_html_document()),
-                                         value.as_ptr())
-        }
-    }
-    fn match_attr_dash(&self, attr: &AttrSelector<Self::Impl>, value: &Atom) -> bool {
-        unsafe {
-            bindings::Gecko_AttrDashEquals(self.0,
-                                           attr.ns_or_null(),
-                                           attr.select_name(self.is_html_element_in_html_document()),
-                                           value.as_ptr())
-        }
-    }
-    fn match_attr_prefix(&self, attr: &AttrSelector<Self::Impl>, value: &Atom) -> bool {
-        unsafe {
-            bindings::Gecko_AttrHasPrefix(self.0,
-                                          attr.ns_or_null(),
-                                          attr.select_name(self.is_html_element_in_html_document()),
-                                          value.as_ptr())
-        }
-    }
-    fn match_attr_substring(&self, attr: &AttrSelector<Self::Impl>, value: &Atom) -> bool {
-        unsafe {
-            bindings::Gecko_AttrHasSubstring(self.0,
-                                             attr.ns_or_null(),
-                                             attr.select_name(self.is_html_element_in_html_document()),
-                                             value.as_ptr())
-        }
-    }
-    fn match_attr_suffix(&self, attr: &AttrSelector<Self::Impl>, value: &Atom) -> bool {
-        unsafe {
-            bindings::Gecko_AttrHasSuffix(self.0,
-                                          attr.ns_or_null(),
-                                          attr.select_name(self.is_html_element_in_html_document()),
-                                          value.as_ptr())
-        }
-    }
 }
 
 impl<'le> ElementExt for GeckoElement<'le> {

components/style/restyle_hints.rs

@@ -7,18 +7,20 @@
 #![deny(missing_docs)]
 
 use Atom;
+use LocalName;
 use dom::TElement;
 use element_state::*;
 #[cfg(feature = "gecko")]
 use gecko_bindings::structs::nsRestyleHint;
 #[cfg(feature = "servo")]
 use heapsize::HeapSizeOf;
-use selector_parser::{AttrValue, NonTSPseudoClass, PseudoElement, SelectorImpl, Snapshot, SnapshotMap};
+use selector_parser::{NonTSPseudoClass, PseudoElement, SelectorImpl, Snapshot, SnapshotMap};
-use selectors::{Element, MatchAttr};
+use selectors::Element;
+use selectors::attr::AttrSelectorOperation;
 use selectors::matching::{ElementSelectorFlags, MatchingContext, MatchingMode};
 use selectors::matching::matches_selector;
-use selectors::parser::{AttrSelector, Combinator, Component, Selector};
+use selectors::parser::{Combinator, Component, Selector};
-use selectors::parser::{SelectorInner, SelectorMethods};
+use selectors::parser::{SelectorInner, SelectorMethods, NamespaceConstraint};
 use selectors::visitor::SelectorVisitor;
 use smallvec::SmallVec;
 use std::borrow::Borrow;
@@ -170,7 +172,7 @@ impl HeapSizeOf for RestyleHint {
 /// still need to take the ElementWrapper approach for attribute-dependent
 /// style. So we do it the same both ways for now to reduce complexity, but it's
 /// worth measuring the performance impact (if any) of the mStateMask approach.
-pub trait ElementSnapshot : Sized + MatchAttr<Impl=SelectorImpl> {
+pub trait ElementSnapshot : Sized {
     /// The state of the snapshot, if any.
     fn state(&self) -> Option<ElementState>;
 
@@ -242,90 +244,6 @@ impl<'a, E> ElementWrapper<'a, E>
     }
 }
 
-impl<'a, E> MatchAttr for ElementWrapper<'a, E>
-    where E: TElement,
-{
-    type Impl = SelectorImpl;
-
-    fn match_attr_has(&self, attr: &AttrSelector<SelectorImpl>) -> bool {
-        match self.snapshot() {
-            Some(snapshot) if snapshot.has_attrs()
-                => snapshot.match_attr_has(attr),
-            _   => self.element.match_attr_has(attr)
-        }
-    }
-
-    fn match_attr_equals(&self,
-                         attr: &AttrSelector<SelectorImpl>,
-                         value: &AttrValue) -> bool {
-        match self.snapshot() {
-            Some(snapshot) if snapshot.has_attrs()
-                => snapshot.match_attr_equals(attr, value),
-            _   => self.element.match_attr_equals(attr, value)
-        }
-    }
-
-    fn match_attr_equals_ignore_ascii_case(&self,
-                                           attr: &AttrSelector<SelectorImpl>,
-                                           value: &AttrValue) -> bool {
-        match self.snapshot() {
-            Some(snapshot) if snapshot.has_attrs()
-                => snapshot.match_attr_equals_ignore_ascii_case(attr, value),
-            _   => self.element.match_attr_equals_ignore_ascii_case(attr, value)
-        }
-    }
-
-    fn match_attr_includes(&self,
-                           attr: &AttrSelector<SelectorImpl>,
-                           value: &AttrValue) -> bool {
-        match self.snapshot() {
-            Some(snapshot) if snapshot.has_attrs()
-                => snapshot.match_attr_includes(attr, value),
-            _   => self.element.match_attr_includes(attr, value)
-        }
-    }
-
-    fn match_attr_dash(&self,
-                       attr: &AttrSelector<SelectorImpl>,
-                       value: &AttrValue) -> bool {
-        match self.snapshot() {
-            Some(snapshot) if snapshot.has_attrs()
-                => snapshot.match_attr_dash(attr, value),
-            _   => self.element.match_attr_dash(attr, value)
-        }
-    }
-
-    fn match_attr_prefix(&self,
-                         attr: &AttrSelector<SelectorImpl>,
-                         value: &AttrValue) -> bool {
-        match self.snapshot() {
-            Some(snapshot) if snapshot.has_attrs()
-                => snapshot.match_attr_prefix(attr, value),
-            _   => self.element.match_attr_prefix(attr, value)
-        }
-    }
-
-    fn match_attr_substring(&self,
-                            attr: &AttrSelector<SelectorImpl>,
-                            value: &AttrValue) -> bool {
-        match self.snapshot() {
-            Some(snapshot) if snapshot.has_attrs()
-                => snapshot.match_attr_substring(attr, value),
-            _   => self.element.match_attr_substring(attr, value)
-        }
-    }
-
-    fn match_attr_suffix(&self,
-                         attr: &AttrSelector<SelectorImpl>,
-                         value: &AttrValue) -> bool {
-        match self.snapshot() {
-            Some(snapshot) if snapshot.has_attrs()
-                => snapshot.match_attr_suffix(attr, value),
-            _   => self.element.match_attr_suffix(attr, value)
-        }
-    }
-}
-
 #[cfg(feature = "gecko")]
 fn dir_selector_to_state(s: &[u16]) -> ElementState {
     // Jump through some hoops to deal with our Box<[u16]> thing.
@@ -345,6 +263,8 @@ fn dir_selector_to_state(s: &[u16]) -> ElementState {
 impl<'a, E> Element for ElementWrapper<'a, E>
     where E: TElement,
 {
+    type Impl = SelectorImpl;
+
     fn match_non_ts_pseudo_class<F>(&self,
                                     pseudo_class: &NonTSPseudoClass,
                                     context: &mut MatchingContext,
@@ -450,6 +370,19 @@ impl<'a, E> Element for ElementWrapper<'a, E>
         self.element.get_namespace()
     }
 
+    fn attr_matches(&self,
+                    ns: &NamespaceConstraint<Self::Impl>,
+                    local_name: &LocalName,
+                    operation: &AttrSelectorOperation<Self::Impl>)
+                    -> bool {
+        match self.snapshot() {
+            Some(snapshot) if snapshot.has_attrs() => {
+                snapshot.attr_matches(ns, local_name, operation)
+            }
+            _ => self.element.attr_matches(ns, local_name, operation)
+        }
+    }
+
     fn get_id(&self) -> Option<Atom> {
         match self.snapshot() {
             Some(snapshot) if snapshot.has_attrs()

components/style/servo/selector_parser.rs

@@ -14,9 +14,10 @@ use element_state::ElementState;
 use fnv::FnvHashMap;
 use restyle_hints::ElementSnapshot;
 use selector_parser::{ElementExt, PseudoElementCascadeType, SelectorParser};
-use selectors::{Element, MatchAttrGeneric};
+use selectors::{Element};
+use selectors::attr::AttrSelectorOperation;
 use selectors::matching::{MatchingContext, MatchingMode};
-use selectors::parser::{AttrSelector, SelectorMethods};
+use selectors::parser::{SelectorMethods, NamespaceConstraint};
 use selectors::visitor::SelectorVisitor;
 use std::borrow::Cow;
 use std::fmt;
@@ -519,10 +520,10 @@ impl ServoElementSnapshot {
             .map(|&(_, ref v)| v)
     }
 
-    fn get_attr_ignore_ns(&self, name: &LocalName) -> Option<&AttrValue> {
+    fn any_attr_ignore_ns<F>(&self, name: &LocalName, mut f: F) -> bool
+    where F: FnMut(&AttrValue) -> bool {
         self.attrs.as_ref().unwrap().iter()
-                  .find(|&&(ref ident, _)| ident.local_name == *name)
+                  .any(|&(ref ident, ref v)| ident.local_name == *name && f(v))
-                  .map(|&(_, ref v)| v)
     }
 }
 
@@ -555,19 +556,23 @@ impl ElementSnapshot for ServoElementSnapshot {
     }
 }
 
-impl MatchAttrGeneric for ServoElementSnapshot {
+impl ServoElementSnapshot {
-    type Impl = SelectorImpl;
+    /// selectors::Element::attr_matches
-
+    pub fn attr_matches(&self,
-    fn match_attr<F>(&self, attr: &AttrSelector<SelectorImpl>, test: F) -> bool
+                        ns: &NamespaceConstraint<SelectorImpl>,
-        where F: Fn(&str) -> bool
+                        local_name: &LocalName,
-    {
+                        operation: &AttrSelectorOperation<SelectorImpl>)
+                        -> bool {
         use selectors::parser::NamespaceConstraint;
-        let html = self.is_html_element_in_html_document;
+        match *ns {
-        let local_name = if html { &attr.lower_name } else { &attr.name };
+            NamespaceConstraint::Specific(ref ns) => {
-        match attr.namespace {
+                self.get_attr(&ns.url, local_name)
-            NamespaceConstraint::Specific(ref ns) => self.get_attr(&ns.url, local_name),
+                    .map_or(false, |value| value.eval_selector(operation))
-            NamespaceConstraint::Any => self.get_attr_ignore_ns(local_name),
+            }
-        }.map_or(false, |v| test(v))
+            NamespaceConstraint::Any => {
+                self.any_attr_ignore_ns(local_name, |value| value.eval_selector(operation))
+            }
+        }
     }
 }
 

servo-tidy.toml

@@ -56,9 +56,6 @@ files = [
   "./tests/wpt/mozilla/tests/css/fonts",
   "./tests/wpt/mozilla/tests/css/pre_with_tab.html",
   "./tests/wpt/mozilla/tests/mozilla/textarea_placeholder.html",
-  # Tidy complains about taking &String instead of &str, but they aren't
-  # equivalent given the way the traits are set up.
-  "./components/selectors/tree.rs",
 ]
 # Directories that are ignored for the non-WPT tidy check.
 directories = [

tests/unit/style/stylesheets.rs

@@ -6,6 +6,7 @@ use cssparser::{self, Parser as CssParser, SourcePosition, SourceLocation};
 use html5ever::{Namespace as NsAtom};
 use media_queries::CSSErrorReporterTest;
 use parking_lot::RwLock;
+use selectors::attr::CaseSensitivity;
 use selectors::parser::*;
 use servo_atoms::Atom;
 use servo_url::ServoUrl;