auto merge of #1438 : Ms2ger/servo/cleanup-get_attribute, r=jdm

Passing these around as strings is an anti-pattern, IMO.

8 files changed, 27 insertions, 19 deletions

diff --git a/src/components/main/layout/wrapper.rs b/src/components/main/layout/wrapper.rs
index 6ab1504df85..619c711329a 100644
--- a/src/components/main/layout/wrapper.rs
+++ b/src/components/main/layout/wrapper.rs
@@ -19,6 +19,7 @@ use script::dom::element::{Element, HTMLAreaElementTypeId, HTMLAnchorElementType
 use script::dom::element::{HTMLLinkElementTypeId};
 use script::dom::htmliframeelement::HTMLIFrameElement;
 use script::dom::htmlimageelement::HTMLImageElement;
+use script::dom::namespace::Namespace;
 use script::dom::node::{AbstractNode, DocumentNodeTypeId, ElementNodeTypeId, Node, NodeTypeId};
 use script::dom::text::Text;
 use servo_msg::constellation_msg::{PipelineId, SubpageId};
@@ -379,7 +380,8 @@ impl<'self> TElement for LayoutElement<'self> {
     }
 
     fn get_attr(&self, ns_url: Option<~str>, name: &str) -> Option<~str> {
-        self.element.get_attr(ns_url, name)
+        let namespace = Namespace::from_str(ns_url);
+        self.element.get_attr(namespace, name)
     }
 
     fn get_link(&self) -> Option<~str> {
diff --git a/src/components/script/dom/document.rs b/src/components/script/dom/document.rs
index 511620aead0..2d5582e73f1 100644
--- a/src/components/script/dom/document.rs
+++ b/src/components/script/dom/document.rs
@@ -15,6 +15,7 @@ use dom::event::{AbstractEvent, Event};
 use dom::htmlcollection::HTMLCollection;
 use dom::htmldocument::HTMLDocument;
 use dom::mouseevent::MouseEvent;
+use dom::namespace::Null;
 use dom::node::{AbstractNode, Node, ElementNodeTypeId, DocumentNodeTypeId};
 use dom::text::Text;
 use dom::uievent::UIEvent;
@@ -354,7 +355,7 @@ impl Document {
 
     pub fn GetElementsByName(&self, name: DOMString) -> @mut HTMLCollection {
         self.createHTMLCollection(|elem|
-            elem.get_attr(None, "name").is_some() && eq_slice(elem.get_attr(None, "name").unwrap(), name))
+            elem.get_attr(Null, "name").is_some() && eq_slice(elem.get_attr(Null, "name").unwrap(), name))
     }
 
     pub fn createHTMLCollection(&self, callback: &fn(elem: &Element) -> bool) -> @mut HTMLCollection {
@@ -434,7 +435,7 @@ fn foreach_ided_elements(root: &AbstractNode, callback: &fn(&DOMString, &Abstrac
         }
 
         do node.with_imm_element |element| {
-            match element.get_attr(None, "id") {
+            match element.get_attr(Null, "id") {
                 Some(id) => {
                     callback(&id.to_str(), &node);
                 }
diff --git a/src/components/script/dom/element.rs b/src/components/script/dom/element.rs
index 31dd20596f4..ba08e3660c6 100644
--- a/src/components/script/dom/element.rs
+++ b/src/components/script/dom/element.rs
@@ -16,7 +16,7 @@ use dom::document::AbstractDocument;
 use dom::node::{AbstractNode, ElementNodeTypeId, Node};
 use dom::document;
 use dom::namespace;
-use dom::namespace::Namespace;
+use dom::namespace::{Namespace, Null};
 use layout_interface::{ContentBoxQuery, ContentBoxResponse, ContentBoxesQuery};
 use layout_interface::{ContentBoxesResponse, ContentChangedDocumentDamage};
 use layout_interface::{MatchSelectorsDocumentDamage};
@@ -149,9 +149,8 @@ impl<'self> Element {
     }
 
     pub fn get_attribute(&self,
-                         namespace_url: Option<DOMString>,
+                         namespace: Namespace,
                          name: &str) -> Option<@mut Attr> {
-        let namespace = Namespace::from_str(namespace_url);
         // FIXME: only case-insensitive in the HTML namespace (as opposed to SVG, etc.)
         let name = name.to_ascii_lower();
         self.attrs.find_equiv(&name).and_then(|attrs| {
@@ -162,8 +161,8 @@ impl<'self> Element {
     }
 
     // FIXME(pcwalton): This is kind of confusingly named relative to the above...
-    pub fn get_attr(&self, ns_url: Option<~str>, name: &str) -> Option<~str> {
-        self.get_attribute(ns_url, name).map(|attr| attr.value.clone())
+    pub fn get_attr(&self, namespace: Namespace, name: &str) -> Option<~str> {
+        self.get_attribute(namespace, name).map(|attr| attr.value.clone())
     }
 
     pub fn set_attr(&mut self, abstract_self: AbstractNode, name: DOMString, value: DOMString)
@@ -287,7 +286,7 @@ impl Element {
     }
 
     pub fn Id(&self, _abstract_self: AbstractNode) -> DOMString {
-        match self.get_attr(None, "id") {
+        match self.get_attr(Null, "id") {
             Some(x) => x,
             None => ~""
         }
@@ -310,10 +309,11 @@ impl Element {
     }
 
     pub fn GetAttribute(&self, name: DOMString) -> Option<DOMString> {
-        self.get_attr(None, name).map(|s| s.to_owned())
+        self.get_attr(Null, name).map(|s| s.to_owned())
     }
 
     pub fn GetAttributeNS(&self, namespace: Option<DOMString>, local_name: DOMString) -> Option<DOMString> {
+        let namespace = Namespace::from_str(namespace);
         self.get_attribute(namespace, local_name)
             .map(|attr| attr.value.clone())
     }
diff --git a/src/components/script/dom/htmldocument.rs b/src/components/script/dom/htmldocument.rs
index 6fb84620ab0..8e92a4b41db 100644
--- a/src/components/script/dom/htmldocument.rs
+++ b/src/components/script/dom/htmldocument.rs
@@ -7,6 +7,7 @@ use dom::bindings::utils::{Reflectable, Reflector, Traceable};
 use dom::document::{AbstractDocument, Document, HTML};
 use dom::element::HTMLHeadElementTypeId;
 use dom::htmlcollection::HTMLCollection;
+use dom::namespace::Null;
 use dom::node::{AbstractNode, ElementNodeTypeId};
 use dom::window::Window;
 
@@ -56,7 +57,7 @@ impl HTMLDocument {
     pub fn Links(&self) -> @mut HTMLCollection {
         self.parent.createHTMLCollection(|elem|
             (eq_slice(elem.tag_name, "a") || eq_slice(elem.tag_name, "area"))
-            && elem.get_attr(None, "href").is_some())
+            && elem.get_attr(Null, "href").is_some())
     }
 
     pub fn Forms(&self) -> @mut HTMLCollection {
@@ -69,7 +70,7 @@ impl HTMLDocument {
 
     pub fn Anchors(&self) -> @mut HTMLCollection {
         self.parent.createHTMLCollection(|elem|
-            eq_slice(elem.tag_name, "a") && elem.get_attr(None, "name").is_some())
+            eq_slice(elem.tag_name, "a") && elem.get_attr(Null, "name").is_some())
     }
 
     pub fn Applets(&self) -> @mut HTMLCollection {
diff --git a/src/components/script/dom/htmlimageelement.rs b/src/components/script/dom/htmlimageelement.rs
index d4f8479abe7..aa1a02d7ecb 100644
--- a/src/components/script/dom/htmlimageelement.rs
+++ b/src/components/script/dom/htmlimageelement.rs
@@ -7,6 +7,7 @@ use dom::bindings::utils::{DOMString, ErrorResult};
 use dom::document::AbstractDocument;
 use dom::element::HTMLImageElementTypeId;
 use dom::htmlelement::HTMLElement;
+use dom::namespace::Null;
 use dom::node::{AbstractNode, Node};
 use extra::url::Url;
 use servo_util::geometry::to_px;
@@ -40,7 +41,7 @@ impl HTMLImageElement {
     /// prefetching the image. This method must be called after `src` is changed.
     pub fn update_image(&mut self, image_cache: ImageCacheTask, url: Option<Url>) {
         let elem = &mut self.htmlelement.element;
-        let src_opt = elem.get_attr(None, "src").map(|x| x.to_str());
+        let src_opt = elem.get_attr(Null, "src").map(|x| x.to_str());
         match src_opt {
             None => {}
             Some(src) => {
diff --git a/src/components/script/dom/htmlscriptelement.rs b/src/components/script/dom/htmlscriptelement.rs
index 50f923e7718..7405db82b29 100644
--- a/src/components/script/dom/htmlscriptelement.rs
+++ b/src/components/script/dom/htmlscriptelement.rs
@@ -7,6 +7,7 @@ use dom::bindings::utils::{DOMString, ErrorResult};
 use dom::document::AbstractDocument;
 use dom::element::HTMLScriptElementTypeId;
 use dom::htmlelement::HTMLElement;
+use dom::namespace::Null;
 use dom::node::{AbstractNode, Node};
 use style::TElement;
 
@@ -29,7 +30,7 @@ impl HTMLScriptElement {
 
 impl HTMLScriptElement {
     pub fn Src(&self) -> DOMString {
-        match self.htmlelement.element.get_attr(None, "src") {
+        match self.htmlelement.element.get_attr(Null, "src") {
             Some(s) => s.to_owned(),
             None => ~""
         }
diff --git a/src/components/script/html/hubbub_html_parser.rs b/src/components/script/html/hubbub_html_parser.rs
index da9adf01680..c25a2fe7b11 100644
--- a/src/components/script/html/hubbub_html_parser.rs
+++ b/src/components/script/html/hubbub_html_parser.rs
@@ -9,6 +9,7 @@ use dom::htmlheadingelement::{Heading1, Heading2, Heading3, Heading4, Heading5,
 use dom::htmliframeelement::IFrameSize;
 use dom::htmlformelement::HTMLFormElement;
 use dom::namespace;
+use dom::namespace::Null;
 use dom::node::{AbstractNode, ElementNodeTypeId};
 use dom::types::*;
 use html::cssparse::{InlineProvenance, StylesheetProvenance, UrlProvenance, spawn_css_parser};
@@ -347,7 +348,7 @@ pub fn parse_html(cx: *JSContext,
                 // Handle CSS style sheets from <link> elements
                 ElementNodeTypeId(HTMLLinkElementTypeId) => {
                     do node.with_imm_element |element| {
-                        match (element.get_attr(None, "rel"), element.get_attr(None, "href")) {
+                        match (element.get_attr(Null, "rel"), element.get_attr(Null, "href")) {
                             (Some(rel), Some(href)) => {
                                 if "stylesheet" == rel {
                                     debug!("found CSS stylesheet: {:s}", href);
@@ -366,7 +367,7 @@ pub fn parse_html(cx: *JSContext,
                         let iframe_chan = iframe_chan.take();
                         let sandboxed = iframe_element.is_sandboxed();
                         let elem = &mut iframe_element.htmlelement.element;
-                        let src_opt = elem.get_attr(None, "src").map(|x| x.to_str());
+                        let src_opt = elem.get_attr(Null, "src").map(|x| x.to_str());
                         for src in src_opt.iter() {
                             let iframe_url = make_url(src.clone(), Some(url2.clone()));
                             iframe_element.frame = Some(iframe_url.clone());
@@ -462,7 +463,7 @@ pub fn parse_html(cx: *JSContext,
             unsafe {
                 let scriptnode: AbstractNode = NodeWrapping::from_hubbub_node(script);
                 do scriptnode.with_imm_element |script| {
-                    match script.get_attr(None, "src") {
+                    match script.get_attr(Null, "src") {
                         Some(src) => {
                             debug!("found script: {:s}", src);
                             let new_url = make_url(src.to_str(), Some(url3.clone()));
diff --git a/src/components/script/script_task.rs b/src/components/script/script_task.rs
index 38803bb4456..9400a519d92 100644
--- a/src/components/script/script_task.rs
+++ b/src/components/script/script_task.rs
@@ -13,6 +13,7 @@ use dom::event::{Event_, ResizeEvent, ReflowEvent, ClickEvent, MouseDownEvent, M
 use dom::event::Event;
 use dom::eventtarget::AbstractEventTarget;
 use dom::htmldocument::HTMLDocument;
+use dom::namespace::Null;
 use dom::node::{AbstractNode, LayoutDataRef};
 use dom::window::{TimerData, Window};
 use html::hubbub_html_parser::HtmlParserResult;
@@ -811,7 +812,7 @@ impl ScriptTask {
                 let mut anchors = doc_node.traverse_preorder().filter(|node| node.is_anchor_element());
                 do anchors.find |node| {
                     do node.with_imm_element |elem| {
-                        match elem.get_attr(None, "name") {
+                        match elem.get_attr(Null, "name") {
                             Some(name) => eq_slice(name, fragid),
                             None => false
                         }
@@ -910,7 +911,7 @@ impl ScriptTask {
 
     fn load_url_from_element(&self, page: @mut Page, element: &Element) {
         // if the node's element is "a," load url from href attr
-        let attr = element.get_attr(None, "href");
+        let attr = element.get_attr(Null, "href");
         for href in attr.iter() {
             debug!("ScriptTask: clicked on link to {:s}", *href);
             let click_frag = href.starts_with("#");