Fix case sensitivity of local name selectors.

src/components/layout/wrapper.rs

@@ -265,13 +265,11 @@ impl<'ln> TNode<LayoutElement<'ln>> for LayoutNode<'ln> {
     }
 
     fn match_attr(&self, attr: &AttrSelector, test: |&str| -> bool) -> bool {
-        let name = unsafe {
-            let element: JS<Element> = self.node.transmute_copy();
-            if element.html_element_in_html_document_for_layout() {
-                attr.lower_name.as_slice()
-            } else {
-                attr.name.as_slice()
-            }
+        assert!(self.is_element())
+        let name = if self.is_html_element_in_html_document() {
+            attr.lower_name.as_slice()
+        } else {
+            attr.name.as_slice()
         };
         match attr.namespace {
             SpecificNamespace(ref ns) => {
@@ -283,6 +281,15 @@ impl<'ln> TNode<LayoutElement<'ln>> for LayoutNode<'ln> {
             AnyNamespace => false,
         }
     }
+
+    fn is_html_element_in_html_document(&self) -> bool {
+        unsafe {
+            self.is_element() && {
+                let element: JS<Element> = self.node.transmute_copy();
+                element.html_element_in_html_document_for_layout()
+            }
+        }
+    }
 }
 
 pub struct LayoutNodeChildrenIterator<'a> {

src/components/script/dom/node.rs

@@ -1997,29 +1997,32 @@ impl<'a> style::TNode<JSRef<'a, Element>> for JSRef<'a, Node> {
     fn parent_node(&self) -> Option<JSRef<'a, Node>> {
         (self as &NodeHelpers).parent_node().map(|node| *node.root())
     }
+
     fn prev_sibling(&self) -> Option<JSRef<'a, Node>> {
         (self as &NodeHelpers).prev_sibling().map(|node| *node.root())
     }
+
     fn next_sibling(&self) -> Option<JSRef<'a, Node>> {
         (self as &NodeHelpers).next_sibling().map(|node| *node.root())
     }
+
     fn is_document(&self) -> bool {
         (self as &NodeHelpers).is_document()
     }
+
     fn is_element(&self) -> bool {
         (self as &NodeHelpers).is_element()
     }
+
     fn as_element(&self) -> JSRef<'a, Element> {
         let elem: Option<&JSRef<'a, Element>> = ElementCast::to_ref(self);
         assert!(elem.is_some());
         *elem.unwrap()
     }
