Make get_attr() take a &Namespace rather than Option<~str>

src/components/main/layout/box_.rs

@@ -22,6 +22,7 @@ use servo_net::local_image_cache::LocalImageCache;
 use servo_util::geometry::Au;
 use servo_util::geometry;
 use servo_util::range::*;
+use servo_util::namespace;
 use std::cast;
 use std::cell::RefCell;
 use std::cmp::ApproxEq;
@@ -127,7 +128,7 @@ impl ImageBoxInfo {
 
         fn convert_length(node: &LayoutNode, name: &str) -> Option<Au> {
             node.with_element(|element| {
-                element.get_attr(None, name).and_then(|string| {
+                element.get_attr(&namespace::Null, name).and_then(|string| {
                     let n: Option<int> = FromStr::from_str(string);
                     n
                 }).and_then(|pixels| Some(Au::from_px(pixels)))

src/components/main/layout/wrapper.rs

@@ -15,7 +15,6 @@
 //!     onto these objects and cause use-after-free.
 
 use extra::url::Url;
-use script::dom::bindings::utils::null_str_as_empty_ref;
 use script::dom::element::{Element, HTMLAreaElementTypeId, HTMLAnchorElementTypeId};
 use script::dom::element::{HTMLLinkElementTypeId};
 use script::dom::htmliframeelement::HTMLIFrameElement;
@@ -290,10 +289,15 @@ impl<'ln> TNode<LayoutElement<'ln>> for LayoutNode<'ln> {
             } else {
                 attr.name.as_slice()
             };
-            // FIXME: avoid .clone() here? See #1367
-            match element.get_attr(attr.namespace.clone(), name) {
-                Some(value) => test(value),
-                None => false,
+            match attr.namespace {
+                Some(ref url) => {
+                    match element.get_attr(&namespace::Namespace::from_str(url.as_slice()), name) {
+                        Some(value) => test(value),
+                        None => false,
+                    }
+                },
+                // FIXME: support `*|attr`, attribute selectors in any namespace
+                None => return false,
             }
         })
     }
@@ -405,8 +409,7 @@ impl<'le> TElement for LayoutElement<'le> {
     }
 
     #[inline]
-    fn get_attr(&self, ns_url: Option<~str>, name: &str) -> Option<&'static str> {
-        let namespace = Namespace::from_str(null_str_as_empty_ref(&ns_url));
+    fn get_attr(&self, namespace: &Namespace, name: &str) -> Option<&'static str> {
         unsafe { self.element.get_attr_val_for_layout(namespace, name) }
     }
 
@@ -418,7 +421,7 @@ impl<'le> TElement for LayoutElement<'le> {
             ElementNodeTypeId(HTMLAnchorElementTypeId) |
             ElementNodeTypeId(HTMLAreaElementTypeId) |
             ElementNodeTypeId(HTMLLinkElementTypeId) => {
-                unsafe { self.element.get_attr_val_for_layout(namespace::Null, "href") }
+                unsafe { self.element.get_attr_val_for_layout(&namespace::Null, "href") }
                 .map(|val| val.to_owned())
             }
             _ => None,

src/components/script/dom/element.rs

@@ -152,13 +152,14 @@ impl Element {
         }).map(|&x| x)
     }
 
-    pub unsafe fn get_attr_val_for_layout(&self, namespace: Namespace, name: &str)
+    #[inline]
+    pub unsafe fn get_attr_val_for_layout(&self, namespace: &Namespace, name: &str)
                                           -> Option<&'static str> {
         self.attrs.iter().find(|attr: & &@mut Attr| {
             // unsafely avoid a borrow because this is accessed by many tasks
             // during parallel layout
             let attr: ***Box<Attr> = cast::transmute(attr);
-            name == (***attr).data.local_name && (***attr).data.namespace == namespace
+            name == (***attr).data.local_name && (***attr).data.namespace == *namespace
        }).map(|attr| {
             let attr: **Box<Attr> = cast::transmute(attr);
             cast::transmute((**attr).data.value.as_slice())

src/components/style/node.rs

@@ -6,6 +6,7 @@
 //! style.
 
 use selectors::AttrSelector;
+use servo_util::namespace::Namespace;
 
 
 pub trait TNode<E:TElement> : Clone {
@@ -23,7 +24,7 @@ pub trait TNode<E:TElement> : Clone {
 }
 
 pub trait TElement {
-    fn get_attr(&self, namespace: Option<~str>, attr: &str) -> Option<&'static str>;
+    fn get_attr(&self, namespace: &Namespace, attr: &str) -> Option<&'static str>;
     fn get_link(&self) -> Option<~str>;
     fn get_local_name<'a>(&'a self) -> &'a str;
     fn get_namespace_url<'a>(&'a self) -> &'a str;

src/components/style/selector_matching.rs

@@ -7,6 +7,8 @@ use std::ascii::StrAsciiExt;
 use std::hashmap::HashMap;
 use std::str;
 
+use servo_util::namespace;
+
 use media_queries::{Device, Screen};
 use node::{TElement, TNode};
 use properties::{PropertyDeclaration, PropertyDeclarationBlock};
@@ -73,7 +75,7 @@ impl SelectorMap {
         // At the end, we're going to sort the rules that we added, so remember where we began.
         let init_len = matching_rules_list.len();
         node.with_element(|element: &E| {
-            match element.get_attr(None, "id") {
+            match element.get_attr(&namespace::Null, "id") {
                 Some(id) => {
                     SelectorMap::get_matching_rules_from_hash(node,
                                                               &self.id_hash,
@@ -83,7 +85,7 @@ impl SelectorMap {
                 None => {}
             }
 
-            match element.get_attr(None, "class") {
+            match element.get_attr(&namespace::Null, "class") {
                 Some(ref class_attr) => {
                     for class in class_attr.split(SELECTOR_WHITESPACE) {
                         SelectorMap::get_matching_rules_from_hash(
@@ -495,7 +497,7 @@ fn matches_simple_selector<E:TElement,N:TNode<E>>(selector: &SimpleSelector, ele
         // TODO: cache and intern IDs on elements.
         IDSelector(ref id) => {
             element.with_element(|element: &E| {
-                match element.get_attr(None, "id") {
+                match element.get_attr(&namespace::Null, "id") {
                     Some(attr) => str::eq_slice(attr, *id),
                     None => false
                 }
@@ -504,7 +506,7 @@ fn matches_simple_selector<E:TElement,N:TNode<E>>(selector: &SimpleSelector, ele
         // TODO: cache and intern classe names on elements.
         ClassSelector(ref class) => {
             element.with_element(|element: &E| {
-                match element.get_attr(None, "class") {
+                match element.get_attr(&namespace::Null, "class") {
                     None => false,
                     // TODO: case-sensitivity depends on the document type and quirks mode
                     Some(ref class_attr)