Auto merge of #7452 - nox:cleanup-attributes, r=nox

Introduce VirtualMethods::attribute_mutated()



<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7452)
<!-- Reviewable:end -->

components/script/dom/attr.rs

@@ -9,7 +9,7 @@ use dom::bindings::global::GlobalRef;
 use dom::bindings::js::{JS, MutNullableHeap};
 use dom::bindings::js::{Root, RootedReference, LayoutJS};
 use dom::bindings::utils::{Reflector, reflect_dom_object};
-use dom::element::Element;
+use dom::element::{AttributeMutation, Element};
 use dom::virtualmethods::vtable_for;
 use dom::window::Window;
 
@@ -23,12 +23,6 @@ use std::cell::Ref;
 use std::mem;
 use std::ops::Deref;
 
-#[derive(HeapSizeOf)]
-pub enum AttrSettingType {
-    FirstSetAttr,
-    ReplacedAttr,
-}
-
 #[derive(JSTraceable, PartialEq, Clone, HeapSizeOf)]
 pub enum AttrValue {
     String(DOMString),
@@ -94,6 +88,17 @@ impl AttrValue {
             _ => None
         }
     }
+
+    /// Return the AttrValue as its integer representation, if any.
+    /// This corresponds to attribute values returned as `AttrValue::UInt(_)`
+    /// by `VirtualMethods::parse_plain_attribute()`.
+    pub fn uint(&self) -> Option<u32> {
+        if let AttrValue::UInt(_, value) = *self {
+            Some(value)
+        } else {
+            None
+        }
+    }
 }
 
 impl Deref for AttrValue {
@@ -179,7 +184,7 @@ impl AttrMethods for Attr {
             None => *self.value.borrow_mut() = AttrValue::String(value),
             Some(owner) => {
                 let value = owner.r().parse_attribute(&self.namespace, self.local_name(), value);
-                self.set_value(AttrSettingType::ReplacedAttr, value, owner.r());
+                self.set_value(value, owner.r());
             }
         }
     }