+
     fn match_attr(&self, attr: &style::AttrSelector, test: |&str| -> bool) -> bool {
         let name = {
-            let elem: Option<&JSRef<'a, Element>> = ElementCast::to_ref(self);
-            assert!(elem.is_some());
-            let elem: &ElementHelpers = elem.unwrap() as &ElementHelpers;
-            if elem.html_element_in_html_document() {
+            if self.is_html_element_in_html_document() {
                 attr.lower_name.as_slice()
             } else {
                 attr.name.as_slice()
@@ -2034,6 +2037,13 @@ impl<'a> style::TNode<JSRef<'a, Element>> for JSRef<'a, Node> {
             style::AnyNamespace => false,
         }
     }
+
+    fn is_html_element_in_html_document(&self) -> bool {
+        let elem: Option<&JSRef<'a, Element>> = ElementCast::to_ref(self);
+        assert!(elem.is_some());
+        let elem: &ElementHelpers = elem.unwrap() as &ElementHelpers;
+        elem.html_element_in_html_document()
+    }
 }
 
 pub trait DisabledStateHelpers {

src/components/style/node.rs

@@ -18,6 +18,7 @@ pub trait TNode<E:TElement> : Clone {
     fn is_element(&self) -> bool;
     fn as_element(&self) -> E;
     fn match_attr(&self, attr: &AttrSelector, test: |&str| -> bool) -> bool;
+    fn is_html_element_in_html_document(&self) -> bool;
 }
 
 pub trait TElement {

src/components/style/selector_matching.rs

@@ -3,7 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use std::collections::hashmap::HashMap;
-use std::ascii::StrAsciiExt;
+use std::hash::Hash;
 use std::num::div_rem;
 use sync::Arc;
 
@@ -52,7 +52,10 @@ struct SelectorMap {
     // TODO: Tune the initial capacity of the HashMap
     id_hash: HashMap<Atom, Vec<Rule>>,
     class_hash: HashMap<Atom, Vec<Rule>>,
-    element_hash: HashMap<Atom, Vec<Rule>>,
+    local_name_hash: HashMap<Atom, Vec<Rule>>,
+    /// Same as local_name_hash, but keys are lower-cased.
+    /// For HTML elements in HTML documents.
+    lower_local_name_hash: HashMap<Atom, Vec<Rule>>,
     // For Rules that don't have ID, class, or element selectors.
     universal_rules: Vec<Rule>,
     /// Whether this hash is empty.
@@ -64,7 +67,8 @@ impl SelectorMap {
         SelectorMap {
             id_hash: HashMap::new(),
             class_hash: HashMap::new(),
-            element_hash: HashMap::new(),
+            local_name_hash: HashMap::new(),
+            lower_local_name_hash: HashMap::new(),
             universal_rules: vec!(),
             empty: true,
         }
@@ -113,13 +117,16 @@ impl SelectorMap {
             None => {}
         }
 
-        // HTML elements in HTML documents must be matched case-insensitively.
-        // TODO(pradeep): Case-sensitivity depends on the document type.
-        SelectorMap::get_matching_rules_from_hash_ignoring_case(node,
-                                                                &self.element_hash,
-                                                                element.get_local_name().as_slice(),
-                                                                matching_rules_list,
-                                                                shareable);
+        let local_name_hash = if node.is_html_element_in_html_document() {
+            &self.lower_local_name_hash
+        } else {
+            &self.local_name_hash
+        };
+        SelectorMap::get_matching_rules_from_hash(node,
+                                                  local_name_hash,
+                                                  element.get_local_name(),
+                                                  matching_rules_list,
+                                                  shareable);
 
         SelectorMap::get_matching_rules(node,
                                         self.universal_rules.as_slice(),
@@ -150,23 +157,6 @@ impl SelectorMap {
         }
     }
 
-    fn get_matching_rules_from_hash_ignoring_case<E:TElement,
-                                                  N:TNode<E>,
-                                                  V:VecLike<DeclarationBlock>>(
-                                                  node: &N,
-                                                  hash: &HashMap<Atom, Vec<Rule>>,
-                                                  key: &str,
-                                                  matching_rules: &mut V,
-                                                  shareable: &mut bool) {
-        // FIXME: Precache the lower case version as an atom.
-        match hash.find(&Atom::from_slice(key.to_ascii_lower().as_slice())) {
-            Some(rules) => {
-                SelectorMap::get_matching_rules(node, rules.as_slice(), matching_rules, shareable)
-            }
-            None => {}
-        }
-    }
-
     /// Adds rules in `rules` that match `node` to the `matching_rules` list.
     fn get_matching_rules<E:TElement,
                           N:TNode<E>,
@@ -183,49 +173,29 @@ impl SelectorMap {
     }
 
     /// Insert rule into the correct hash.
-    /// Order in which to try: id_hash, class_hash, element_hash, universal_rules.
+    /// Order in which to try: id_hash, class_hash, local_name_hash, universal_rules.
     fn insert(&mut self, rule: Rule) {
         self.empty = false;
 
         match SelectorMap::get_id_name(&rule) {
             Some(id_name) => {
-                match self.id_hash.find_mut(&id_name) {
-                    Some(rules) => {
-                        rules.push(rule);
-                        return;
-                    }
-                    None => {}
-                }
-                self.id_hash.insert(id_name, vec!(rule));
+                self.id_hash.find_push(id_name, rule);
                 return;
             }
             None => {}
         }
         match SelectorMap::get_class_name(&rule) {
             Some(class_name) => {
-                match self.class_hash.find_mut(&class_name) {
-                    Some(rules) => {
-                        rules.push(rule);
-                        return;
-                    }
-                    None => {}
-                }
-                self.class_hash.insert(class_name, vec!(rule));
+                self.class_hash.find_push(class_name, rule);
                 return;
             }
             None => {}
         }
 
-        match SelectorMap::get_element_name(&rule) {
-            Some(element_name) => {
-                match self.element_hash.find_mut(&element_name) {
-                    Some(rules) => {
-                        rules.push(rule);
-                        return;
-                    }
-                    None => {}
-                }
-                self.element_hash.insert(element_name, vec!(rule));
+        match SelectorMap::get_local_name(&rule) {
+            Some(LocalNameSelector { name, lower_name }) => {
+                self.local_name_hash.find_push(name, rule.clone());
+                self.lower_local_name_hash.find_push(lower_name, rule);
                 return;
             }
             None => {}
@@ -263,14 +233,12 @@ impl SelectorMap {
     }
 
     /// Retrieve the name if it is a type selector, or None otherwise.
-    fn get_element_name(rule: &Rule) -> Option<Atom> {
+    fn get_local_name(rule: &Rule) -> Option<LocalNameSelector> {
         let simple_selector_sequence = &rule.selector.simple_selectors;
         for ss in simple_selector_sequence.iter() {
             match *ss {
-                // HTML elements in HTML documents must be matched case-insensitively
-                // TODO: case-sensitivity depends on the document type
                 LocalNameSelector(ref name) => {
-                    return Some(Atom::from_slice(name.as_slice().to_ascii_lower().as_slice()));
+                    return Some(name.clone())
                 }
                 _ => {}
             }
@@ -629,11 +597,10 @@ fn matches_simple_selector<E:TElement,
                            shareable: &mut bool)
                            -> bool {
     match *selector {
-        // TODO: case-sensitivity depends on the document type
-        // TODO: intern element names
-        LocalNameSelector(ref name) => {
+        LocalNameSelector(LocalNameSelector { ref name, ref lower_name }) => {
+            let name = if element.is_html_element_in_html_document() { lower_name } else { name };
             let element = element.as_element();
-            element.get_local_name().as_slice().eq_ignore_ascii_case(name.as_slice())
+            element.get_local_name() == name
         }
 
         NamespaceSelector(ref namespace) => {
@@ -918,11 +885,30 @@ fn matches_last_child<E:TElement,N:TNode<E>>(element: &N) -> bool {
 }
 
 
+trait FindPush<K, V> {
+    fn find_push(&mut self, key: K, value: V);
+}
+
+impl<K: Eq + Hash, V> FindPush<K, V> for HashMap<K, Vec<V>> {
+    fn find_push(&mut self, key: K, value: V) {
+        match self.find_mut(&key) {
+            Some(vec) => {
+                vec.push(value);
+                return
+            }
+            None => {}
+        }
+        self.insert(key, vec![value]);
+    }
+}
+
+
 #[cfg(test)]
 mod tests {
     use servo_util::atom::Atom;
     use sync::Arc;
     use super::{DeclarationBlock, Rule, SelectorMap};
+    use selectors::LocalNameSelector;
 
     /// Helper method to get some Rules from selector strings.
     /// Each sublist of the result contains the Rules for one StyleRule.
@@ -971,12 +957,18 @@ mod tests {
     }
 
     #[test]
-    fn test_get_element_name(){
+    fn test_get_local_name(){
         let rules_list = get_mock_rules(["img.foo", "#top", "IMG", "ImG"]);
-        assert_eq!(SelectorMap::get_element_name(&rules_list[0][0]), Some(Atom::from_slice("img")));
-        assert_eq!(SelectorMap::get_element_name(&rules_list[1][0]), None);
-        assert_eq!(SelectorMap::get_element_name(&rules_list[2][0]), Some(Atom::from_slice("img")));
-        assert_eq!(SelectorMap::get_element_name(&rules_list[3][0]), Some(Atom::from_slice("img")));
+        let check = |i, names: Option<(&str, &str)>| {
+            assert!(SelectorMap::get_local_name(&rules_list[i][0])
+                    == names.map(|(name, lower_name)| LocalNameSelector {
+                            name: Atom::from_slice(name),
+                            lower_name: Atom::from_slice(lower_name) }))
+        };
+        check(0, Some(("img", "img")));
+        check(1, None);
+        check(2, Some(("IMG", "img")));
+        check(3, Some(("ImG", "img")));
     }
 
     #[test]

src/components/style/selectors.rs

@@ -3,7 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use std::{cmp, iter};
-use std::ascii::StrAsciiExt;
+use std::ascii::{StrAsciiExt, OwnedStrAsciiExt};
 use std::vec;
 use sync::Arc;
 
@@ -59,7 +59,7 @@ pub enum Combinator {
 pub enum SimpleSelector {
     IDSelector(Atom),
     ClassSelector(Atom),
-    LocalNameSelector(Atom),
+    LocalNameSelector(LocalNameSelector),
     NamespaceSelector(Namespace),
 
     // Attribute selectors
@@ -93,6 +93,12 @@ pub enum SimpleSelector {
     // ...
 }
 
+#[deriving(PartialEq, Clone)]
+pub struct LocalNameSelector {
+    pub name: Atom,
+    pub lower_name: Atom,
+}
+
 #[deriving(PartialEq, Clone)]
 pub struct AttrSelector {
     pub name: String,
@@ -268,8 +274,10 @@ fn parse_type_selector(iter: &mut Iter, namespaces: &NamespaceMap)
             }
             match local_name {
                 Some(name) => {
-                    let name_atom = Atom::from_slice(name.as_slice());
-                    simple_selectors.push(LocalNameSelector(name_atom))
+                    simple_selectors.push(LocalNameSelector(LocalNameSelector {
+                        name: Atom::from_slice(name.as_slice()),
+                        lower_name: Atom::from_slice(name.into_ascii_lower().as_slice())
+                    }))
                 }
                 None => (),
             }
@@ -574,9 +582,11 @@ mod tests {
     #[test]
     fn test_parsing() {
         assert!(parse("") == Err(()))
-        assert!(parse("e") == Ok(vec!(Selector {
+        assert!(parse("EeÉ") == Ok(vec!(Selector {
             compound_selectors: Arc::new(CompoundSelector {
-                simple_selectors: vec!(LocalNameSelector(Atom::from_slice("e"))),
+                simple_selectors: vec!(LocalNameSelector(LocalNameSelector {
+                    name: Atom::from_slice("EeÉ"),
+                    lower_name: Atom::from_slice("eeÉ") })),
                 next: None,
             }),
             pseudo_element: None,
@@ -600,7 +610,9 @@ mod tests {
         })))
         assert!(parse("e.foo#bar") == Ok(vec!(Selector {
             compound_selectors: Arc::new(CompoundSelector {
-                simple_selectors: vec!(LocalNameSelector(Atom::from_slice("e")),
+                simple_selectors: vec!(LocalNameSelector(LocalNameSelector {
+                                            name: Atom::from_slice("e"),
+                                            lower_name: Atom::from_slice("e") }),
                                        ClassSelector(Atom::from_slice("foo")),
                                        IDSelector(Atom::from_slice("bar"))),
                 next: None,
@@ -612,7 +624,9 @@ mod tests {
             compound_selectors: Arc::new(CompoundSelector {
                 simple_selectors: vec!(IDSelector(Atom::from_slice("bar"))),
                 next: Some((box CompoundSelector {
-                    simple_selectors: vec!(LocalNameSelector(Atom::from_slice("e")),
+                    simple_selectors: vec!(LocalNameSelector(LocalNameSelector {
+                                                name: Atom::from_slice("e"),
+                                                lower_name: Atom::from_slice("e") }),
                                            ClassSelector(Atom::from_slice("foo"))),
                     next: None,
                 }, Descendant)),
@@ -655,7 +669,9 @@ mod tests {
             compound_selectors: Arc::new(CompoundSelector {
                 simple_selectors: vec!(
                     NamespaceSelector(namespace::MathML),
-                    LocalNameSelector(Atom::from_slice("e")),
+                    LocalNameSelector(LocalNameSelector {
+                        name: Atom::from_slice("e"),
+                        lower_name: Atom::from_slice("e") }),
                 ),
                 next: None,
             }),
@@ -675,7 +691,9 @@ mod tests {
             compound_selectors: Arc::new(CompoundSelector {
                 simple_selectors: vec!(),
                 next: Some((box CompoundSelector {
-                    simple_selectors: vec!(LocalNameSelector(Atom::from_slice("div"))),
+                    simple_selectors: vec!(LocalNameSelector(LocalNameSelector {
+                        name: Atom::from_slice("div"),
+                        lower_name: Atom::from_slice("div") })),
                     next: None,
                 }, Descendant)),
             }),

src/components/style/style.rs

@@ -41,7 +41,7 @@ pub use properties::longhands;
 pub use node::{TElement, TNode};
 pub use selectors::{PseudoElement, Before, After, AttrSelector, SpecificNamespace, AnyNamespace};
 pub use selectors::{NamespaceConstraint, Selector, CompoundSelector, SimpleSelector, Combinator};
-pub use selectors::{parse_selector_list};
+pub use selectors::{LocalNameSelector, parse_selector_list};
 pub use namespaces::NamespaceMap;
 pub use media_queries::{MediaRule, MediaQueryList, MediaQuery, Device, MediaType, MediaQueryType};
 pub use font_face::{FontFaceFormat, FontFaceRule, FontFaceSource,FontFaceSourceLine, TtfFormat};