Move attributes-related functions onto JS<Element>.

src/components/script/dom/bindings/codegen/Bindings.conf

@@ -53,11 +53,15 @@ DOMInterfaces = {
     'needsAbstract': [
         'attributes',
         'className',
+        'getAttribute',
+        'getAttributeNS',
         'getBoundingClientRect',
         'getClientRects',
         'getElementsByClassName',
         'getElementsByTagName',
         'getElementsByTagNameNS',
+        'hasAttribute',
+        'hasAttributeNS',
         'id',
         'innerHTML',
         'outerHTML',
@@ -179,7 +183,7 @@ addHTMLElement('HTMLParamElement')
 addHTMLElement('HTMLPreElement')
 addHTMLElement('HTMLProgressElement')
 addHTMLElement('HTMLQuoteElement')
-addHTMLElement('HTMLScriptElement')
+addHTMLElement('HTMLScriptElement', needsAbstract=['src'])
 addHTMLElement('HTMLSelectElement')
 addHTMLElement('HTMLSourceElement')
 addHTMLElement('HTMLSpanElement')

src/components/script/dom/document.rs

@@ -15,7 +15,7 @@ use dom::comment::Comment;
 use dom::documentfragment::DocumentFragment;
 use dom::documenttype::DocumentType;
 use dom::domimplementation::DOMImplementation;
-use dom::element::{Element};
+use dom::element::{Element, AttributeHandlers};
 use dom::element::{HTMLHtmlElementTypeId, HTMLHeadElementTypeId, HTMLTitleElementTypeId};
 use dom::element::{HTMLBodyElementTypeId, HTMLFrameSetElementTypeId};
 use dom::event::Event;
@@ -435,7 +435,7 @@ impl Document {
             }
 
             let element: JS<Element> = ElementCast::to(node).unwrap();
-            element.get().get_attribute(Null, "name").map_or(false, |attr| {
+            element.get_attribute(Null, "name").map_or(false, |attr| {
                 attr.get().value_ref() == name
             })
         })
@@ -460,7 +460,7 @@ impl Document {
         // FIXME: https://github.com/mozilla/servo/issues/1847
         HTMLCollection::create(&self.window, &NodeCast::from(abstract_self), |elem| {
             ("a" == elem.get().tag_name || "area" == elem.get().tag_name) &&
-            elem.get().get_attribute(Null, "href").is_some()
+            elem.get_attribute(Null, "href").is_some()
         })
     }
 
@@ -477,7 +477,7 @@ impl Document {
     pub fn Anchors(&self, abstract_self: &JS<Document>) -> JS<HTMLCollection> {
         // FIXME: https://github.com/mozilla/servo/issues/1847
         HTMLCollection::create(&self.window, &NodeCast::from(abstract_self), |elem| {
-            "a" == elem.get().tag_name && elem.get().get_attribute(Null, "name").is_some()
+            "a" == elem.get().tag_name && elem.get_attribute(Null, "name").is_some()
         })
     }
 

src/components/script/dom/element.rs

@@ -155,7 +155,9 @@ impl Element {
         self.namespace == namespace::HTML &&
         self.node.owner_doc().get().is_html_document
     }
+}
 
+impl Element {
     pub unsafe fn html_element_in_html_document_for_layout(&self) -> bool {
         if self.namespace != namespace::HTML {
             return false
@@ -167,15 +169,6 @@ impl Element {
         (**owner_doc).is_html_document
     }
 
-    pub fn get_attribute(&self,
-                         namespace: Namespace,
-                         name: &str) -> Option<JS<Attr>> {
-        self.attrs.iter().find(|attr| {
-            let attr = attr.get();
-            name == attr.local_name && attr.namespace == namespace
-        }).map(|x| x.clone())
-    }
-
     #[inline]
     pub unsafe fn get_attr_val_for_layout(&self, namespace: &Namespace, name: &str)
                                           -> Option<&'static str> {
@@ -189,16 +182,47 @@ impl Element {
             cast::transmute((**attr).value.as_slice())
         })
     }
+}
 
-    pub fn set_attr(&mut self, abstract_self: &JS<Element>, name: DOMString, value: DOMString)
+pub trait AttributeHandlers {
-                    -> ErrorResult {
+    fn get_attribute(&self, namespace: Namespace, name: &str) -> Option<JS<Attr>>;
-        self.set_attribute(abstract_self, namespace::Null, name, value)
+    fn set_attr(&mut self, name: DOMString, value: DOMString) -> ErrorResult;
+    fn set_attribute(&mut self, namespace: Namespace, name: DOMString,
+                     value: DOMString) -> ErrorResult;
+    fn after_set_attr(&mut self, local_name: DOMString, value: DOMString);
+    fn remove_attribute(&mut self, namespace: Namespace, name: DOMString) -> ErrorResult;
+    fn before_remove_attr(&mut self, local_name: DOMString, old_value: DOMString);
+    fn notify_attribute_changed(&self, local_name: DOMString);
+    fn has_class(&self, name: &str) -> bool;
+
+    // http://www.whatwg.org/html/#reflecting-content-attributes-in-idl-attributes
+    fn get_url_attribute(&self, name: &str) -> DOMString;
+    fn set_url_attribute(&mut self, name: &str, value: DOMString);
+    fn get_string_attribute(&self, name: &str) -> DOMString;
+    fn set_string_attribute(&mut self, name: &str, value: DOMString);
+}
+
+pub trait AfterSetAttrListener {
+    fn AfterSetAttr(&mut self, name: DOMString, value: DOMString);
+}
+
+pub trait BeforeRemoveAttrListener {
+    fn BeforeRemoveAttr(&mut self, name: DOMString);
+}
+
+impl AttributeHandlers for JS<Element> {
+    fn get_attribute(&self, namespace: Namespace, name: &str) -> Option<JS<Attr>> {
+        self.get().attrs.iter().find(|attr| {
+            let attr = attr.get();
+            name == attr.local_name && attr.namespace == namespace
+        }).map(|x| x.clone())
     }
 
-    pub fn set_attribute(&mut self,
+    fn set_attr(&mut self, name: DOMString, value: DOMString) -> ErrorResult {
-                         abstract_self: &JS<Element>,
+        self.set_attribute(namespace::Null, name, value)
-                         namespace: Namespace,
+    }
-                         name: DOMString,
+
+    fn set_attribute(&mut self, namespace: Namespace, name: DOMString,
                      value: DOMString) -> ErrorResult {
         let (prefix, local_name) = get_attribute_parts(name.clone());
         match prefix {
@@ -212,92 +236,84 @@ impl Element {
             None => {}
         }
 
-        self.node.wait_until_safe_to_modify_dom();
+        let node: JS<Node> = NodeCast::from(self);
+        node.get().wait_until_safe_to_modify_dom();
 
         // FIXME: reduce the time of `value.clone()`.
-        let idx = self.attrs.iter().position(|attr| {
+        let idx = self.get().attrs.iter().position(|attr| {
             attr.get().local_name == local_name
         });
 
         match idx {
             Some(idx) => {
                 if namespace == namespace::Null {
-                    let old_value = self.attrs[idx].get().Value();
+                    let old_value = self.get().attrs[idx].get().Value();
-                    self.before_remove_attr(abstract_self, local_name.clone(),
+                    self.before_remove_attr(local_name.clone(), old_value);
-                                            old_value);
                 }
-                self.attrs[idx].get_mut().set_value(value.clone());
+                self.get_mut().attrs[idx].get_mut().set_value(value.clone());
             }
             None => {
-                let doc = self.node.owner_doc();
+                let node: JS<Node> = NodeCast::from(self);
-                let doc = doc.get();
+                let doc = node.get().owner_doc().get();
                 let new_attr = Attr::new_ns(&doc.window, local_name.clone(), value.clone(),
                                             name.clone(), namespace.clone(),
                                             prefix);
-                self.attrs.push(new_attr);
+                self.get_mut().attrs.push(new_attr);
             }
         }
 
         if namespace == namespace::Null {
-            self.after_set_attr(abstract_self, local_name, value);
+            self.after_set_attr(local_name, value);
         }
         Ok(())
     }
 
-    fn after_set_attr(&mut self,
+    fn after_set_attr(&mut self, local_name: DOMString, value: DOMString) {
-                      abstract_self: &JS<Element>,
+        let node: JS<Node> = NodeCast::from(self);
-                      local_name: DOMString,
-                      value: DOMString) {
-
         match local_name.as_slice() {
             "style" => {
-                let doc = self.node.owner_doc();
+                let doc = node.get().owner_doc();
                 let base_url = doc.get().url().clone();
-                self.style_attribute = Some(style::parse_style_attribute(value, &base_url))
+                self.get_mut().style_attribute = Some(style::parse_style_attribute(value, &base_url))
             }
-            "id" => {
+            "id" if node.is_in_doc() => {
-                let self_node: JS<Node> = NodeCast::from(abstract_self);
-                if self_node.is_in_doc() {
                 // XXX: this dual declaration are workaround to avoid the compile error:
                 // "borrowed value does not live long enough"
-                    let mut doc = self.node.owner_doc().clone();
+                let mut doc = node.get().owner_doc().clone();
                 let doc = doc.get_mut();
-                    doc.register_named_element(abstract_self, value.clone());
+                doc.register_named_element(self, value.clone());
-                }
             }
             _ => ()
         }
 
         //XXXjdm We really need something like a vtable so we can call AfterSetAttr.
         //       This hardcoding is awful.
-        match abstract_self.get().node.type_id {
+        match node.type_id() {
             ElementNodeTypeId(HTMLImageElementTypeId) => {
-                let mut elem: JS<HTMLImageElement> = HTMLImageElementCast::to(abstract_self).unwrap();
+                let mut elem: JS<HTMLImageElement> = HTMLImageElementCast::to(self).unwrap();
-                elem.get_mut().AfterSetAttr(local_name.clone(), value.clone());
+                elem.AfterSetAttr(local_name.clone(), value.clone());
             }
             ElementNodeTypeId(HTMLIFrameElementTypeId) => {
-                let mut elem: JS<HTMLIFrameElement> = HTMLIFrameElementCast::to(abstract_self).unwrap();
+                let mut elem: JS<HTMLIFrameElement> = HTMLIFrameElementCast::to(self).unwrap();
-                elem.get_mut().AfterSetAttr(local_name.clone(), value.clone());
+                elem.AfterSetAttr(local_name.clone(), value.clone());
             }
             ElementNodeTypeId(HTMLObjectElementTypeId) => {
-                let mut elem: JS<HTMLObjectElement> = HTMLObjectElementCast::to(abstract_self).unwrap();
+                let mut elem: JS<HTMLObjectElement> = HTMLObjectElementCast::to(self).unwrap();
-                elem.get_mut().AfterSetAttr(local_name.clone(), value.clone());
+                elem.AfterSetAttr(local_name.clone(), value.clone());
             }
             _ => ()
         }
 
-        self.notify_attribute_changed(abstract_self, local_name);
+        self.notify_attribute_changed(local_name);
     }
 
-    pub fn remove_attribute(&mut self,
+    fn remove_attribute(&mut self, namespace: Namespace, name: DOMString) -> ErrorResult {
-                            abstract_self: &JS<Element>,
-                            namespace: Namespace,
-                            name: DOMString) -> ErrorResult {
         let (_, local_name) = get_attribute_parts(name.clone());
 
-        self.node.wait_until_safe_to_modify_dom();
+        let node: JS<Node> = NodeCast::from(self);
+        node.get().wait_until_safe_to_modify_dom();
 
-        let idx = self.attrs.iter().position(|attr: &JS<Attr>| -> bool {
+        let idx = self.get().attrs.iter().position(|attr| {
             attr.get().local_name == local_name
         });
 
@@ -305,69 +321,90 @@ impl Element {
             None => (),
             Some(idx) => {
                 if namespace == namespace::Null {
-                    let removed_raw_value = self.attrs[idx].get().Value();
+                    let removed_raw_value = self.get().attrs[idx].get().Value();
-                    self.before_remove_attr(abstract_self, local_name, removed_raw_value);
+                    self.before_remove_attr(local_name, removed_raw_value);
                 }
 
-                self.attrs.remove(idx);
+                self.get_mut().attrs.remove(idx);
             }
         };
 
         Ok(())
     }
 
-    fn before_remove_attr(&mut self,
+    fn before_remove_attr(&mut self, local_name: DOMString, old_value: DOMString) {
-                          abstract_self: &JS<Element>,
+        let node: JS<Node> = NodeCast::from(self);
-                          local_name: DOMString,
-                          old_value: DOMString) {
         match local_name.as_slice() {
             "style" => {
-                self.style_attribute = None
+                self.get_mut().style_attribute = None
             }
-            "id" => {
+            "id" if node.is_in_doc() => {
-                let self_node: JS<Node> = NodeCast::from(abstract_self);
-                if self_node.is_in_doc() {
                 // XXX: this dual declaration are workaround to avoid the compile error:
                 // "borrowed value does not live long enough"
-                    let mut doc = self.node.owner_doc().clone();
+                let mut doc = node.get().owner_doc().clone();
                 let doc = doc.get_mut();
                 doc.unregister_named_element(old_value);
             }
-            }
             _ => ()
         }
 
-        //XXXjdm We really need something like a vtable so we can call AfterSetAttr.
+        //XXXjdm We really need something like a vtable so we can call BeforeRemoveAttr.
         //       This hardcoding is awful.
-        match abstract_self.get().node.type_id {
+        match node.type_id() {
             ElementNodeTypeId(HTMLImageElementTypeId) => {
-                let mut elem: JS<HTMLImageElement> = HTMLImageElementCast::to(abstract_self).unwrap();
+                let mut elem: JS<HTMLImageElement> = HTMLImageElementCast::to(self).unwrap();
-                elem.get_mut().BeforeRemoveAttr(local_name.clone());
+                elem.BeforeRemoveAttr(local_name.clone());
             }
             ElementNodeTypeId(HTMLIFrameElementTypeId) => {
-                let mut elem: JS<HTMLIFrameElement> = HTMLIFrameElementCast::to(abstract_self).unwrap();
+                let mut elem: JS<HTMLIFrameElement> = HTMLIFrameElementCast::to(self).unwrap();
-                elem.get_mut().BeforeRemoveAttr(local_name.clone());
+                elem.BeforeRemoveAttr(local_name.clone());
             }
             _ => ()
         }
 
-        self.notify_attribute_changed(abstract_self, local_name);
+        self.notify_attribute_changed(local_name);
     }
 
-    fn notify_attribute_changed(&self,
+    fn notify_attribute_changed(&self, local_name: DOMString) {
-                                abstract_self: &JS<Element>,
+        let node: JS<Node> = NodeCast::from(self);
-                                local_name: DOMString) {
-        let node: JS<Node> = NodeCast::from(abstract_self);
         if node.is_in_doc() {
             let damage = match local_name.as_slice() {
                 "style" | "id" | "class" => MatchSelectorsDocumentDamage,
                 _ => ContentChangedDocumentDamage
             };
-            let document = self.node.owner_doc();
+            let document = node.get().owner_doc();
             document.get().damage_and_reflow(damage);
         }
     }
 
+    fn has_class(&self, name: &str) -> bool {
+        // FIXME: https://github.com/mozilla/servo/issues/1840
+        let class_names = self.get_string_attribute("class");
+        let mut classes = class_names.split(' ');
+        classes.any(|class| name == class)
+    }
+
+    fn get_url_attribute(&self, name: &str) -> DOMString {
+        // XXX Resolve URL.
+        self.get_string_attribute(name)
+    }
+    fn set_url_attribute(&mut self, name: &str, value: DOMString) {
+        self.set_string_attribute(name, value);
+    }
+
+    fn get_string_attribute(&self, name: &str) -> DOMString {
+        match self.get_attribute(Null, name) {
+            Some(x) => x.get().Value(),
+            None => ~""
+        }
+    }
+    fn set_string_attribute(&mut self, name: &str, value: DOMString) {
+        assert!(name == name.to_ascii_lower());
+        assert!(self.set_attribute(Null, name.to_owned(), value).is_ok());
+    }
+}
+
+impl Element {
     pub fn is_void(&self) -> bool {
         if self.namespace != namespace::HTML {
             return false
@@ -381,37 +418,6 @@ impl Element {
             _ => false
         }
     }
-
-    pub fn has_class(&self, name: &str) -> bool {
-        // FIXME: https://github.com/mozilla/servo/issues/1840
-        let class_names = self.get_string_attribute("class");
-        let mut classes = class_names.split(' ');
-        classes.any(|class| name == class)
-    }
-}
-
-// http://www.whatwg.org/html/#reflecting-content-attributes-in-idl-attributes
-impl Element {
-    pub fn get_url_attribute(&self, name: &str) -> DOMString {
-        // XXX Resolve URL.
-        self.get_string_attribute(name)
-    }
-    pub fn set_url_attribute(&mut self, abstract_self: &JS<Element>,
-                             name: &str, value: DOMString) {
-        self.set_string_attribute(abstract_self, name, value);
-    }
-
-    pub fn get_string_attribute(&self, name: &str) -> DOMString {
-        match self.get_attribute(Null, name) {
-            Some(x) => x.get().Value(),
-            None => ~""
-        }
-    }
-    pub fn set_string_attribute(&mut self, abstract_self: &JS<Element>,
-                                name: &str, value: DOMString) {
-        assert!(name == name.to_ascii_lower());
-        assert!(self.set_attribute(abstract_self, Null, name.to_owned(), value).is_ok());
-    }
 }
 
 impl Element {
@@ -421,23 +427,23 @@ impl Element {
     }
 
     // http://dom.spec.whatwg.org/#dom-element-id
-    pub fn Id(&self, _abstract_self: &JS<Element>) -> DOMString {
+    pub fn Id(&self, abstract_self: &JS<Element>) -> DOMString {
-        self.get_string_attribute("id")
+        abstract_self.get_string_attribute("id")
     }
 
     // http://dom.spec.whatwg.org/#dom-element-id
-    pub fn SetId(&mut self, abstract_self: &JS<Element>, id: DOMString) {
+    pub fn SetId(&mut self, abstract_self: &mut JS<Element>, id: DOMString) {
-        self.set_string_attribute(abstract_self, "id", id);
+        abstract_self.set_string_attribute("id", id);
     }
 
     // http://dom.spec.whatwg.org/#dom-element-classname
-    pub fn ClassName(&self, _abstract_self: &JS<Element>) -> DOMString {
+    pub fn ClassName(&self, abstract_self: &JS<Element>) -> DOMString {
-        self.get_string_attribute("class")
+        abstract_self.get_string_attribute("class")
     }
 
     // http://dom.spec.whatwg.org/#dom-element-classname
-    pub fn SetClassName(&mut self, abstract_self: &JS<Element>, class: DOMString) {
+    pub fn SetClassName(&self, abstract_self: &mut JS<Element>, class: DOMString) {
-        self.set_string_attribute(abstract_self, "class", class);
+        abstract_self.set_string_attribute("class", class);
     }
 
     // http://dom.spec.whatwg.org/#dom-element-attributes
@@ -455,37 +461,40 @@ impl Element {
     }
 
     // http://dom.spec.whatwg.org/#dom-element-getattribute
-    pub fn GetAttribute(&self, name: DOMString) -> Option<DOMString> {
+    pub fn GetAttribute(&self, abstract_self: &JS<Element>, name: DOMString) -> Option<DOMString> {
-        let name = if self.html_element_in_html_document() {
+        let name = if abstract_self.get().html_element_in_html_document() {
             name.to_ascii_lower()
         } else {
             name
         };
-        self.get_attribute(Null, name).map(|s| s.get().Value())
+        abstract_self.get_attribute(Null, name).map(|s| s.get().Value())
     }
 
     // http://dom.spec.whatwg.org/#dom-element-getattributens
-    pub fn GetAttributeNS(&self, namespace: Option<DOMString>, local_name: DOMString) -> Option<DOMString> {
+    pub fn GetAttributeNS(&self, abstract_self: &JS<Element>,
+                          namespace: Option<DOMString>,
+                          local_name: DOMString) -> Option<DOMString> {
         let namespace = Namespace::from_str(null_str_as_empty_ref(&namespace));
-        self.get_attribute(namespace, local_name)
+        abstract_self.get_attribute(namespace, local_name)
                      .map(|attr| attr.get().value.clone())
     }
 
     // http://dom.spec.whatwg.org/#dom-element-setattribute
-    pub fn SetAttribute(&mut self, abstract_self: &JS<Element>, name: DOMString, value: DOMString)
+    pub fn SetAttribute(&mut self, abstract_self: &mut JS<Element>,
-                        -> ErrorResult {
+                        name: DOMString,
+                        value: DOMString) -> ErrorResult {
         // FIXME: If name does not match the Name production in XML, throw an "InvalidCharacterError" exception.
         let name = if self.html_element_in_html_document() {
             name.to_ascii_lower()
         } else {
             name
         };
-        self.set_attr(abstract_self, name, value)
+        abstract_self.set_attr(name, value)
     }
 
     // http://dom.spec.whatwg.org/#dom-element-setattributens
     pub fn SetAttributeNS(&mut self,
-                          abstract_self: &JS<Element>,
+                          abstract_self: &mut JS<Element>,
                           namespace_url: Option<DOMString>,
                           name: DOMString,
                           value: DOMString) -> ErrorResult {
@@ -497,38 +506,41 @@ impl Element {
         }
 
         let namespace = Namespace::from_str(null_str_as_empty_ref(&namespace_url));
-        self.set_attribute(abstract_self, namespace, name, value)
+        abstract_self.set_attribute(namespace, name, value)
     }
 
     // http://dom.spec.whatwg.org/#dom-element-removeattribute
     pub fn RemoveAttribute(&mut self,
-                           abstract_self: &JS<Element>,
+                           abstract_self: &mut JS<Element>,
                            name: DOMString) -> ErrorResult {
         let name = if self.html_element_in_html_document() {
             name.to_ascii_lower()
         } else {
             name
         };
-        self.remove_attribute(abstract_self, namespace::Null, name)
+        abstract_self.remove_attribute(namespace::Null, name)
     }
 
     // http://dom.spec.whatwg.org/#dom-element-removeattributens
     pub fn RemoveAttributeNS(&mut self,
-                             abstract_self: &JS<Element>,
+                             abstract_self: &mut JS<Element>,
                              namespace: Option<DOMString>,
                              localname: DOMString) -> ErrorResult {
         let namespace = Namespace::from_str(null_str_as_empty_ref(&namespace));
-        self.remove_attribute(abstract_self, namespace, localname)
+        abstract_self.remove_attribute(namespace, localname)
     }
 
     // http://dom.spec.whatwg.org/#dom-element-hasattribute
-    pub fn HasAttribute(&self, name: DOMString) -> bool {
+    pub fn HasAttribute(&self, abstract_self: &JS<Element>,
-        self.GetAttribute(name).is_some()
+                        name: DOMString) -> bool {
+        self.GetAttribute(abstract_self, name).is_some()
     }
 
     // http://dom.spec.whatwg.org/#dom-element-hasattributens
-    pub fn HasAttributeNS(&self, namespace: Option<DOMString>, local_name: DOMString) -> bool {
+    pub fn HasAttributeNS(&self, abstract_self: &JS<Element>,
-        self.GetAttributeNS(namespace, local_name).is_some()
+                          namespace: Option<DOMString>,
+                          local_name: DOMString) -> bool {
+        self.GetAttributeNS(abstract_self, namespace, local_name).is_some()
     }
 
     pub fn GetElementsByTagName(&self, abstract_self: &JS<Element>, localname: DOMString) -> JS<HTMLCollection> {
@@ -614,7 +626,7 @@ pub trait IElement {
 
 impl IElement for JS<Element> {
     fn bind_to_tree_impl(&self) {
-        match self.get().get_attribute(Null, "id") {
+        match self.get_attribute(Null, "id") {
             Some(attr) => {
                 let mut doc = document_from_node(self);
                 doc.get_mut().register_named_element(self, attr.get().Value());
@@ -624,7 +636,7 @@ impl IElement for JS<Element> {
     }
 
     fn unbind_from_tree_impl(&self) {
-        match self.get().get_attribute(Null, "id") {
+        match self.get_attribute(Null, "id") {
             Some(attr) => {
                 let mut doc = document_from_node(self);
                 doc.get_mut().unregister_named_element(attr.get().Value());

src/components/script/dom/htmlcollection.rs

@@ -6,7 +6,7 @@ use dom::bindings::codegen::InheritTypes::{ElementCast};
 use dom::bindings::codegen::HTMLCollectionBinding;
 use dom::bindings::js::JS;
 use dom::bindings::utils::{Reflectable, Reflector, reflect_dom_object};
-use dom::element::Element;
+use dom::element::{Element, AttributeHandlers};
 use dom::node::{Node, NodeHelpers};
 use dom::window::Window;
 use servo_util::namespace::Namespace;
@@ -59,7 +59,7 @@ impl HTMLCollection {
     pub fn by_class_name(window: &JS<Window>, root: &JS<Node>, classes: DOMString) -> JS<HTMLCollection> {
         // FIXME: https://github.com/mozilla/servo/issues/1840
         let classes: ~[&str] = classes.split(' ').collect();
-        HTMLCollection::create(window, root, |elem| classes.iter().all(|class| elem.get().has_class(*class)))
+        HTMLCollection::create(window, root, |elem| classes.iter().all(|class| elem.has_class(*class)))
     }
 }
 
@@ -87,7 +87,6 @@ impl HTMLCollection {
 
         // Step 2.
         self.elements.iter().find(|elem| {
-            let elem = elem.get();
             elem.get_string_attribute("name") == key || elem.get_string_attribute("id") == key
         }).map(|maybe_elem| maybe_elem.clone())
     }

src/components/script/dom/htmliframeelement.rs

@@ -7,7 +7,8 @@ use dom::bindings::codegen::InheritTypes::{ElementCast, HTMLIFrameElementDerived
 use dom::bindings::js::JS;
 use dom::bindings::error::ErrorResult;
 use dom::document::Document;
-use dom::element::HTMLIFrameElementTypeId;
+use dom::element::{HTMLIFrameElementTypeId, Element};
+use dom::element::{AttributeHandlers, AfterSetAttrListener, BeforeRemoveAttrListener};
 use dom::eventtarget::{EventTarget, NodeTargetTypeId};
 use dom::htmlelement::HTMLElement;
 use dom::node::{Node, ElementNodeTypeId};
@@ -114,40 +115,14 @@ impl HTMLIFrameElement {
         Ok(())
     }
 
-    pub fn Sandbox(&self, _abstract_self: &JS<HTMLIFrameElement>) -> DOMString {
+    pub fn Sandbox(&self, abstract_self: &JS<HTMLIFrameElement>) -> DOMString {
-        self.htmlelement.element.get_string_attribute("sandbox")
+        let element: JS<Element> = ElementCast::from(abstract_self);
+        element.get_string_attribute("sandbox")
     }
 
-    pub fn SetSandbox(&mut self, abstract_self: &JS<HTMLIFrameElement>, sandbox: DOMString) {
+    pub fn SetSandbox(&mut self, abstract_self: &mut JS<HTMLIFrameElement>, sandbox: DOMString) {
-        self.htmlelement.element.set_string_attribute(&ElementCast::from(abstract_self),
+        let mut element: JS<Element> = ElementCast::from(abstract_self);
-                                                      "sandbox",
+        element.set_string_attribute("sandbox", sandbox);
-                                                      sandbox);
-    }
-
-    pub fn AfterSetAttr(&mut self, name: DOMString, value: DOMString) {
-        if "sandbox" == name {
-            let mut modes = AllowNothing as u8;
-            for word in value.split(' ') {
-                // FIXME: Workaround for https://github.com/mozilla/rust/issues/10683
-                let word_lower = word.to_ascii_lower();
-                modes |= match word_lower.as_slice() {
-                    "allow-same-origin" => AllowSameOrigin,
-                    "allow-forms" => AllowForms,
-                    "allow-pointer-lock" => AllowPointerLock,
-                    "allow-popups" => AllowPopups,
-                    "allow-scripts" => AllowScripts,
-                    "allow-top-navigation" => AllowTopNavigation,
-                    _ => AllowNothing
-                } as u8;
-            }
-            self.sandbox = Some(modes);
-        }
-    }
-
-    pub fn BeforeRemoveAttr(&mut self, name: DOMString) {
-        if "sandbox" == name {
-            self.sandbox = None;
-        }
     }
 
     pub fn AllowFullscreen(&self) -> bool {
@@ -234,3 +209,33 @@ impl HTMLIFrameElement {
         None
     }
 }
+
+impl AfterSetAttrListener for JS<HTMLIFrameElement> {
+    fn AfterSetAttr(&mut self, name: DOMString, value: DOMString) {
+        if "sandbox" == name {
+            let mut modes = AllowNothing as u8;
+            for word in value.split(' ') {
+                // FIXME: Workaround for https://github.com/mozilla/rust/issues/10683
+                let word_lower = word.to_ascii_lower();
+                modes |= match word_lower.as_slice() {
+                    "allow-same-origin" => AllowSameOrigin,
+                    "allow-forms" => AllowForms,
+                    "allow-pointer-lock" => AllowPointerLock,
+                    "allow-popups" => AllowPopups,
+                    "allow-scripts" => AllowScripts,
+                    "allow-top-navigation" => AllowTopNavigation,
+                    _ => AllowNothing
+                } as u8;
+            }
+            self.get_mut().sandbox = Some(modes);
+        }
+    }
+}
+
+impl BeforeRemoveAttrListener for JS<HTMLIFrameElement> {
+    fn BeforeRemoveAttr(&mut self, name: DOMString) {
+        if "sandbox" == name {
+            self.get_mut().sandbox = None;
+        }
+    }
+}

src/components/script/dom/htmlimageelement.rs

@@ -9,6 +9,7 @@ use dom::bindings::js::JS;
 use dom::bindings::error::ErrorResult;
 use dom::document::Document;
 use dom::element::{Element, HTMLImageElementTypeId};
+use dom::element::{AttributeHandlers, AfterSetAttrListener, BeforeRemoveAttrListener};
 use dom::eventtarget::{EventTarget, NodeTargetTypeId};
 use dom::htmlelement::HTMLElement;
 use dom::node::{Node, ElementNodeTypeId, NodeHelpers, window_from_node};
@@ -91,21 +92,6 @@ impl HTMLImageElement {
         }
     }
 
-    pub fn AfterSetAttr(&mut self, name: DOMString, value: DOMString) {
-        if "src" == name {
-            let document = self.htmlelement.element.node.owner_doc().clone();
-            let window = document.get().window.get();
-            let url = Some(window.get_url());
-            self.update_image(Some(value), url);
-        }
-    }
-
-    pub fn BeforeRemoveAttr(&mut self, name: DOMString) {
-        if "src" == name {
-            self.update_image(None, None);
-        }
-    }
-
     pub fn Alt(&self) -> DOMString {
         ~""
     }
@@ -118,9 +104,10 @@ impl HTMLImageElement {
         ~""
     }
 
-    pub fn SetSrc(&mut self, abstract_self: &JS<HTMLImageElement>, src: DOMString) -> ErrorResult {
+    pub fn SetSrc(&mut self, abstract_self: &mut JS<HTMLImageElement>, src: DOMString) -> ErrorResult {
-        let node = &mut self.htmlelement.element;
+        let mut element: JS<Element> = ElementCast::from(abstract_self);
-        node.set_attr(&ElementCast::from(abstract_self), ~"src", src.clone())
+        element.set_url_attribute("src", src);
+        Ok(())
     }
 
     pub fn CrossOrigin(&self) -> DOMString {
@@ -162,8 +149,7 @@ impl HTMLImageElement {
 
     pub fn SetWidth(&mut self, abstract_self: &JS<HTMLImageElement>, width: u32) -> ErrorResult {
         let mut elem: JS<Element> = ElementCast::from(abstract_self);
-        let mut elem_clone = elem.clone();
+        elem.set_attr(~"width", width.to_str())
-        elem.get_mut().set_attr(&mut elem_clone, ~"width", width.to_str())
     }
 
     pub fn Height(&self, abstract_self: &JS<HTMLImageElement>) -> u32 {
@@ -181,8 +167,8 @@ impl HTMLImageElement {
     }
 
     pub fn SetHeight(&mut self, abstract_self: &JS<HTMLImageElement>, height: u32) -> ErrorResult {
-        let node = &mut self.htmlelement.element;
+        let mut elem: JS<Element> = ElementCast::from(abstract_self);
-        node.set_attr(&ElementCast::from(abstract_self), ~"height", height.to_str())
+        elem.set_attr(~"height", height.to_str())
     }
 
     pub fn NaturalWidth(&self) -> u32 {
@@ -245,3 +231,21 @@ impl HTMLImageElement {
         Ok(())
     }
 }
+
+impl AfterSetAttrListener for JS<HTMLImageElement> {
+    fn AfterSetAttr(&mut self, name: DOMString, value: DOMString) {
+        if "src" == name {
+            let window = window_from_node(self);
+            let url = Some(window.get().get_url());
+            self.get_mut().update_image(Some(value), url);
+        }
+    }
+}
+
+impl BeforeRemoveAttrListener for JS<HTMLImageElement> {
+    fn BeforeRemoveAttr(&mut self, name: DOMString) {
+        if "src" == name {
+            self.get_mut().update_image(None, None);
+        }
+    }
+}

src/components/script/dom/htmlobjectelement.rs

@@ -4,14 +4,16 @@
 
 use dom::bindings::codegen::HTMLObjectElementBinding;
 use dom::bindings::codegen::InheritTypes::HTMLObjectElementDerived;
+use dom::bindings::codegen::InheritTypes::ElementCast;
 use dom::bindings::js::JS;
 use dom::bindings::error::ErrorResult;
 use dom::document::Document;
-use dom::element::HTMLObjectElementTypeId;
+use dom::element::{Element, HTMLObjectElementTypeId};
+use dom::element::{AttributeHandlers, AfterSetAttrListener};
 use dom::eventtarget::{EventTarget, NodeTargetTypeId};
 use dom::htmlelement::HTMLElement;
 use dom::htmlformelement::HTMLFormElement;
-use dom::node::{Node, ElementNodeTypeId};
+use dom::node::{Node, ElementNodeTypeId, NodeHelpers, window_from_node};
 use dom::validitystate::ValidityState;
 use dom::windowproxy::WindowProxy;
 use servo_util::str::DOMString;
@@ -50,12 +52,15 @@ impl HTMLObjectElement {
     }
 }
 
-impl HTMLObjectElement {
+trait ProcessDataURL {
+    fn process_data_url(&mut self, image_cache: ImageCacheTask, url: Option<Url>);
+}
 
+impl ProcessDataURL for JS<HTMLObjectElement> {
     // Makes the local `data` member match the status of the `data` attribute and starts
     /// prefetching the image. This method must be called after `data` is changed.
-    pub fn process_data_url(&mut self, image_cache: ImageCacheTask, url: Option<Url>) {
+    fn process_data_url(&mut self, image_cache: ImageCacheTask, url: Option<Url>) {
-        let elem = &mut self.htmlelement.element;
+        let elem: JS<Element> = ElementCast::from(self);
 
         // TODO: support other values
         match (elem.get_attribute(Null, "type").map(|x| x.get().Value()),
@@ -70,16 +75,9 @@ impl HTMLObjectElement {
             _ => { }
         }
     }
+}
 
-    pub fn AfterSetAttr(&mut self, name: DOMString, _value: DOMString) {
+impl HTMLObjectElement {
-        if "data" == name {
-            let document = self.htmlelement.element.node.owner_doc().clone();
-            let window = document.get().window.clone();
-            let url = Some(window.get().get_url());
-            self.process_data_url(window.get().image_cache_task.clone(), url);
-        }
-    }
-
     pub fn Data(&self) -> DOMString {
         ~""
     }
@@ -245,3 +243,13 @@ impl HTMLObjectElement {
         None
     }
 }
+
+impl AfterSetAttrListener for JS<HTMLObjectElement> {
+    fn AfterSetAttr(&mut self, name: DOMString, _value: DOMString) {
+        if "data" == name {
+            let window = window_from_node(self);
+            let url = Some(window.get().get_url());
+            self.process_data_url(window.get().image_cache_task.clone(), url);
+        }
+    }
+}

src/components/script/dom/htmlscriptelement.rs

@@ -4,10 +4,11 @@
 
 use dom::bindings::codegen::HTMLScriptElementBinding;
 use dom::bindings::codegen::InheritTypes::HTMLScriptElementDerived;
+use dom::bindings::codegen::InheritTypes::ElementCast;
 use dom::bindings::js::JS;
 use dom::bindings::error::ErrorResult;
 use dom::document::Document;
-use dom::element::HTMLScriptElementTypeId;
+use dom::element::{HTMLScriptElementTypeId, Element, AttributeHandlers};
 use dom::eventtarget::{EventTarget, NodeTargetTypeId};
 use dom::htmlelement::HTMLElement;
 use dom::node::{Node, ElementNodeTypeId};
@@ -41,11 +42,12 @@ impl HTMLScriptElement {
 }
 
 impl HTMLScriptElement {
-    pub fn Src(&self) -> DOMString {
+    pub fn Src(&self, abstract_self: &JS<HTMLScriptElement>) -> DOMString {
-        self.htmlelement.element.get_url_attribute("src")
+        let element: JS<Element> = ElementCast::from(abstract_self);
+        element.get_url_attribute("src")
     }
 
-    pub fn SetSrc(&mut self, _src: DOMString) -> ErrorResult {
+    pub fn SetSrc(&mut self, _abstract_self: &JS<HTMLScriptElement>, _src: DOMString) -> ErrorResult {
         Ok(())
     }
 

src/components/script/html/hubbub_html_parser.rs

@@ -7,7 +7,7 @@ use dom::bindings::codegen::InheritTypes::HTMLIFrameElementCast;
 use dom::bindings::js::JS;
 use dom::bindings::utils::Reflectable;
 use dom::document::Document;
-use dom::element::{HTMLLinkElementTypeId, HTMLIFrameElementTypeId};
+use dom::element::{AttributeHandlers, HTMLLinkElementTypeId, HTMLIFrameElementTypeId};
 use dom::htmlelement::HTMLElement;
 use dom::htmlheadingelement::{Heading1, Heading2, Heading3, Heading4, Heading5, Heading6};
 use dom::htmliframeelement::IFrameSize;
@@ -337,11 +337,9 @@ pub fn parse_html(page: &Page,
 
             debug!("-- attach attrs");
             for attr in tag.attributes.iter() {
-                let elem = element.clone();
                 //FIXME: this should have proper error handling or explicitly drop
                 //       exceptions on the ground
-                assert!(element.get_mut().set_attr(&elem,
+                assert!(element.set_attr(attr.name.clone(),
-                                                   attr.name.clone(),
                                          attr.value.clone()).is_ok());
             }
 
@@ -349,8 +347,8 @@ pub fn parse_html(page: &Page,
             match element.get().node.type_id {
                 // Handle CSS style sheets from <link> elements
                 ElementNodeTypeId(HTMLLinkElementTypeId) => {
-                    match (element.get().get_attribute(Null, "rel"),
+                    match (element.get_attribute(Null, "rel"),
-                           element.get().get_attribute(Null, "href")) {
+                           element.get_attribute(Null, "href")) {
                         (Some(ref rel), Some(ref href)) if rel.get()
                                                               .value_ref()
                                                               .split(HTML_SPACE_CHARACTERS.
@@ -372,7 +370,7 @@ pub fn parse_html(page: &Page,
                         HTMLIFrameElementCast::to(&element).unwrap();
                     let sandboxed = iframe_element.get().is_sandboxed();
                     let elem: JS<Element> = ElementCast::from(&iframe_element);
-                    let src_opt = elem.get().get_attribute(Null, "src").map(|x| x.get().Value());
+                    let src_opt = elem.get_attribute(Null, "src").map(|x| x.get().Value());
                     for src in src_opt.iter() {
                         let iframe_url = parse_url(*src, Some(url2.clone()));
                         iframe_element.get_mut().set_frame(iframe_url.clone());
@@ -463,7 +461,7 @@ pub fn parse_html(page: &Page,
         complete_script: |script| {
             unsafe {
                 let script: JS<Element> = NodeWrapping::from_hubbub_node(script);
-                match script.get().get_attribute(Null, "src") {
+                match script.get_attribute(Null, "src") {
                     Some(src) => {
                         debug!("found script: {:s}", src.get().Value());
                         let new_url = parse_url(src.get().value_ref(), Some(url3.clone()));

src/components/script/script_task.rs

@@ -10,7 +10,7 @@ use dom::bindings::codegen::InheritTypes::{EventTargetCast, NodeCast, ElementCas
 use dom::bindings::js::JS;
 use dom::bindings::utils::{Reflectable, GlobalStaticData, with_gc_enabled};
 use dom::document::{Document, HTMLDocument};
-use dom::element::Element;
+use dom::element::{Element, AttributeHandlers};
 use dom::event::{Event_, ResizeEvent, ReflowEvent, ClickEvent, MouseDownEvent, MouseMoveEvent, MouseUpEvent};
 use dom::event::Event;
 use dom::uievent::UIEvent;
@@ -916,7 +916,7 @@ impl ScriptTask {
                 let mut anchors = doc_node.traverse_preorder().filter(|node| node.is_anchor_element());
                 anchors.find(|node| {
                     let elem: JS<Element> = ElementCast::to(node).unwrap();
-                    elem.get().get_attribute(Null, "name").map_or(false, |attr| {
+                    elem.get_attribute(Null, "name").map_or(false, |attr| {
                         attr.get().value_ref() == fragid
                     })
                 }).map(|node| ElementCast::to(&node).unwrap())
@@ -1029,7 +1029,7 @@ impl ScriptTask {
                         if node.is_element() {
                             let element: JS<Element> = ElementCast::to(&node).unwrap();
                             if "a" == element.get().tag_name {
-                                self.load_url_from_element(page, element.get())
+                                self.load_url_from_element(page, &element)
                             }
                         }
                     },
@@ -1113,7 +1113,7 @@ impl ScriptTask {
         }
     }
 
-    fn load_url_from_element(&self, page: &Page, element: &Element) {
+    fn load_url_from_element(&self, page: &Page, element: &JS<Element>) {
         // if the node's element is "a," load url from href attr
         let attr = element.get_attribute(Null, "href");
         for href in attr.iter() {