@@ -236,22 +241,12 @@ impl AttrMethods for Attr {
 
 
 impl Attr {
-    pub fn set_value(&self, set_type: AttrSettingType, value: AttrValue, owner: &Element) {
+    pub fn set_value(&self, mut value: AttrValue, owner: &Element) {
         assert!(Some(owner) == self.owner().r());
-
+        mem::swap(&mut *self.value.borrow_mut(), &mut value);
-        let node = NodeCast::from_ref(owner);
+        if self.namespace == ns!("") {
-        let namespace_is_null = self.namespace == ns!("");
+            vtable_for(NodeCast::from_ref(owner)).attribute_mutated(
-
+                self, AttributeMutation::Set(Some(&value)));
-        match set_type {
-            AttrSettingType::ReplacedAttr if namespace_is_null =>
-                vtable_for(&node).before_remove_attr(self),
-            _ => ()
-        }
-
-        *self.value.borrow_mut() = value;
-
-        if namespace_is_null {
-            vtable_for(&node).after_set_attr(self)
         }
     }
 

components/script/dom/blob.rs

@@ -123,9 +123,10 @@ impl BlobMethods for Blob {
         };
         let relativeContentType = match contentType {
             None => "".to_owned(),
-            Some(str) => {
+            Some(mut str) => {
                 if is_ascii_printable(&str) {
-                    str.to_ascii_lowercase()
+                    str.make_ascii_lowercase();
+                    str
                 } else {
                     "".to_owned()
                 }

components/script/dom/cssstyledeclaration.rs

@@ -122,11 +122,12 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
     }
 
     // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-getpropertyvalue
-    fn GetPropertyValue(&self, property: DOMString) -> DOMString {
+    fn GetPropertyValue(&self, mut property: DOMString) -> DOMString {
         let owner = self.owner.root();
 
         // Step 1
-        let property = Atom::from_slice(&property.to_ascii_lowercase());
+        property.make_ascii_lowercase();
+        let property = Atom::from_slice(&property);
 
         if self.readonly {
             // Readonly style declarations are used for getComputedStyle.
@@ -165,9 +166,10 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
     }
 
     // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-getpropertypriority
-    fn GetPropertyPriority(&self, property: DOMString) -> DOMString {
+    fn GetPropertyPriority(&self, mut property: DOMString) -> DOMString {
         // Step 1
-        let property = Atom::from_slice(&property.to_ascii_lowercase());
+        property.make_ascii_lowercase();
+        let property = Atom::from_slice(&property);
 
         // Step 2
         let longhand_properties = longhands_from_shorthand(&property);
@@ -193,7 +195,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
     }
 
     // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-setproperty
-    fn SetProperty(&self, property: DOMString, value: DOMString,
+    fn SetProperty(&self, mut property: DOMString, value: DOMString,
                    priority: DOMString) -> ErrorResult {
         // Step 1
         if self.readonly {
@@ -201,7 +203,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
         }
 
         // Step 2
-        let property = property.to_ascii_lowercase();
+        property.make_ascii_lowercase();
 
         // Step 3
         if !is_supported_property(&property) {
@@ -287,14 +289,14 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
     }
 
     // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-removeproperty
-    fn RemoveProperty(&self, property: DOMString) -> Fallible<DOMString> {
+    fn RemoveProperty(&self, mut property: DOMString) -> Fallible<DOMString> {
         // Step 1
         if self.readonly {
             return Err(Error::NoModificationAllowed);
         }
 
         // Step 2
-        let property = property.to_ascii_lowercase();
+        property.make_ascii_lowercase();
 
         // Step 3
         let value = self.GetPropertyValue(property.clone());

components/script/dom/document.rs

@@ -297,6 +297,7 @@ impl Document {
     }
 
     /// Refresh the cached first base element in the DOM.
+    /// https://github.com/w3c/web-platform-tests/issues/2122
     pub fn refresh_base_element(&self) {
         let base = NodeCast::from_ref(self)
             .traverse_preorder()
@@ -1243,7 +1244,7 @@ impl DocumentMethods for Document {
             return Err(InvalidCharacter);
         }
         if self.is_html_document {
-            local_name = local_name.to_ascii_lowercase()
+            local_name.make_ascii_lowercase();
         }
         let name = QualName::new(ns!(HTML), Atom::from_slice(&local_name));
         Ok(Element::create(name, None, self, ElementCreator::ScriptCreated))
@@ -1350,10 +1351,11 @@ impl DocumentMethods for Document {
     }
 
     // https://dom.spec.whatwg.org/#dom-document-createevent
-    fn CreateEvent(&self, interface: DOMString) -> Fallible<Root<Event>> {
+    fn CreateEvent(&self, mut interface: DOMString) -> Fallible<Root<Event>> {
         let window = self.window.root();
 
-        match &*interface.to_ascii_lowercase() {
+        interface.make_ascii_lowercase();
+        match &*interface {
             "uievents" | "uievent" => Ok(EventCast::from_root(
                 UIEvent::new_uninitialized(window.r()))),
             "mouseevents" | "mouseevent" => Ok(EventCast::from_root(

components/script/dom/element.rs

@@ -6,7 +6,7 @@
 
 use dom::activation::Activatable;
 use dom::attr::AttrValue;
-use dom::attr::{Attr, AttrSettingType, AttrHelpersForLayout};
+use dom::attr::{Attr, AttrHelpersForLayout};
 use dom::bindings::cell::DOMRefCell;
 use dom::bindings::codegen::Bindings::AttrBinding::AttrMethods;
 use dom::bindings::codegen::Bindings::ElementBinding;
@@ -32,7 +32,6 @@ use dom::bindings::error::Error::{InvalidCharacter, Syntax};
 use dom::bindings::error::{ErrorResult, Fallible};
 use dom::bindings::js::{JS, LayoutJS, MutNullableHeap};
 use dom::bindings::js::{Root, RootedReference};
-use dom::bindings::trace::RootedVec;
 use dom::bindings::utils::XMLName::InvalidXMLName;
 use dom::bindings::utils::{namespace_from_domstring, xml_name_type, validate_and_extract};
 use dom::create::create_element;
@@ -580,12 +579,11 @@ impl Element {
         &self.local_name
     }
 
-    pub fn parsed_name(&self, name: DOMString) -> Atom {
+    pub fn parsed_name(&self, mut name: DOMString) -> Atom {
         if self.html_element_in_html_document() {
-            Atom::from_slice(&name.to_ascii_lowercase())
+            name.make_ascii_lowercase();
-        } else {
-            Atom::from_slice(&name)
         }
+        Atom::from_slice(&name)
     }
 
     pub fn namespace(&self) -> &Namespace {
@@ -829,37 +827,41 @@ impl Element {
 
 
 impl Element {
+    pub fn push_new_attribute(&self,
+                              local_name: Atom,
+                              value: AttrValue,
+                              name: Atom,
+                              namespace: Namespace,
+                              prefix: Option<Atom>) {
+        let window = window_from_node(self);
+        let in_empty_ns = namespace == ns!("");
+        let attr = Attr::new(&window, local_name, value, name, namespace, prefix, Some(self));
+        self.attrs.borrow_mut().push(JS::from_rooted(&attr));
+        if in_empty_ns {
+            vtable_for(NodeCast::from_ref(self)).attribute_mutated(
+                &attr, AttributeMutation::Set(None));
+        }
+    }
+
     pub fn get_attribute(&self, namespace: &Namespace, local_name: &Atom) -> Option<Root<Attr>> {
-        let mut attributes = RootedVec::new();
+        self.attrs.borrow().iter().map(JS::root).find(|attr| {
-        self.get_attributes(local_name, &mut attributes);
+            attr.local_name() == local_name && attr.namespace() == namespace
-        attributes.r().iter()
+        })
-                  .find(|attr| attr.namespace() == namespace)
-                  .map(|attr| Root::from_ref(*attr))
     }
 
     // https://dom.spec.whatwg.org/#concept-element-attributes-get-by-name
     pub fn get_attribute_by_name(&self, name: DOMString) -> Option<Root<Attr>> {
         let name = &self.parsed_name(name);
-        self.attrs.borrow().iter().map(|attr| attr.root())
+        self.attrs.borrow().iter().map(JS::root)
              .find(|a| a.r().name() == name)
     }
 
-    // https://dom.spec.whatwg.org/#concept-element-attributes-get-by-name
-    pub fn get_attributes(&self, local_name: &Atom, attributes: &mut RootedVec<JS<Attr>>) {
-        for ref attr in self.attrs.borrow().iter() {
-            let attr = attr.root();
-            if attr.r().local_name() == local_name {
-                attributes.push(JS::from_rooted(&attr));
-            }
-        }
-    }
-
     pub fn set_attribute_from_parser(&self,
-                                 qname: QualName,
+                                     qname: QualName,
-                                 value: DOMString,
+                                     value: DOMString,
-                                 prefix: Option<Atom>) {
+                                     prefix: Option<Atom>) {
         // Don't set if the attribute already exists, so we can handle add_attrs_if_missing
-        if self.attrs.borrow().iter().map(|attr| attr.root())
+        if self.attrs.borrow().iter().map(JS::root)
                 .any(|a| *a.r().local_name() == qname.local && *a.r().namespace() == qname.ns) {
             return;
         }
@@ -872,15 +874,16 @@ impl Element {
             },
         };
         let value = self.parse_attribute(&qname.ns, &qname.local, value);
-        self.do_set_attribute(qname.local, value, name, qname.ns, prefix, |_| false)
+        self.push_new_attribute(qname.local, value, name, qname.ns, prefix);
     }
 
     pub fn set_attribute(&self, name: &Atom, value: AttrValue) {
         assert!(&**name == name.to_ascii_lowercase());
         assert!(!name.contains(":"));
 
-        self.do_set_attribute(name.clone(), value, name.clone(),
+        self.set_first_matching_attribute(
-            ns!(""), None, |attr| attr.local_name() == name);
+            name.clone(), value, name.clone(), ns!(""), None,
+            |attr| attr.local_name() == name);
     }
 
     // https://html.spec.whatwg.org/multipage/#attr-data-*
@@ -894,36 +897,27 @@ impl Element {
         // Steps 2-5.
         let name = Atom::from_slice(&name);
         let value = self.parse_attribute(&ns!(""), &name, value);
-        self.do_set_attribute(name.clone(), value, name.clone(), ns!(""), None, |attr| {
+        self.set_first_matching_attribute(
-            *attr.name() == name && *attr.namespace() == ns!("")
+            name.clone(), value, name.clone(), ns!(""), None,
-        });
+            |attr| *attr.name() == name && *attr.namespace() == ns!(""));
         Ok(())
     }
 
-    pub fn do_set_attribute<F>(&self,
+    fn set_first_matching_attribute<F>(&self,
-                           local_name: Atom,
+                                       local_name: Atom,
-                           value: AttrValue,
+                                       value: AttrValue,
-                           name: Atom,
+                                       name: Atom,
-                           namespace: Namespace,
+                                       namespace: Namespace,
-                           prefix: Option<Atom>,
+                                       prefix: Option<Atom>,
-                           cb: F)
+                                       find: F)
-        where F: Fn(&Attr) -> bool
+                                       where F: Fn(&Attr)
-    {
+                                       -> bool {
-        let idx = self.attrs.borrow().iter()
+        let attr = self.attrs.borrow().iter().map(JS::root).find(|attr| find(&attr));
-                                     .map(|attr| attr.root())
+        if let Some(attr) = attr {
-                                     .position(|attr| cb(attr.r()));
+            attr.set_value(value, self);
-        let (idx, set_type) = match idx {
+        } else {
-            Some(idx) => (idx, AttrSettingType::ReplacedAttr),
+            self.push_new_attribute(local_name, value, name, namespace, prefix);
-            None => {
-                let window = window_from_node(self);
-                let attr = Attr::new(window.r(), local_name, value.clone(),
-                                     name, namespace.clone(), prefix, Some(self));
-                self.attrs.borrow_mut().push(JS::from_rooted(&attr));
-                (self.attrs.borrow().len() - 1, AttrSettingType::FirstSetAttr)
-            }
         };
-
-        (*self.attrs.borrow())[idx].root().r().set_value(set_type, value, self);
     }
 
     pub fn parse_attribute(&self, namespace: &Namespace, local_name: &Atom,
@@ -938,43 +932,27 @@ impl Element {
 
     pub fn remove_attribute(&self, namespace: &Namespace, local_name: &Atom)
                         -> Option<Root<Attr>> {
-        self.do_remove_attribute(|attr| {
+        self.remove_first_matching_attribute(|attr| {
             attr.namespace() == namespace && attr.local_name() == local_name
         })
     }
 
     pub fn remove_attribute_by_name(&self, name: &Atom) -> Option<Root<Attr>> {
-        self.do_remove_attribute(|attr| attr.name() == name)
+        self.remove_first_matching_attribute(|attr| attr.name() == name)
     }
 
-    pub fn do_remove_attribute<F>(&self, find: F) -> Option<Root<Attr>>
+    fn remove_first_matching_attribute<F>(&self, find: F) -> Option<Root<Attr>>
         where F: Fn(&Attr) -> bool
     {
-        let idx = self.attrs.borrow().iter()
+        let idx = self.attrs.borrow().iter().map(JS::root).position(|attr| find(&attr));
-                                     .map(|attr| attr.root())
-                                     .position(|attr| find(attr.r()));
 
         idx.map(|idx| {
             let attr = (*self.attrs.borrow())[idx].root();
-            if attr.r().namespace() == &ns!("") {
-                vtable_for(&NodeCast::from_ref(self)).before_remove_attr(attr.r());
-            }
-
             self.attrs.borrow_mut().remove(idx);
-            attr.r().set_owner(None);
+            attr.set_owner(None);
-            if attr.r().namespace() == &ns!("") {
-                vtable_for(&NodeCast::from_ref(self)).after_remove_attr(attr.r().name());
-            }
-
             let node = NodeCast::from_ref(self);
-            if node.is_in_doc() {
+            if attr.namespace() == &ns!("") {
-                let document = document_from_node(self);
+                vtable_for(node).attribute_mutated(&attr, AttributeMutation::Removed);
-                let damage = if attr.r().local_name() == &atom!("style") {
-                    NodeDamage::NodeStyleDamaged
-                } else {
-                    NodeDamage::OtherNodeDamage
-                };
-                document.r().content_changed(node, damage);
             }
             attr
         })
@@ -1005,7 +983,7 @@ impl Element {
 
     pub fn has_attribute(&self, local_name: &Atom) -> bool {
         assert!(local_name.bytes().all(|b| b.to_ascii_lowercase() == b));
-        self.attrs.borrow().iter().map(|attr| attr.root()).any(|attr| {
+        self.attrs.borrow().iter().map(JS::root).any(|attr| {
             attr.r().local_name() == local_name && attr.r().namespace() == &ns!("")
         })
     }
@@ -1188,9 +1166,9 @@ impl ElementMethods for Element {
 
         // Step 3-5.
         let value = self.parse_attribute(&ns!(""), &name, value);
-        self.do_set_attribute(name.clone(), value, name.clone(), ns!(""), None, |attr| {
+        self.set_first_matching_attribute(
-            *attr.name() == name
+            name.clone(), value, name.clone(), ns!(""), None,
-        });
+            |attr| *attr.name() == name);
         Ok(())
     }
 
@@ -1203,11 +1181,9 @@ impl ElementMethods for Element {
             try!(validate_and_extract(namespace, &qualified_name));
         let qualified_name = Atom::from_slice(&qualified_name);
         let value = self.parse_attribute(&namespace, &local_name, value);
-        self.do_set_attribute(local_name.clone(), value, qualified_name,
+        self.set_first_matching_attribute(
-                              namespace.clone(), prefix, |attr| {
+            local_name.clone(), value, qualified_name, namespace.clone(), prefix,
-            *attr.local_name() == local_name &&
+            |attr| *attr.local_name() == local_name && *attr.namespace() == namespace);
-            *attr.namespace() == namespace
-        });
         Ok(())
     }
 
@@ -1474,96 +1450,52 @@ impl VirtualMethods for Element {
         Some(node as &VirtualMethods)
     }
 
-    fn after_set_attr(&self, attr: &Attr) {
+    fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) {
-        if let Some(ref s) = self.super_type() {
+        self.super_type().unwrap().attribute_mutated(attr, mutation);
-            s.after_set_attr(attr);
-        }
-
         let node = NodeCast::from_ref(self);
-        match attr.local_name() {
+        let doc = node.owner_doc();
-            &atom!("style") => {
+        let damage = match attr.local_name() {
+            &atom!(style) => {
                 // Modifying the `style` attribute might change style.
-                let doc = document_from_node(self);
+                *self.style_attribute.borrow_mut() =
-                let base_url = doc.r().base_url();
+                    mutation.new_value(attr).map(|value| {
-                let value = attr.value();
+                        parse_style_attribute(&value, &doc.base_url())
-                let style = Some(parse_style_attribute(&value, &base_url));
+                    });
-                *self.style_attribute.borrow_mut() = style;
+                NodeDamage::NodeStyleDamaged
-
+            },
-                if node.is_in_doc() {
+            &atom!(class) => {
-                    doc.r().content_changed(node, NodeDamage::NodeStyleDamaged);
-                }
-            }
-            &atom!("class") => {
                 // Modifying a class can change style.
+                NodeDamage::NodeStyleDamaged
+            },
+            &atom!(id) => {
                 if node.is_in_doc() {
-                    let document = document_from_node(self);
+                    let value = attr.value().atom().unwrap().clone();
-                    document.r().content_changed(node, NodeDamage::NodeStyleDamaged);
+                    match mutation {
-                }
+                        AttributeMutation::Set(old_value) => {
-            }
+                            if let Some(old_value) = old_value {
-            &atom!("id") => {
+                                let old_value = old_value.atom().unwrap().clone();
-                // Modifying an ID might change style.
+                                doc.unregister_named_element(self, old_value);
-                let value = attr.value();
+                            }
-                if node.is_in_doc() {
+                            if value != atom!("") {
-                    let doc = document_from_node(self);
+                                doc.register_named_element(self, value);
-                    if !value.is_empty() {
+                            }
-                        let value = value.atom().unwrap().clone();
+                        },
-                        doc.r().register_named_element(self, value);
+                        AttributeMutation::Removed => {
+                            if value != atom!("") {
+                                doc.unregister_named_element(self, value);
+                            }
+                        }
                     }
-                    doc.r().content_changed(node, NodeDamage::NodeStyleDamaged);
                 }
-            }
+                NodeDamage::NodeStyleDamaged
+            },
             _ => {
                 // Modifying any other attribute might change arbitrary things.
-                if node.is_in_doc() {
+                NodeDamage::OtherNodeDamage
-                    let document = document_from_node(self);
+            },
-                    document.r().content_changed(node, NodeDamage::OtherNodeDamage);
+        };
-                }
+        if node.is_in_doc() {
-            }
+            doc.content_changed(node, damage);
-        }
-    }
-
-    fn before_remove_attr(&self, attr: &Attr) {
-        if let Some(ref s) = self.super_type() {
-            s.before_remove_attr(attr);
-        }
-
-        let node = NodeCast::from_ref(self);
-        match attr.local_name() {
-            &atom!("style") => {
-                // Modifying the `style` attribute might change style.
-                *self.style_attribute.borrow_mut() = None;
-
-                if node.is_in_doc() {
-                    let doc = document_from_node(self);
-                    doc.r().content_changed(node, NodeDamage::NodeStyleDamaged);
-                }
-            }
-            &atom!("id") => {
-                // Modifying an ID can change style.
-                let value = attr.value();
-                if node.is_in_doc() {
-                    let doc = document_from_node(self);
-                    if !value.is_empty() {
-                        let value = value.atom().unwrap().clone();
-                        doc.r().unregister_named_element(self, value);
-                    }
-                    doc.r().content_changed(node, NodeDamage::NodeStyleDamaged);
-                }
-            }
-            &atom!("class") => {
-                // Modifying a class can change style.
-                if node.is_in_doc() {
-                    let document = document_from_node(self);
-                    document.r().content_changed(node, NodeDamage::NodeStyleDamaged);
-                }
-            }
-            _ => {
-                // Modifying any other attribute might change arbitrary things.
-                if node.is_in_doc() {
-                    let doc = document_from_node(self);
-                    doc.r().content_changed(node, NodeDamage::OtherNodeDamage);
-                }
-            }
         }
     }
 
@@ -1583,11 +1515,11 @@ impl VirtualMethods for Element {
         if !tree_in_doc { return; }
 
         if let Some(ref attr) = self.get_attribute(&ns!(""), &atom!("id")) {
-            let doc = document_from_node(self);
+            let value = attr.value();
-            let value = attr.r().Value();
             if !value.is_empty() {
+                let doc = document_from_node(self);
                 let value = Atom::from_slice(&value);
-                doc.r().register_named_element(self, value);
+                doc.register_named_element(self, value.to_owned());
             }
         }
     }
@@ -1600,11 +1532,11 @@ impl VirtualMethods for Element {
         if !tree_in_doc { return; }
 
         if let Some(ref attr) = self.get_attribute(&ns!(""), &atom!("id")) {
-            let doc = document_from_node(self);
+            let value = attr.value();
-            let value = attr.r().Value();
             if !value.is_empty() {
+                let doc = document_from_node(self);
                 let value = Atom::from_slice(&value);
-                doc.r().unregister_named_element(self, value);
+                doc.unregister_named_element(self, value.to_owned());
             }
         }
     }
@@ -1765,14 +1697,9 @@ impl<'a> ::selectors::Element for Root<Element> {
                     })
             },
             NamespaceConstraint::Any => {
-                let mut attributes: RootedVec<JS<Attr>> = RootedVec::new();
+                self.attrs.borrow().iter().map(JS::root).any(|attr| {
-                self.get_attributes(local_name, &mut attributes);
+                     attr.local_name() == local_name && test(&attr.value())
-                attributes.iter().any(|attr| {
+                })
-                        // FIXME(https://github.com/rust-lang/rust/issues/23338)
-                        let attr = attr.root();
-                        let value = attr.r().value();
-                        test(&value)
-                    })
             }
         }
     }
@@ -1878,3 +1805,23 @@ impl Element {
         self.set_click_in_progress(false);
     }
 }
+
+#[derive(Clone, Copy, PartialEq)]
+pub enum AttributeMutation<'a> {
+    /// The attribute is set, keep track of old value.
+    /// https://dom.spec.whatwg.org/#attribute-is-set
+    Set(Option<&'a AttrValue>),
+
+    /// The attribute is removed.
+    /// https://dom.spec.whatwg.org/#attribute-is-removed
+    Removed
+}
+
+impl<'a> AttributeMutation<'a> {
+    pub fn new_value<'b>(&self, attr: &'b Attr) -> Option<Ref<'b, AttrValue>> {
+        match *self {
+            AttributeMutation::Set(_) => Some(attr.value()),
+            AttributeMutation::Removed => None,
+        }
+    }
+}

components/script/dom/htmlbaseelement.rs

@@ -9,7 +9,7 @@ use dom::bindings::codegen::InheritTypes::HTMLBaseElementDerived;
 use dom::bindings::codegen::InheritTypes::HTMLElementCast;
 use dom::bindings::js::Root;
 use dom::document::Document;
-use dom::element::ElementTypeId;
+use dom::element::{AttributeMutation, ElementTypeId};
 use dom::eventtarget::{EventTarget, EventTargetTypeId};
 use dom::htmlelement::{HTMLElement, HTMLElementTypeId};
 use dom::node::{Node, NodeTypeId, document_from_node};
@@ -56,15 +56,6 @@ impl HTMLBaseElement {
         parsed.unwrap_or(base)
     }
 
-    /// Update the cached base element in response to adding or removing an
-    /// attribute.
-    pub fn add_remove_attr(&self, attr: &Attr) {
-        if *attr.local_name() == atom!("href") {
-            let document = document_from_node(self);
-            document.refresh_base_element();
-        }
-    }
-
     /// Update the cached base element in response to binding or unbinding from
     /// a tree.
     pub fn bind_unbind(&self, tree_in_doc: bool) {
@@ -84,14 +75,11 @@ impl VirtualMethods for HTMLBaseElement {
         Some(HTMLElementCast::from_ref(self) as &VirtualMethods)
     }
 
-    fn after_set_attr(&self, attr: &Attr) {
+    fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) {
-        self.super_type().unwrap().after_set_attr(attr);
+        self.super_type().unwrap().attribute_mutated(attr, mutation);
-        self.add_remove_attr(attr);
+        if *attr.local_name() == atom!(href) {
-    }
+            document_from_node(self).refresh_base_element();
-
+        }
-    fn before_remove_attr(&self, attr: &Attr) {
-        self.super_type().unwrap().before_remove_attr(attr);
-        self.add_remove_attr(attr);
     }
 
     fn bind_to_tree(&self, tree_in_doc: bool) {

components/script/dom/htmlbodyelement.rs

@@ -12,7 +12,7 @@ use dom::bindings::codegen::InheritTypes::{HTMLBodyElementDerived, HTMLElementCa
 use dom::bindings::js::Root;
 use dom::bindings::utils::Reflectable;
 use dom::document::Document;
-use dom::element::ElementTypeId;
+use dom::element::{AttributeMutation, ElementTypeId};
 use dom::eventtarget::{EventTarget, EventTargetTypeId};
 use dom::htmlelement::{HTMLElement, HTMLElementTypeId};
 use dom::node::{Node, NodeTypeId, window_from_node, document_from_node};
@@ -125,56 +125,40 @@ impl VirtualMethods for HTMLBodyElement {
         chan.send(event).unwrap();
     }
 
-    fn after_set_attr(&self, attr: &Attr) {
+    fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) {
-        if let Some(ref s) = self.super_type() {
+        self.super_type().unwrap().attribute_mutated(attr, mutation);
-            s.after_set_attr(attr);
+        match (attr.local_name(), mutation) {
-        }
+            (&atom!(bgcolor), _) => {
-
+                self.background_color.set(mutation.new_value(attr).and_then(|value| {
-        let name = attr.local_name();
+                    str::parse_legacy_color(&value).ok()
-        if name.starts_with("on") {
+                }));
-            static FORWARDED_EVENTS: &'static [&'static str] =
+            },
-                &["onfocus", "onload", "onscroll", "onafterprint", "onbeforeprint",
+            (&atom!(background), _) => {
-                  "onbeforeunload", "onhashchange", "onlanguagechange", "onmessage",
+                *self.background.borrow_mut() = mutation.new_value(attr).and_then(|value| {
-                  "onoffline", "ononline", "onpagehide", "onpageshow", "onpopstate",
+                    let base = document_from_node(self).url();
-                  "onstorage", "onresize", "onunload", "onerror"];
+                    UrlParser::new().base_url(&base).parse(&value).ok()
-            let window = window_from_node(self);
+                });
-            let (cx, url, reflector) = (window.r().get_cx(),
+            },
-                                        window.r().get_url(),
+            (name, AttributeMutation::Set(_)) if name.starts_with("on") => {
-                                        window.r().reflector().get_jsobject());
+                static FORWARDED_EVENTS: &'static [&'static str] =
-            let evtarget =
+                    &["onfocus", "onload", "onscroll", "onafterprint", "onbeforeprint",
-                if FORWARDED_EVENTS.iter().any(|&event| &**name == event) {
+                      "onbeforeunload", "onhashchange", "onlanguagechange", "onmessage",
-                    EventTargetCast::from_ref(window.r())
+                      "onoffline", "ononline", "onpagehide", "onpageshow", "onpopstate",
-                } else {
+                      "onstorage", "onresize", "onunload", "onerror"];
-                    EventTargetCast::from_ref(self)
+                let window = window_from_node(self);
-                };
+                let (cx, url, reflector) = (window.get_cx(),
-            evtarget.set_event_handler_uncompiled(cx, url, reflector,
+                                            window.get_url(),
-                                                  &name[2..],
+                                            window.reflector().get_jsobject());
-                                                  (**attr.value()).to_owned());
+                let evtarget =
-        }
+                    if FORWARDED_EVENTS.iter().any(|&event| &**name == event) {
-
+                        EventTargetCast::from_ref(window.r())
-        match attr.local_name() {
+                    } else {
-            &atom!("bgcolor") => {
+                        EventTargetCast::from_ref(self)
-                self.background_color.set(str::parse_legacy_color(&attr.value()).ok())
+                    };
-            }
+                evtarget.set_event_handler_uncompiled(cx, url, reflector,
-            &atom!("background") => {
+                                                      &name[2..],
-                let doc = document_from_node(self);
+                                                      (**attr.value()).to_owned());
-                let base = doc.r().url();
+            },
-
-                *self.background.borrow_mut() = UrlParser::new().base_url(&base).parse(&attr.value()).ok();
-            }
-            _ => {}
-        }
-    }
-
-    fn before_remove_attr(&self, attr: &Attr) {
-        match self.super_type() {
-            Some(ref s) => s.before_remove_attr(attr),
-            _ => {}
-        }
-
-        match attr.local_name() {
-            &atom!("bgcolor") => self.background_color.set(None),
-            &atom!("background") => *self.background.borrow_mut() = None,
             _ => {}
         }
     }

components/script/dom/htmlbuttonelement.rs

@@ -10,7 +10,7 @@ use dom::bindings::codegen::InheritTypes::{ElementCast, HTMLElementCast, HTMLBut
 use dom::bindings::codegen::InheritTypes::{HTMLButtonElementDerived, HTMLFieldSetElementDerived};
 use dom::bindings::js::Root;
 use dom::document::Document;
-use dom::element::{Element, ElementTypeId};
+use dom::element::{AttributeMutation, Element, ElementTypeId};
 use dom::event::Event;
 use dom::eventtarget::{EventTarget, EventTargetTypeId};
 use dom::htmlelement::{HTMLElement, HTMLElementTypeId};
@@ -56,7 +56,7 @@ impl HTMLButtonElement {
         HTMLButtonElement {
             htmlelement:
                 HTMLElement::new_inherited(HTMLElementTypeId::HTMLButtonElement, localName, prefix, document),
-            //TODO: implement button_type in after_set_attr
+            //TODO: implement button_type in attribute_mutated
             button_type: Cell::new(ButtonType::ButtonSubmit)
         }
     }
@@ -142,34 +142,25 @@ impl VirtualMethods for HTMLButtonElement {
         Some(htmlelement as &VirtualMethods)
     }
 
-    fn after_set_attr(&self, attr: &Attr) {
+    fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) {
-        if let Some(ref s) = self.super_type() {
+        self.super_type().unwrap().attribute_mutated(attr, mutation);
-            s.after_set_attr(attr);
-        }
-
         match attr.local_name() {
-            &atom!("disabled") => {
+            &atom!(disabled) => {
                 let node = NodeCast::from_ref(self);
-                node.set_disabled_state(true);
+                match mutation {
-                node.set_enabled_state(false);
+                    AttributeMutation::Set(Some(_)) => {}
+                    AttributeMutation::Set(None) => {
+                        node.set_disabled_state(true);
+                        node.set_enabled_state(false);
+                    },
+                    AttributeMutation::Removed => {
+                        node.set_disabled_state(false);
+                        node.set_enabled_state(true);
+                        node.check_ancestors_disabled_state_for_form_control();
+                    }
+                }
             },
-            _ => ()
+            _ => {},
-        }
-    }
-
-    fn before_remove_attr(&self, attr: &Attr) {
-        if let Some(ref s) = self.super_type() {
-            s.before_remove_attr(attr);
-        }
-
-        match attr.local_name() {
-            &atom!("disabled") => {
-                let node = NodeCast::from_ref(self);
-                node.set_disabled_state(false);
-                node.set_enabled_state(true);
-                node.check_ancestors_disabled_state_for_form_control();
-            },
-            _ => ()
         }
     }
 

components/script/dom/htmlcanvaselement.rs

@@ -14,7 +14,7 @@ use dom::bindings::js::{JS, LayoutJS, MutNullableHeap, HeapGCValue, Root};
 use dom::bindings::utils::{Reflectable};
 use dom::canvasrenderingcontext2d::{CanvasRenderingContext2D, LayoutCanvasRenderingContext2DHelpers};
 use dom::document::Document;
-use dom::element::ElementTypeId;
+use dom::element::{AttributeMutation, ElementTypeId};
 use dom::eventtarget::{EventTarget, EventTargetTypeId};
 use dom::htmlelement::{HTMLElement, HTMLElementTypeId};
 use dom::node::{Node, NodeTypeId, window_from_node};
@@ -256,46 +256,25 @@ impl VirtualMethods for HTMLCanvasElement {
         Some(element as &VirtualMethods)
     }
 
-    fn before_remove_attr(&self, attr: &Attr) {
+    fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) {
-        if let Some(ref s) = self.super_type() {
+        self.super_type().unwrap().attribute_mutated(attr, mutation);
-            s.before_remove_attr(attr);
-        }
-
         let recreate = match attr.local_name() {
-            &atom!("width") => {
+            &atom!(width) => {
-                self.width.set(DEFAULT_WIDTH);
+                let width = mutation.new_value(attr).and_then(|value| {
+                    parse_unsigned_integer(value.chars())
+                });
+                self.width.set(width.unwrap_or(DEFAULT_WIDTH));
                 true
-            }
+            },
-            &atom!("height") => {
+            &atom!(height) => {
-                self.height.set(DEFAULT_HEIGHT);
+                let height = mutation.new_value(attr).and_then(|value| {
+                    parse_unsigned_integer(value.chars())
+                });
+                self.height.set(height.unwrap_or(DEFAULT_HEIGHT));
                 true
-            }
+            },
             _ => false,
         };
-
-        if recreate {
-           self.recreate_contexts();
-        }
-    }
-
-    fn after_set_attr(&self, attr: &Attr) {
-        if let Some(ref s) = self.super_type() {
-            s.after_set_attr(attr);
-        }
-
-        let value = attr.value();
-        let recreate = match attr.local_name() {
-            &atom!("width") => {
-                self.width.set(parse_unsigned_integer(value.chars()).unwrap_or(DEFAULT_WIDTH));
-                true
-            }
-            &atom!("height") => {
-                self.height.set(parse_unsigned_integer(value.chars()).unwrap_or(DEFAULT_HEIGHT));
-                true
-            }
-            _ => false,
-        };
-
         if recreate {
             self.recreate_contexts();
         }

components/script/dom/htmlcollection.rs

@@ -68,7 +68,7 @@ impl HTMLCollection {
         HTMLCollection::create(window, root, box filter)
     }
 
-    pub fn by_tag_name(window: &Window, root: &Node, tag: DOMString)
+    pub fn by_tag_name(window: &Window, root: &Node, mut tag: DOMString)
                        -> Root<HTMLCollection> {
         if tag == "*" {
             return HTMLCollection::all_elements(window, root, None);
@@ -88,9 +88,12 @@ impl HTMLCollection {
                 }
             }
         }
+        let tag_atom = Atom::from_slice(&tag);
+        tag.make_ascii_lowercase();
+        let ascii_lower_tag = Atom::from_slice(&tag);
         let filter = TagNameFilter {
-            tag: Atom::from_slice(&tag),
+            tag: tag_atom,
-            ascii_lower_tag: Atom::from_slice(&tag.to_ascii_lowercase()),
+            ascii_lower_tag: ascii_lower_tag,
         };
         HTMLCollection::create(window, root, box filter)
     }

components/script/dom/htmlelement.rs

@@ -19,7 +19,7 @@ use dom::bindings::utils::Reflectable;
 use dom::cssstyledeclaration::{CSSStyleDeclaration, CSSModificationAccess};
 use dom::document::Document;
 use dom::domstringmap::DOMStringMap;
-use dom::element::{Element, ElementTypeId};
+use dom::element::{AttributeMutation, Element, ElementTypeId};
 use dom::eventtarget::{EventTarget, EventTargetTypeId};
 use dom::htmlinputelement::HTMLInputElement;
 use dom::htmlmediaelement::HTMLMediaElementTypeId;
@@ -319,37 +319,23 @@ impl VirtualMethods for HTMLElement {
         Some(element as &VirtualMethods)
     }
 
-    fn before_remove_attr(&self, attr: &Attr) {
+    fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) {
-        if let Some(ref s) = self.super_type() {
+        self.super_type().unwrap().attribute_mutated(attr, mutation);
-            s.before_remove_attr(attr);
+        match (attr.local_name(), mutation) {
+            (name, AttributeMutation::Set(_)) if name.starts_with("on") => {
+                let window = window_from_node(self);
+                let (cx, url, reflector) = (window.r().get_cx(),
+                                            window.r().get_url(),
+                                            window.r().reflector().get_jsobject());
+                let evtarget = EventTargetCast::from_ref(self);
+                evtarget.set_event_handler_uncompiled(cx, url, reflector,
+                                                      &name[2..],
+                                                      (**attr.value()).to_owned());
+            },
+            _ => {}
         }
     }
 
-    fn after_remove_attr(&self, name: &Atom) {
-        if let Some(ref s) = self.super_type() {
-            s.after_remove_attr(name);
-        }
-        self.update_sequentially_focusable_status();
-    }
-
-    fn after_set_attr(&self, attr: &Attr) {
-        if let Some(ref s) = self.super_type() {
-            s.after_set_attr(attr);
-        }
-
-        let name = attr.local_name();
-        if name.starts_with("on") {
-            let window = window_from_node(self);
-            let (cx, url, reflector) = (window.r().get_cx(),
-                                        window.r().get_url(),
-                                        window.r().reflector().get_jsobject());
-            let evtarget = EventTargetCast::from_ref(self);
-            evtarget.set_event_handler_uncompiled(cx, url, reflector,
-                                                  &name[2..],
-                                                  (**attr.value()).to_owned());
-        }
-        self.update_sequentially_focusable_status();
-    }
     fn bind_to_tree(&self, tree_in_doc: bool) {
         if let Some(ref s) = self.super_type() {
             s.bind_to_tree(tree_in_doc);

components/script/dom/htmlfieldsetelement.rs

@@ -9,7 +9,7 @@ use dom::bindings::codegen::InheritTypes::{HTMLElementCast, HTMLLegendElementDer
 use dom::bindings::codegen::InheritTypes::{HTMLFieldSetElementDerived, NodeCast};
 use dom::bindings::js::{Root, RootedReference};
 use dom::document::Document;
-use dom::element::{Element, ElementTypeId};
+use dom::element::{AttributeMutation, Element, ElementTypeId};
 use dom::eventtarget::{EventTarget, EventTargetTypeId};
 use dom::htmlcollection::{HTMLCollection, CollectionFilter};
 use dom::htmlelement::{HTMLElement, HTMLElementTypeId};
@@ -88,83 +88,66 @@ impl VirtualMethods for HTMLFieldSetElement {
         Some(htmlelement as &VirtualMethods)
     }
 
-    fn after_set_attr(&self, attr: &Attr) {
+    fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) {
-        if let Some(ref s) = self.super_type() {
+        self.super_type().unwrap().attribute_mutated(attr, mutation);
-            s.after_set_attr(attr);
-        }
-
         match attr.local_name() {
-            &atom!("disabled") => {
+            &atom!(disabled) => {
+                let disabled_state = match mutation {
+                    AttributeMutation::Set(None) => true,
+                    AttributeMutation::Set(Some(_)) => {
+                        // Fieldset was already disabled before.
+                        return;
+                    },
+                    AttributeMutation::Removed => false,
+                };
                 let node = NodeCast::from_ref(self);
-                node.set_disabled_state(true);
+                node.set_disabled_state(disabled_state);
-                node.set_enabled_state(false);
+                node.set_enabled_state(!disabled_state);
-                let maybe_legend = node.children()
+                let mut found_legend = false;
-                                       .find(|node| node.r().is_htmllegendelement());
+                let children = node.children().filter(|node| {
-
+                    if found_legend {
-                for child in node.children() {
+                        true
-                    if Some(child.r()) == maybe_legend.r() {
+                    } else if node.is_htmllegendelement() {
-                        continue;
+                        found_legend = true;
+                        false
+                    } else {
+                        true
                     }
-
+                });
-                    for descendant in child.r().traverse_preorder() {
+                let fields = children.flat_map(|child| {
+                    child.traverse_preorder().filter(|descendant| {
                         match descendant.r().type_id() {
                             NodeTypeId::Element(
-                                    ElementTypeId::HTMLElement(HTMLElementTypeId::HTMLButtonElement)) |
+                                    ElementTypeId::HTMLElement(
+                                        HTMLElementTypeId::HTMLButtonElement)) |
                             NodeTypeId::Element(
-                                    ElementTypeId::HTMLElement(HTMLElementTypeId::HTMLInputElement)) |
+                                    ElementTypeId::HTMLElement(
+                                        HTMLElementTypeId::HTMLInputElement)) |
                             NodeTypeId::Element(
-                                    ElementTypeId::HTMLElement(HTMLElementTypeId::HTMLSelectElement)) |
+                                    ElementTypeId::HTMLElement(
+                                        HTMLElementTypeId::HTMLSelectElement)) |
                             NodeTypeId::Element(
-                                    ElementTypeId::HTMLElement(HTMLElementTypeId::HTMLTextAreaElement)) => {
+                                    ElementTypeId::HTMLElement(
-                                descendant.r().set_disabled_state(true);
+                                        HTMLElementTypeId::HTMLTextAreaElement)) => {
-                                descendant.r().set_enabled_state(false);
+                                true
                             },
-                            _ => ()
+                            _ => false,
                         }
+                    })
+                });
+                if disabled_state {
+                    for field in fields {
+                        field.set_disabled_state(true);
+                        field.set_enabled_state(false);
+                    }
+                } else {
+                    for field in fields {
+                        field.check_disabled_attribute();
+                        field.check_ancestors_disabled_state_for_form_control();
                     }
                 }
             },
-            _ => ()
+            _ => {},
-        }
-    }
-
-    fn before_remove_attr(&self, attr: &Attr) {
-        if let Some(ref s) = self.super_type() {
-            s.before_remove_attr(attr);
-        }
-
-        match attr.local_name() {
-            &atom!("disabled") => {
-                let node = NodeCast::from_ref(self);
-                node.set_disabled_state(false);
-                node.set_enabled_state(true);
-                let maybe_legend = node.children()
-                                       .find(|node| node.r().is_htmllegendelement());
-
-                for child in node.children() {
-                    if Some(child.r()) == maybe_legend.r() {
-                        continue;
-                    }
-
-                    for descendant in child.r().traverse_preorder() {
-                        match descendant.r().type_id() {
-                            NodeTypeId::Element(
-                                    ElementTypeId::HTMLElement(HTMLElementTypeId::HTMLButtonElement)) |
-                            NodeTypeId::Element(
-                                    ElementTypeId::HTMLElement(HTMLElementTypeId::HTMLInputElement)) |
-                            NodeTypeId::Element(
-                                    ElementTypeId::HTMLElement(HTMLElementTypeId::HTMLSelectElement)) |
-                            NodeTypeId::Element(
-                                    ElementTypeId::HTMLElement(HTMLElementTypeId::HTMLTextAreaElement)) => {
-                                descendant.r().check_disabled_attribute();
-                                descendant.r().check_ancestors_disabled_state_for_form_control();
-                            },
-                            _ => ()
-                        }
-                    }
-                }
-            },
-            _ => ()
         }
     }
 }

components/script/dom/htmlfontelement.rs

@@ -8,7 +8,7 @@ use dom::bindings::codegen::Bindings::HTMLFontElementBinding::HTMLFontElementMet
 use dom::bindings::codegen::InheritTypes::{HTMLElementCast, HTMLFontElementDerived};
 use dom::bindings::js::Root;
 use dom::document::Document;
-use dom::element::ElementTypeId;
+use dom::element::{AttributeMutation, ElementTypeId};
 use dom::eventtarget::{EventTarget, EventTargetTypeId};
 use dom::htmlelement::{HTMLElement, HTMLElementTypeId};
 use dom::node::{Node, NodeTypeId};
@@ -63,27 +63,15 @@ impl VirtualMethods for HTMLFontElement {
         Some(htmlelement as &VirtualMethods)
     }
 
-    fn after_set_attr(&self, attr: &Attr) {
+    fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) {
-        if let Some(ref s) = self.super_type() {
+        self.super_type().unwrap().attribute_mutated(attr, mutation);
-            s.after_set_attr(attr);
-        }
-
         match attr.local_name() {
-            &atom!("color") => {
+                &atom!(color) => {
-                self.color.set(str::parse_legacy_color(&attr.value()).ok())
+                self.color.set(mutation.new_value(attr).and_then(|value| {
-            }
+                    str::parse_legacy_color(&value).ok()
-            _ => {}
+                }));
-        }
+            },
-    }
+            _ => {},
-
-    fn before_remove_attr(&self, attr: &Attr) {
-        if let Some(ref s) = self.super_type() {
-            s.before_remove_attr(attr);
-        }
-
-        match attr.local_name() {
-            &atom!("color") => self.color.set(None),
-            _ => ()
         }
     }
 }

components/script/dom/htmlformelement.rs

@@ -316,7 +316,7 @@ impl HTMLFormElement {
                         _ => Some(FormDatum {
                             ty: ty,
                             name: name,
-                            value: input.Value()
+                            value: value
                         })
                     }
                 }

components/script/dom/htmliframeelement.rs

@@ -17,7 +17,7 @@ use dom::bindings::js::{Root};
 use dom::bindings::utils::Reflectable;
 use dom::customevent::CustomEvent;
 use dom::document::Document;
-use dom::element::{ElementTypeId, self};
+use dom::element::{AttributeMutation, ElementTypeId, self};
 use dom::eventtarget::{EventTarget, EventTargetTypeId};
 use dom::htmlelement::{HTMLElement, HTMLElementTypeId};
 use dom::node::{Node, NodeTypeId, window_from_node};
@@ -356,16 +356,13 @@ impl VirtualMethods for HTMLIFrameElement {
         Some(htmlelement as &VirtualMethods)
     }
 
-    fn after_set_attr(&self, attr: &Attr) {
+    fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) {
-        if let Some(ref s) = self.super_type() {
+        self.super_type().unwrap().attribute_mutated(attr, mutation);
-            s.after_set_attr(attr);
-        }
-
         match attr.local_name() {
-            &atom!("sandbox") => {
+            &atom!(sandbox) => {
-                let mut modes = SandboxAllowance::AllowNothing as u8;
+                self.sandbox.set(mutation.new_value(attr).map(|value| {
-                if let Some(ref tokens) = attr.value().tokens() {
+                    let mut modes = SandboxAllowance::AllowNothing as u8;
-                    for token in *tokens {
+                    for token in value.tokens().unwrap() {
                         modes |= match &*token.to_ascii_lowercase() {
                             "allow-same-origin" => SandboxAllowance::AllowSameOrigin,
                             "allow-forms" => SandboxAllowance::AllowForms,
@@ -376,16 +373,17 @@ impl VirtualMethods for HTMLIFrameElement {
                             _ => SandboxAllowance::AllowNothing
                         } as u8;
                     }
-                }
+                    modes
-                self.sandbox.set(Some(modes));
+                }));
-            }
+            },
-            &atom!("src") => {
+            &atom!(src) => {
-                let node = NodeCast::from_ref(self);
+                if let AttributeMutation::Set(_) = mutation {
-                if node.is_in_doc() {
+                    if NodeCast::from_ref(self).is_in_doc() {
-                    self.process_the_iframe_attributes()
+                        self.process_the_iframe_attributes();
+                    }
                 }
             },
-            _ => ()
+            _ => {},
         }
     }
 
@@ -396,17 +394,6 @@ impl VirtualMethods for HTMLIFrameElement {
         }
     }
 
-    fn before_remove_attr(&self, attr: &Attr) {
-        if let Some(ref s) = self.super_type() {
-           s.before_remove_attr(attr);
-        }
-
-        match attr.local_name() {
-            &atom!("sandbox") => self.sandbox.set(None),
-            _ => ()
-        }
-    }
-
     fn bind_to_tree(&self, tree_in_doc: bool) {
         if let Some(ref s) = self.super_type() {
             s.bind_to_tree(tree_in_doc);

components/script/dom/htmlimageelement.rs

@@ -15,7 +15,7 @@ use dom::bindings::global::GlobalRef;
 use dom::bindings::js::{LayoutJS, Root};
 use dom::bindings::refcounted::Trusted;
 use dom::document::Document;
-use dom::element::ElementTypeId;
+use dom::element::{AttributeMutation, ElementTypeId};
 use dom::event::{Event, EventBubbles, EventCancelable};
 use dom::eventtarget::{EventTarget, EventTargetTypeId};
 use dom::htmlelement::{HTMLElement, HTMLElementTypeId};
@@ -108,7 +108,7 @@ impl Runnable for ImageResponseHandlerRunnable {
 impl HTMLImageElement {
     /// Makes the local `image` member match the status of the `src` attribute and starts
     /// prefetching the image. This method must be called after `src` is changed.
-    fn update_image(&self, value: Option<(DOMString, &Url)>) {
+    fn update_image(&self, value: Option<(DOMString, Url)>) {
         let node = NodeCast::from_ref(self);
         let document = node.owner_doc();
         let window = document.r().window();
@@ -120,7 +120,7 @@ impl HTMLImageElement {
                 *self.image.borrow_mut() = None;
             }
             Some((src, base_url)) => {
-                let img_url = UrlParser::new().base_url(base_url).parse(&src);
+                let img_url = UrlParser::new().base_url(&base_url).parse(&src);
                 // FIXME: handle URL parse errors more gracefully.
                 let img_url = img_url.unwrap();
                 *self.url.borrow_mut() = Some(img_url.clone());
@@ -316,29 +316,15 @@ impl VirtualMethods for HTMLImageElement {
         Some(htmlelement as &VirtualMethods)
     }
 
-    fn after_set_attr(&self, attr: &Attr) {
+    fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) {
-        if let Some(ref s) = self.super_type() {
+        self.super_type().unwrap().attribute_mutated(attr, mutation);
-            s.after_set_attr(attr);
-        }
-
         match attr.local_name() {
-            &atom!("src") => {
+            &atom!(src) => {
-                let window = window_from_node(self);
+                self.update_image(mutation.new_value(attr).map(|value| {
-                let url = window.r().get_url();
+                    ((**value).to_owned(), window_from_node(self).get_url())
-                self.update_image(Some(((**attr.value()).to_owned(), &url)));
+                }));
             },
-            _ => ()
+            _ => {},
-        }
-    }
-
-    fn before_remove_attr(&self, attr: &Attr) {
-        if let Some(ref s) = self.super_type() {
-            s.before_remove_attr(attr);
-        }
-
-        match attr.local_name() {
-            &atom!("src") => self.update_image(None),
-            _ => ()
         }
     }
 

components/script/dom/htmlinputelement.rs

@@ -16,7 +16,7 @@ use dom::bindings::codegen::InheritTypes::{HTMLInputElementDerived, HTMLFieldSet
 use dom::bindings::global::GlobalRef;
 use dom::bindings::js::{JS, LayoutJS, Root, RootedReference};
 use dom::document::Document;
-use dom::element::{Element, ElementTypeId, RawLayoutElementHelpers};
+use dom::element::{AttributeMutation, Element, ElementTypeId, RawLayoutElementHelpers};
 use dom::event::{Event, EventBubbles, EventCancelable};
 use dom::eventtarget::{EventTarget, EventTargetTypeId};
 use dom::htmlelement::{HTMLElement, HTMLElementTypeId};
@@ -449,113 +449,87 @@ impl VirtualMethods for HTMLInputElement {
         Some(htmlelement as &VirtualMethods)
     }
 
-    fn after_set_attr(&self, attr: &Attr) {
+    fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) {
-        if let Some(ref s) = self.super_type() {
+        self.super_type().unwrap().attribute_mutated(attr, mutation);
-            s.after_set_attr(attr);
-        }
-
         match attr.local_name() {
-            &atom!("disabled") => {
+            &atom!(disabled) => {
+                let disabled_state = match mutation {
+                    AttributeMutation::Set(None) => true,
+                    AttributeMutation::Set(Some(_)) => {
+                       // Input was already disabled before.
+                       return;
+                    },
+                    AttributeMutation::Removed => false,
+                };
                 let node = NodeCast::from_ref(self);
-                node.set_disabled_state(true);
+                node.set_disabled_state(disabled_state);
-                node.set_enabled_state(false);
+                node.set_enabled_state(!disabled_state);
-            }
-            &atom!("checked") => {
-                // https://html.spec.whatwg.org/multipage/#the-input-element:concept-input-checked-dirty
-                if !self.checked_changed.get() {
-                    self.update_checked_state(true, false);
-                }
-            }
-            &atom!("size") => {
-                match *attr.value() {
-                    AttrValue::UInt(_, value) => self.size.set(value),
-                    _ => panic!("Expected an AttrValue::UInt"),
-                }
-            }
-            &atom!("type") => {
-                let value = attr.value();
-                self.input_type.set(match &**value {
-                    "button" => InputType::InputButton,
-                    "submit" => InputType::InputSubmit,
-                    "reset" => InputType::InputReset,
-                    "file" => InputType::InputFile,
-                    "radio" => InputType::InputRadio,
-                    "checkbox" => InputType::InputCheckbox,
-                    "password" => InputType::InputPassword,
-                    _ => InputType::InputText,
-                });
-                if self.input_type.get() == InputType::InputRadio {
-                    self.radio_group_updated(self.get_radio_group_name()
-                                                 .as_ref()
-                                                 .map(|group| &**group));
-                }
-            }
-            &atom!("value") => {
-                if !self.value_changed.get() {
-                    self.textinput.borrow_mut().set_content((**attr.value()).to_owned());
-                }
-            }
-            &atom!("name") => {
-                if self.input_type.get() == InputType::InputRadio {
-                    let value = attr.value();
-                    self.radio_group_updated(Some(&value));
-                }
-            }
-            _ if attr.local_name() == &Atom::from_slice("placeholder") => {
-                let value = attr.value();
-                let stripped = value.chars()
-                    .filter(|&c| c != '\n' && c != '\r')
-                    .collect::<String>();
-                *self.placeholder.borrow_mut() = stripped;
-            }
-            _ => ()
-        }
-    }
-
-    fn before_remove_attr(&self, attr: &Attr) {
-        if let Some(ref s) = self.super_type() {
-            s.before_remove_attr(attr);
-        }
-
-        match attr.local_name() {
-            &atom!("disabled") => {
-                let node = NodeCast::from_ref(self);
-                node.set_disabled_state(false);
-                node.set_enabled_state(true);
                 node.check_ancestors_disabled_state_for_form_control();
+            },
+            &atom!(checked) if !self.checked_changed.get() => {
+                let checked_state = match mutation {
+                    AttributeMutation::Set(None) => true,
+                    AttributeMutation::Set(Some(_)) => {
+                       // Input was already checked before.
+                       return;
+                    },
+                    AttributeMutation::Removed => false,
+                };
+                self.update_checked_state(checked_state, false);
+            },
+            &atom!(size) => {
+                let size = mutation.new_value(attr).map(|value| {
+                    value.uint().expect("Expected an AttrValue::UInt")
+                });
+                self.size.set(size.unwrap_or(DEFAULT_INPUT_SIZE));
             }
-            &atom!("checked") => {
+            &atom!(type) => {
-                // https://html.spec.whatwg.org/multipage/#the-input-element:concept-input-checked-dirty
+                match mutation {
-                if !self.checked_changed.get() {
+                    AttributeMutation::Set(_) => {
-                    self.update_checked_state(false, false);
+                        let value = match &**attr.value() {
+                            "button" => InputType::InputButton,
+                            "submit" => InputType::InputSubmit,
+                            "reset" => InputType::InputReset,
+                            "file" => InputType::InputFile,
+                            "radio" => InputType::InputRadio,
+                            "checkbox" => InputType::InputCheckbox,
+                            "password" => InputType::InputPassword,
+                            _ => InputType::InputText,
+                        };
+                        self.input_type.set(value);
+                        if value == InputType::InputRadio {
+                            self.radio_group_updated(
+                                self.get_radio_group_name().as_ref().map(|group| &**group));
+                        }
+                    },
+                    AttributeMutation::Removed => {
+                        if self.input_type.get() == InputType::InputRadio {
+                            broadcast_radio_checked(
+                                self,
+                                self.get_radio_group_name().as_ref().map(|group| &**group));
+                        }
+                        self.input_type.set(InputType::InputText);
+                    }
                 }
-            }
+            },
-            &atom!("size") => {
+            &atom!(value) if !self.value_changed.get() => {
-                self.size.set(DEFAULT_INPUT_SIZE);
+                let value = mutation.new_value(attr).map(|value| (**value).to_owned());
-            }
+                self.textinput.borrow_mut().set_content(
-            &atom!("type") => {
+                    value.unwrap_or_else(|| "".to_owned()));
-                if self.input_type.get() == InputType::InputRadio {
+            },
-                    broadcast_radio_checked(self,
+            &atom!(name) if self.input_type.get() == InputType::InputRadio => {
-                                            self.get_radio_group_name()
+                self.radio_group_updated(
-                                                .as_ref()
+                    mutation.new_value(attr).as_ref().map(|value| &***value));
-                                                .map(|group| &**group));
+            },
+            name if name == &Atom::from_slice("placeholder") => {
+                let mut placeholder = self.placeholder.borrow_mut();
+                placeholder.clear();
+                if let AttributeMutation::Set(_) = mutation {
+                    placeholder.extend(
+                        attr.value().chars().filter(|&c| c != '\n' && c != '\r'));
                 }
-                self.input_type.set(InputType::InputText);
+            },
-            }
+            _ => {},
-            &atom!("value") => {
-                if !self.value_changed.get() {
-                    self.textinput.borrow_mut().set_content("".to_owned());
-                }
-            }
-            &atom!("name") => {
-                if self.input_type.get() == InputType::InputRadio {
-                    self.radio_group_updated(None);
-                }
-            }
-            _ if attr.local_name() == &Atom::from_slice("placeholder") => {
-                self.placeholder.borrow_mut().clear();
-            }
-            _ => ()
         }
     }
 

components/script/dom/htmllinkelement.rs

@@ -16,7 +16,7 @@ use dom::bindings::js::{RootedReference};
 use dom::bindings::refcounted::Trusted;
 use dom::document::Document;
 use dom::domtokenlist::DOMTokenList;
-use dom::element::{Element, ElementTypeId};
+use dom::element::{AttributeMutation, Element, ElementTypeId};
 use dom::event::{EventBubbles, EventCancelable, Event};
 use dom::eventtarget::{EventTarget, EventTargetTypeId};
 use dom::htmlelement::{HTMLElement, HTMLElementTypeId};
@@ -103,33 +103,26 @@ impl VirtualMethods for HTMLLinkElement {
         Some(htmlelement as &VirtualMethods)
     }
 
-    fn after_set_attr(&self, attr: &Attr) {
+    fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) {
-        if let Some(ref s) = self.super_type() {
+        self.super_type().unwrap().attribute_mutated(attr, mutation);
-            s.after_set_attr(attr);
+        if !NodeCast::from_ref(self).is_in_doc() || mutation == AttributeMutation::Removed {
-        }
-
-        let node = NodeCast::from_ref(self);
-        if !node.is_in_doc() {
             return;
         }
-
+        let rel = get_attr(ElementCast::from_ref(self), &atom!(rel));
-        let element = ElementCast::from_ref(self);
+        match attr.local_name() {
-        let rel = get_attr(element, &atom!("rel"));
+            &atom!(href) => {
-
+                if is_stylesheet(&rel) {
-        match (rel, attr.local_name()) {
-            (ref rel, &atom!("href")) => {
-                if is_stylesheet(rel) {
                     self.handle_stylesheet_url(&attr.value());
-                } else if is_favicon(rel) {
+                } else if is_favicon(&rel) {
                     self.handle_favicon_url(&attr.value());
                 }
-            }
+            },
-            (ref rel, &atom!("media")) => {
+            &atom!(media) => {
-                if is_stylesheet(rel) {
+                if is_stylesheet(&rel) {
                     self.handle_stylesheet_url(&attr.value());
                 }
-            }
+            },
-            (_, _) => ()
+            _ => {},
         }
     }
 

components/script/dom/htmlobjectelement.rs

@@ -4,14 +4,13 @@
 
 use dom::attr::Attr;
 use dom::bindings::cell::DOMRefCell;
-use dom::bindings::codegen::Bindings::AttrBinding::AttrMethods;
 use dom::bindings::codegen::Bindings::HTMLObjectElementBinding;
 use dom::bindings::codegen::Bindings::HTMLObjectElementBinding::HTMLObjectElementMethods;
 use dom::bindings::codegen::InheritTypes::HTMLObjectElementDerived;
 use dom::bindings::codegen::InheritTypes::{ElementCast, HTMLElementCast};
 use dom::bindings::js::Root;
 use dom::document::Document;
-use dom::element::ElementTypeId;
+use dom::element::{AttributeMutation, ElementTypeId};
 use dom::eventtarget::{EventTarget, EventTargetTypeId};
 use dom::htmlelement::{HTMLElement, HTMLElementTypeId};
 use dom::node::{Node, NodeTypeId, window_from_node};
@@ -67,8 +66,8 @@ impl<'a> ProcessDataURL for &'a HTMLObjectElement {
         let elem = ElementCast::from_ref(*self);
 
         // TODO: support other values
-        match (elem.get_attribute(&ns!(""), &atom!("type")).map(|x| x.r().Value()),
+        match (elem.get_attribute(&ns!(""), &atom!("type")),
-               elem.get_attribute(&ns!(""), &atom!("data")).map(|x| x.r().Value())) {
+               elem.get_attribute(&ns!(""), &atom!("data"))) {
             (None, Some(_uri)) => {
                 // TODO(gw): Prefetch the image here.
             }
@@ -102,16 +101,15 @@ impl VirtualMethods for HTMLObjectElement {
         Some(htmlelement as &VirtualMethods)
     }
 
-    fn after_set_attr(&self, attr: &Attr) {
+    fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) {
-        if let Some(ref s) = self.super_type() {
+        self.super_type().unwrap().attribute_mutated(attr, mutation);
-            s.after_set_attr(attr);
-        }
-
         match attr.local_name() {
-            &atom!("data") => {
+            &atom!(data) => {
-                self.process_data_url();
+                if let AttributeMutation::Set(_) = mutation {
+                    self.process_data_url();
+                }
             },
-            _ => ()
+            _ => {},
         }
     }
 }

components/script/dom/htmloptgroupelement.rs

@@ -9,7 +9,7 @@ use dom::bindings::codegen::InheritTypes::{HTMLElementCast, NodeCast};
 use dom::bindings::codegen::InheritTypes::{HTMLOptGroupElementDerived, HTMLOptionElementDerived};
 use dom::bindings::js::Root;
 use dom::document::Document;
-use dom::element::ElementTypeId;
+use dom::element::{AttributeMutation, ElementTypeId};
 use dom::eventtarget::{EventTarget, EventTargetTypeId};
 use dom::htmlelement::{HTMLElement, HTMLElementTypeId};
 use dom::node::{Node, NodeTypeId};
@@ -63,44 +63,36 @@ impl VirtualMethods for HTMLOptGroupElement {
         Some(htmlelement as &VirtualMethods)
     }
 
-    fn after_set_attr(&self, attr: &Attr) {
+    fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) {
-        if let Some(ref s) = self.super_type() {
+        self.super_type().unwrap().attribute_mutated(attr, mutation);
-            s.after_set_attr(attr);
-        }
-
         match attr.local_name() {
-            &atom!("disabled") => {
+            &atom!(disabled) => {
+                let disabled_state = match mutation {
+                    AttributeMutation::Set(None) => true,
+                    AttributeMutation::Set(Some(_)) => {
+                        // Option group was already disabled.
+                        return;
+                    },
+                    AttributeMutation::Removed => false,
+                };
                 let node = NodeCast::from_ref(self);
-                node.set_disabled_state(true);
+                node.set_disabled_state(disabled_state);
-                node.set_enabled_state(false);
+                node.set_enabled_state(!disabled_state);
-                for child in node.children() {
+                let options = node.children().filter(|child| {
-                    if child.r().is_htmloptionelement() {
+                    child.is_htmloptionelement()
-                        child.r().set_disabled_state(true);
+                });
-                        child.r().set_enabled_state(false);
+                if disabled_state {
+                    for option in options {
+                        option.set_disabled_state(true);
+                        option.set_enabled_state(false);
+                    }
+                } else {
+                    for option in options {
+                        option.check_disabled_attribute();
                     }
                 }
             },
-            _ => (),
+            _ => {},
-        }
-    }
-
-    fn before_remove_attr(&self, attr: &Attr) {
-        if let Some(ref s) = self.super_type() {
-            s.before_remove_attr(attr);
-        }
-
-        match attr.local_name() {
-            &atom!("disabled") => {
-                let node = NodeCast::from_ref(self);
-                node.set_disabled_state(false);
-                node.set_enabled_state(true);
-                for child in node.children() {
-                    if child.r().is_htmloptionelement() {
-                        child.r().check_disabled_attribute();
-                    }
-                }
-            },
-            _ => ()
         }
     }
 }

components/script/dom/htmloptionelement.rs

@@ -12,7 +12,7 @@ use dom::bindings::codegen::InheritTypes::{HTMLOptionElementDerived};
 use dom::bindings::codegen::InheritTypes::{HTMLScriptElementDerived};
 use dom::bindings::js::Root;
 use dom::document::Document;
-use dom::element::ElementTypeId;
+use dom::element::{AttributeMutation, ElementTypeId};
 use dom::eventtarget::{EventTarget, EventTargetTypeId};
 use dom::htmlelement::{HTMLElement, HTMLElementTypeId};
 use dom::node::{Node, NodeTypeId};
@@ -131,34 +131,24 @@ impl VirtualMethods for HTMLOptionElement {
         Some(htmlelement as &VirtualMethods)
     }
 
-    fn after_set_attr(&self, attr: &Attr) {
+    fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) {
-        if let Some(ref s) = self.super_type() {
+        self.super_type().unwrap().attribute_mutated(attr, mutation);
-            s.after_set_attr(attr);
-        }
-
         match attr.local_name() {
-            &atom!("disabled") => {
+            &atom!(disabled) => {
                 let node = NodeCast::from_ref(self);
-                node.set_disabled_state(true);
+                match mutation {
-                node.set_enabled_state(false);
+                    AttributeMutation::Set(_) => {
+                        node.set_disabled_state(true);
+                        node.set_enabled_state(false);
+                    },
+                    AttributeMutation::Removed => {
+                        node.set_disabled_state(false);
+                        node.set_enabled_state(true);
+                        node.check_parent_disabled_state_for_option();
+                    }
+                }
             },
-            _ => ()
+            _ => {},
-        }
-    }
-
-    fn before_remove_attr(&self, attr: &Attr) {
-        if let Some(ref s) = self.super_type() {
-            s.before_remove_attr(attr);
-        }
-
-        match attr.local_name() {
-            &atom!("disabled") => {
-                let node = NodeCast::from_ref(self);
-                node.set_disabled_state(false);
-                node.set_enabled_state(true);
-                node.check_parent_disabled_state_for_option();
-            },
-            _ => ()
         }
     }
 

components/script/dom/htmlscriptelement.rs

@@ -21,7 +21,7 @@ use dom::bindings::js::{JS, Root};
 use dom::bindings::refcounted::Trusted;
 use dom::bindings::trace::JSTraceable;
 use dom::document::Document;
-use dom::element::{ElementCreator, ElementTypeId};
+use dom::element::{AttributeMutation, ElementCreator, ElementTypeId};
 use dom::event::{Event, EventBubbles, EventCancelable};
 use dom::eventtarget::{EventTarget, EventTargetTypeId};
 use dom::htmlelement::{HTMLElement, HTMLElementTypeId};
@@ -236,14 +236,13 @@ impl HTMLScriptElement {
         let event_attribute = element.get_attribute(&ns!(""), &Atom::from_slice("event"));
         match (for_attribute.r(), event_attribute.r()) {
             (Some(for_attribute), Some(event_attribute)) => {
-                let for_value = for_attribute.Value()
+                let for_value = for_attribute.value().to_ascii_lowercase();
-                                             .to_ascii_lowercase();
                 let for_value = for_value.trim_matches(HTML_SPACE_CHARACTERS);
                 if for_value != "window" {
                     return NextParserState::Continue;
                 }
 
-                let event_value = event_attribute.Value().to_ascii_lowercase();
+                let event_value = event_attribute.value().to_ascii_lowercase();
                 let event_value = event_value.trim_matches(HTML_SPACE_CHARACTERS);
                 if event_value != "onload" && event_value != "onload()" {
                     return NextParserState::Continue;
@@ -268,7 +267,7 @@ impl HTMLScriptElement {
             // Step 14.
             Some(ref src) => {
                 // Step 14.1
-                let src = src.r().Value();
+                let src = src.value();
 
                 // Step 14.2
                 if src.is_empty() {
@@ -277,10 +276,10 @@ impl HTMLScriptElement {
                 }
 
                 // Step 14.3
-                match UrlParser::new().base_url(&base_url).parse(&*src) {
+                match UrlParser::new().base_url(&base_url).parse(&src) {
                     Err(_) => {
                         // Step 14.4
-                        error!("error parsing URL for script {}", src);
+                        error!("error parsing URL for script {}", &**src);
                         self.queue_error_event();
                         return NextParserState::Continue;
                     }
@@ -469,35 +468,42 @@ impl HTMLScriptElement {
 
     pub fn is_javascript(&self) -> bool {
         let element = ElementCast::from_ref(self);
-        match element.get_attribute(&ns!(""), &atom!("type")).map(|s| s.r().Value()) {
+        let type_attr = element.get_attribute(&ns!(""), &atom!("type"));
+        let is_js = match type_attr.as_ref().map(|s| s.value()) {
             Some(ref s) if s.is_empty() => {
                 // type attr exists, but empty means js
                 debug!("script type empty, inferring js");
                 true
             },
-            Some(ref s) => {
+            Some(s) => {
-                debug!("script type={}", *s);
+                debug!("script type={}", &**s);
                 SCRIPT_JS_MIMES.contains(&s.to_ascii_lowercase().trim_matches(HTML_SPACE_CHARACTERS))
             },
             None => {
                 debug!("no script type");
-                match element.get_attribute(&ns!(""), &atom!("language"))
+                let language_attr = element.get_attribute(&ns!(""), &atom!("language"));
-                             .map(|s| s.r().Value()) {
+                let is_js = match language_attr.as_ref().map(|s| s.value()) {
                     Some(ref s) if s.is_empty() => {
                         debug!("script language empty, inferring js");
                         true
                     },
-                    Some(ref s) => {
+                    Some(s) => {
-                        debug!("script language={}", *s);
+                        debug!("script language={}", &**s);
-                        SCRIPT_JS_MIMES.contains(&&*format!("text/{}", s).to_ascii_lowercase())
+                        let mut language = format!("text/{}", &**s);
+                        language.make_ascii_lowercase();
+                        SCRIPT_JS_MIMES.contains(&&*language)
                     },
                     None => {
                         debug!("no script type or language, inferring js");
                         true
                     }
-                }
+                };
+                // https://github.com/rust-lang/rust/issues/21114
+                is_js
             }
-        }
+        };
+        // https://github.com/rust-lang/rust/issues/21114
+        is_js
     }
 
     pub fn mark_already_started(&self) {
@@ -526,13 +532,17 @@ impl VirtualMethods for HTMLScriptElement {
         Some(htmlelement as &VirtualMethods)
     }
 
-    fn after_set_attr(&self, attr: &Attr) {
+    fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) {
-        if let Some(ref s) = self.super_type() {
+        self.super_type().unwrap().attribute_mutated(attr, mutation);
-            s.after_set_attr(attr);
+        match attr.local_name() {
-        }
+            &atom!("src") => {
-        let node = NodeCast::from_ref(self);
+                if let AttributeMutation::Set(_) = mutation {
-        if attr.local_name() == &atom!("src") && !self.parser_inserted.get() && node.is_in_doc() {
+                    if !self.parser_inserted.get() && NodeCast::from_ref(self).is_in_doc() {
-            self.prepare();
+                        self.prepare();
+                    }
+                }
+            },
+            _ => {},
         }
     }
 

components/script/dom/htmlselectelement.rs

@@ -11,7 +11,7 @@ use dom::bindings::codegen::UnionTypes::HTMLElementOrLong;
 use dom::bindings::codegen::UnionTypes::HTMLOptionElementOrHTMLOptGroupElement;
 use dom::bindings::js::Root;
 use dom::document::Document;
-use dom::element::ElementTypeId;
+use dom::element::{AttributeMutation, ElementTypeId};
 use dom::eventtarget::{EventTarget, EventTargetTypeId};
 use dom::htmlelement::{HTMLElement, HTMLElementTypeId};
 use dom::node::{Node, NodeTypeId, window_from_node};
@@ -109,34 +109,21 @@ impl VirtualMethods for HTMLSelectElement {
         Some(htmlelement as &VirtualMethods)
     }
 
-    fn after_set_attr(&self, attr: &Attr) {
+    fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) {
-        if let Some(ref s) = self.super_type() {
+        self.super_type().unwrap().attribute_mutated(attr, mutation);
-            s.after_set_attr(attr);
+        if attr.local_name() == &atom!(disabled) {
-        }
+            let node = NodeCast::from_ref(self);
-
+            match mutation {
-        match attr.local_name() {
+                AttributeMutation::Set(_) => {
-            &atom!("disabled") => {
+                    node.set_disabled_state(true);
-                let node = NodeCast::from_ref(self);
+                    node.set_enabled_state(false);
-                node.set_disabled_state(true);
+                },
-                node.set_enabled_state(false);
+                AttributeMutation::Removed => {
-            },
+                    node.set_disabled_state(false);
-            _ => ()
+                    node.set_enabled_state(true);
-        }
+                    node.check_ancestors_disabled_state_for_form_control();
-    }
+                }
-
+            }
-    fn before_remove_attr(&self, attr: &Attr) {
-        if let Some(ref s) = self.super_type() {
-            s.before_remove_attr(attr);
-        }
-
-        match attr.local_name() {
-            &atom!("disabled") => {
-                let node = NodeCast::from_ref(self);
-                node.set_disabled_state(false);
-                node.set_enabled_state(true);
-                node.check_ancestors_disabled_state_for_form_control();
-            },
-            _ => ()
         }
     }
 

components/script/dom/htmltablecellelement.rs

@@ -6,7 +6,7 @@ use dom::attr::{Attr, AttrValue};
 use dom::bindings::codegen::Bindings::HTMLTableCellElementBinding::HTMLTableCellElementMethods;
 use dom::bindings::codegen::InheritTypes::{HTMLElementCast, HTMLTableCellElementDerived};
 use dom::document::Document;
-use dom::element::ElementTypeId;
+use dom::element::{AttributeMutation, ElementTypeId};
 use dom::eventtarget::{EventTarget, EventTargetTypeId};
 use dom::htmlelement::{HTMLElement, HTMLElementTypeId};
 use dom::node::NodeTypeId;
@@ -103,38 +103,26 @@ impl VirtualMethods for HTMLTableCellElement {
         Some(htmlelement as &VirtualMethods)
     }
 
-    fn after_set_attr(&self, attr: &Attr) {
+    fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) {
-        if let Some(ref s) = self.super_type() {
+        self.super_type().unwrap().attribute_mutated(attr, mutation);
-            s.after_set_attr(attr);
-        }
-
         match attr.local_name() {
-            &atom!("bgcolor") => {
+            &atom!(bgcolor) => {
-                self.background_color.set(str::parse_legacy_color(&attr.value()).ok())
+                self.background_color.set(mutation.new_value(attr).and_then(|value| {
-            }
+                    str::parse_legacy_color(&value).ok()
-            &atom!("colspan") => {
+                }));
-                match *attr.value() {
-                    AttrValue::UInt(_, colspan) => {
-                        self.colspan.set(Some(max(DEFAULT_COLSPAN, colspan)))
-                    },
-                    _ => unreachable!(),
-                }
             },
-            &atom!("width") => self.width.set(str::parse_length(&attr.value())),
+            &atom!(colspan) => {
-            _ => ()
+                self.colspan.set(mutation.new_value(attr).map(|value| {
-        }
+                    max(DEFAULT_COLSPAN, value.uint().unwrap())
-    }
+                }));
-
+            },
-    fn before_remove_attr(&self, attr: &Attr) {
+            &atom!(width) => {
-        if let Some(ref s) = self.super_type() {
+                let width = mutation.new_value(attr).map(|value| {
-            s.before_remove_attr(attr);
+                    str::parse_length(&value)
-        }
+                });
-
+                self.width.set(width.unwrap_or(LengthOrPercentageOrAuto::Auto));
-        match attr.local_name() {
+            },
-            &atom!("bgcolor") => self.background_color.set(None),
+            _ => {},
-            &atom!("colspan") => self.colspan.set(None),
-            &atom!("width") => self.width.set(LengthOrPercentageOrAuto::Auto),
-            _ => ()
         }
     }
 

components/script/dom/htmltableelement.rs

@@ -11,7 +11,7 @@ use dom::bindings::codegen::InheritTypes::{ElementCast, HTMLElementCast, HTMLTab
 use dom::bindings::codegen::InheritTypes::{HTMLTableElementDerived, NodeCast};
 use dom::bindings::js::{Root, RootedReference};
 use dom::document::Document;
-use dom::element::ElementTypeId;
+use dom::element::{AttributeMutation, ElementTypeId};
 use dom::eventtarget::{EventTarget, EventTargetTypeId};
 use dom::htmlelement::{HTMLElement, HTMLElementTypeId};
 use dom::htmltablecaptionelement::HTMLTableCaptionElement;
@@ -157,39 +157,32 @@ impl VirtualMethods for HTMLTableElement {
         Some(htmlelement as &VirtualMethods)
     }
 
-    fn after_set_attr(&self, attr: &Attr) {
+    fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) {
-        if let Some(ref s) = self.super_type() {
+        self.super_type().unwrap().attribute_mutated(attr, mutation);
-            s.after_set_attr(attr);
-        }
-
         match attr.local_name() {
-            &atom!("bgcolor") => {
+            &atom!(bgcolor) => {
-                self.background_color.set(str::parse_legacy_color(&attr.value()).ok())
+                self.background_color.set(mutation.new_value(attr).and_then(|value| {
-            }
+                    str::parse_legacy_color(&value).ok()
-            &atom!("border") => {
+                }));
+            },
+            &atom!(border) => {
                 // According to HTML5 § 14.3.9, invalid values map to 1px.
-                self.border.set(Some(str::parse_unsigned_integer(attr.value()
+                self.border.set(mutation.new_value(attr).map(|value| {
-                                                                     .chars()).unwrap_or(1)))
+                    str::parse_unsigned_integer(value.chars()).unwrap_or(1)
+                }));
             }
-            &atom!("cellspacing") => {
+            &atom!(cellspacing) => {
-                self.cellspacing.set(str::parse_unsigned_integer(attr.value().chars()))
+                self.cellspacing.set(mutation.new_value(attr).and_then(|value| {
-            }
+                    str::parse_unsigned_integer(value.chars())
-            &atom!("width") => self.width.set(str::parse_length(&attr.value())),
+                }));
-            _ => ()
+            },
-        }
+            &atom!(width) => {
-    }
+                let width = mutation.new_value(attr).map(|value| {
-
+                    str::parse_length(&value)
-    fn before_remove_attr(&self, attr: &Attr) {
+                });
-        if let Some(ref s) = self.super_type() {
+                self.width.set(width.unwrap_or(LengthOrPercentageOrAuto::Auto));
-            s.before_remove_attr(attr);
+            },
-        }
+            _ => {},
-
-        match attr.local_name() {
-            &atom!("bgcolor") => self.background_color.set(None),
-            &atom!("border") => self.border.set(None),
-            &atom!("cellspacing") => self.cellspacing.set(None),
-            &atom!("width") => self.width.set(LengthOrPercentageOrAuto::Auto),
-            _ => ()
         }
     }
 

components/script/dom/htmltablerowelement.rs

@@ -7,7 +7,7 @@ use dom::bindings::codegen::Bindings::HTMLTableRowElementBinding;
 use dom::bindings::codegen::InheritTypes::{HTMLElementCast, HTMLTableRowElementDerived};
 use dom::bindings::js::Root;
 use dom::document::Document;
-use dom::element::ElementTypeId;
+use dom::element::{AttributeMutation, ElementTypeId};
 use dom::eventtarget::{EventTarget, EventTargetTypeId};
 use dom::htmlelement::{HTMLElement, HTMLElementTypeId};
 use dom::node::{Node, NodeTypeId};
@@ -62,29 +62,15 @@ impl VirtualMethods for HTMLTableRowElement {
         Some(htmlelement as &VirtualMethods)
     }
 
-    fn after_set_attr(&self, attr: &Attr) {
+    fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) {
-        if let Some(ref s) = self.super_type() {
+        self.super_type().unwrap().attribute_mutated(attr, mutation);
-            s.after_set_attr(attr);
-        }
-
         match attr.local_name() {
-            &atom!("bgcolor") => {
+            &atom!(bgcolor) => {
-                self.background_color.set(str::parse_legacy_color(&attr.value()).ok());
+                self.background_color.set(mutation.new_value(attr).and_then(|value| {
+                    str::parse_legacy_color(&value).ok()
+                }));
             },
-            _ => ()
+            _ => {},
-        }
-    }
-
-    fn before_remove_attr(&self, attr: &Attr) {
-        if let Some(ref s) = self.super_type() {
-            s.before_remove_attr(attr);
-        }
-
-        match attr.local_name() {
-            &atom!("bgcolor") => {
-                self.background_color.set(None);
-            },
-            _ => ()
         }
     }
 }

components/script/dom/htmltablesectionelement.rs

@@ -7,7 +7,7 @@ use dom::bindings::codegen::Bindings::HTMLTableSectionElementBinding;
 use dom::bindings::codegen::InheritTypes::{HTMLElementCast, HTMLTableSectionElementDerived};
 use dom::bindings::js::Root;
 use dom::document::Document;
-use dom::element::ElementTypeId;
+use dom::element::{AttributeMutation, ElementTypeId};
 use dom::eventtarget::{EventTarget, EventTargetTypeId};
 use dom::htmlelement::{HTMLElement, HTMLElementTypeId};
 use dom::node::{Node, NodeTypeId};
@@ -61,29 +61,15 @@ impl VirtualMethods for HTMLTableSectionElement {
         Some(htmlelement as &VirtualMethods)
     }
 
-    fn after_set_attr(&self, attr: &Attr) {
+    fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) {
-        if let Some(ref s) = self.super_type() {
+        self.super_type().unwrap().attribute_mutated(attr, mutation);
-            s.after_set_attr(attr);
-        }
-
         match attr.local_name() {
-            &atom!("bgcolor") => {
+            &atom!(bgcolor) => {
-                self.background_color.set(str::parse_legacy_color(&attr.value()).ok());
+                self.background_color.set(mutation.new_value(attr).and_then(|value| {
+                    str::parse_legacy_color(&value).ok()
+                }));
             },
-            _ => ()
+            _ => {},
-        }
-    }
-
-    fn before_remove_attr(&self, attr: &Attr) {
-        if let Some(ref s) = self.super_type() {
-            s.before_remove_attr(attr);
-        }
-
-        match attr.local_name() {
-            &atom!("bgcolor") => {
-                self.background_color.set(None);
-            },
-            _ => ()
         }
     }
 }

components/script/dom/htmltextareaelement.rs

@@ -15,7 +15,7 @@ use dom::bindings::global::GlobalRef;
 use dom::bindings::js::{LayoutJS, Root};
 use dom::bindings::refcounted::Trusted;
 use dom::document::Document;
-use dom::element::{Element, ElementTypeId};
+use dom::element::{AttributeMutation, Element, ElementTypeId};
 use dom::event::{Event, EventBubbles, EventCancelable};
 use dom::eventtarget::{EventTarget, EventTargetTypeId};
 use dom::htmlelement::{HTMLElement, HTMLElementTypeId};
@@ -246,52 +246,36 @@ impl VirtualMethods for HTMLTextAreaElement {
         Some(htmlelement as &VirtualMethods)
     }
 
-    fn after_set_attr(&self, attr: &Attr) {
+    fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) {
-        if let Some(ref s) = self.super_type() {
+        self.super_type().unwrap().attribute_mutated(attr, mutation);
-            s.after_set_attr(attr);
-        }
-
         match attr.local_name() {
-            &atom!("disabled") => {
+            &atom!(disabled) => {
                 let node = NodeCast::from_ref(self);
-                node.set_disabled_state(true);
+                match mutation {
-                node.set_enabled_state(false);
+                    AttributeMutation::Set(_) => {
-            },
+                        node.set_disabled_state(true);
-            &atom!("cols") => {
+                        node.set_enabled_state(false);
-                match *attr.value() {
+                    },
-                    AttrValue::UInt(_, value) => self.cols.set(value),
+                    AttributeMutation::Removed => {
-                    _ => panic!("Expected an AttrValue::UInt"),
+                        node.set_disabled_state(false);
+                        node.set_enabled_state(true);
+                        node.check_ancestors_disabled_state_for_form_control();
+                    }
                 }
             },
-            &atom!("rows") => {
+            &atom!(cols) => {
-                match *attr.value() {
+                let cols = mutation.new_value(attr).map(|value| {
-                    AttrValue::UInt(_, value) => self.rows.set(value),
+                    value.uint().expect("Expected an AttrValue::UInt")
-                    _ => panic!("Expected an AttrValue::UInt"),
+                });
-                }
+                self.cols.set(cols.unwrap_or(DEFAULT_COLS));
             },
-            _ => ()
+            &atom!(rows) => {
-        }
+                let rows = mutation.new_value(attr).map(|value| {
-    }
+                    value.uint().expect("Expected an AttrValue::UInt")
-
+                });
-    fn before_remove_attr(&self, attr: &Attr) {
+                self.rows.set(rows.unwrap_or(DEFAULT_ROWS));
-        if let Some(ref s) = self.super_type() {
-            s.before_remove_attr(attr);
-        }
-
-        match attr.local_name() {
-            &atom!("disabled") => {
-                let node = NodeCast::from_ref(self);
-                node.set_disabled_state(false);
-                node.set_enabled_state(true);
-                node.check_ancestors_disabled_state_for_form_control();
             },
-            &atom!("cols") => {
+            _ => {},
-                self.cols.set(DEFAULT_COLS);
-            },
-            &atom!("rows") => {
-                self.rows.set(DEFAULT_ROWS);
-            },
-            _ => ()
         }
     }
 

components/script/dom/virtualmethods.rs

@@ -33,7 +33,7 @@ use dom::bindings::codegen::InheritTypes::HTMLTableSectionElementCast;
 use dom::bindings::codegen::InheritTypes::HTMLTextAreaElementCast;
 use dom::bindings::codegen::InheritTypes::HTMLTitleElementCast;
 use dom::document::Document;
-use dom::element::ElementTypeId;
+use dom::element::{AttributeMutation, ElementTypeId};
 use dom::event::Event;
 use dom::htmlelement::HTMLElementTypeId;
 use dom::node::NodeTypeId;
@@ -50,27 +50,12 @@ pub trait VirtualMethods {
     /// if any.
     fn super_type(&self) -> Option<&VirtualMethods>;
 
-    /// Called when changing or adding attributes, after the attribute's value
+    /// Called when attributes of a node are mutated.
-    /// has been updated.
+    /// https://dom.spec.whatwg.org/#attribute-is-set
-    fn after_set_attr(&self, attr: &Attr) {
+    /// https://dom.spec.whatwg.org/#attribute-is-removed
-        if let Some(ref s) = self.super_type() {
+    fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) {
-            s.after_set_attr(attr);
+        if let Some(s) = self.super_type() {
-        }
+            s.attribute_mutated(attr, mutation);
-    }
-
-    /// Called when changing or removing attributes, before any modification
-    /// has taken place.
-    fn before_remove_attr(&self, attr: &Attr) {
-        if let Some(ref s) = self.super_type() {
-            s.before_remove_attr(attr);
-        }
-    }
-
-    /// Called when changing or removing attributes, after all modification
-    /// has taken place.
-    fn after_remove_attr(&self, name: &Atom) {
-        if let Some(ref s) = self.super_type() {
-            s.after_remove_attr(name);
         }
     }
 

components/script/dom/window.rs

@@ -599,7 +599,7 @@ impl WindowMethods for Window {
                         element: &Element,
                         pseudo: Option<DOMString>) -> Root<CSSStyleDeclaration> {
         // Steps 1-4.
-        let pseudo = match pseudo.map(|s| s.to_ascii_lowercase()) {
+        let pseudo = match pseudo.map(|mut s| { s.make_ascii_lowercase(); s }) {
             Some(ref pseudo) if pseudo == ":before" || pseudo == "::before" =>
                 Some(PseudoElement::Before),
             Some(ref pseudo) if pseudo == ":after" || pseudo == "::after" =>

components/script/script_task.rs

@@ -22,7 +22,6 @@
 use devtools;
 use document_loader::{LoadType, DocumentLoader, NotifierData};
 use dom::bindings::cell::DOMRefCell;
-use dom::bindings::codegen::Bindings::AttrBinding::AttrMethods;
 use dom::bindings::codegen::Bindings::DocumentBinding::{DocumentMethods, DocumentReadyState};
 use dom::bindings::codegen::InheritTypes::{ElementCast, EventTargetCast, NodeCast, EventCast};
 use dom::bindings::conversions::FromJSValConvertible;
@@ -1798,7 +1797,7 @@ impl ScriptTask {
                         let element = ElementCast::to_ref(target.r()).unwrap();
                         let status = element.get_attribute(&ns!(""), &atom!("href"))
                             .and_then(|href| {
-                                let value = href.r().Value();
+                                let value = href.value();
                                 let url = document.r().url();
                                 UrlParser::new().base_url(&url).parse(&value).map(|url| url.serialize()).ok()
                             });

tests/wpt/mozilla/tests/mozilla/enabled_disabled_selectors.html

@@ -170,6 +170,7 @@
             test(function() {
                 var optgroup = document.createElement("optgroup");
                 optgroup.disabled = true;
+                check_disabled_selector(optgroup, true);
 
                 var option = document.createElement("option");
                 check_disabled_selector(option, false);

tests/wpt/web-platform-tests/html/semantics/selectors/pseudo-classes/disabled.html

@@ -48,6 +48,9 @@
   document.getElementById("button1").setAttribute("disabled", "disabled");
   testSelector(":disabled", ["button1", "input2", "select2", "optgroup2", "option2", "textarea2", "fieldset2", "clubname", "clubnum"], "':disabled' should also match elements whose disabled attribute has been set");
 
+  document.getElementById("button1").setAttribute("disabled", "disabled");
+  testSelector(":disabled", ["button1", "input2", "select2", "optgroup2", "option2", "textarea2", "fieldset2", "clubname", "clubnum"], "':disabled' should also match elements whose disabled attribute has been set twice");
+
   document.getElementById("input2").setAttribute("type", "submit"); // change input type to submit
   testSelector(":disabled", ["button1", "input2", "select2", "optgroup2", "option2", "textarea2", "fieldset2", "clubname", "clubnum"], "':disabled' should also match disabled elements whose type has changed");