Make Attr::SetValue to reflect attribute change.

src/components/script/dom/attr.rs

@@ -3,9 +3,14 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use dom::bindings::codegen::AttrBinding;
+use dom::bindings::codegen::InheritTypes::NodeCast;
 use dom::bindings::js::JS;
 use dom::bindings::utils::{Reflectable, Reflector, reflect_dom_object};
+use dom::element::Element;
+use dom::node::Node;
 use dom::window::Window;
+use dom::virtualmethods::vtable_for;
+use servo_util::namespace;
 use servo_util::namespace::Namespace;
 use servo_util::str::DOMString;
 
@@ -16,7 +21,10 @@ pub struct Attr {
     value: DOMString,
     name: DOMString,
     namespace: Namespace,
-    prefix: Option<DOMString>
+    prefix: Option<DOMString>,
+
+    /// the element that owns this attribute.
+    owner: JS<Element>,
 }
 
 impl Reflectable for Attr {
@@ -32,26 +40,38 @@ impl Reflectable for Attr {
 impl Attr {
     fn new_inherited(local_name: DOMString, value: DOMString,
                      name: DOMString, namespace: Namespace,
-                     prefix: Option<DOMString>) -> Attr {
+                     prefix: Option<DOMString>, owner: JS<Element>) -> Attr {
         Attr {
             reflector_: Reflector::new(),
             local_name: local_name,
             value: value,
             name: name, //TODO: Intern attribute names
             namespace: namespace,
-            prefix: prefix
+            prefix: prefix,
+            owner: owner,
         }
     }
 
     pub fn new(window: &JS<Window>, local_name: DOMString, value: DOMString,
                name: DOMString, namespace: Namespace,
-               prefix: Option<DOMString>) -> JS<Attr> {
+               prefix: Option<DOMString>, owner: JS<Element>) -> JS<Attr> {
-        let attr = Attr::new_inherited(local_name, value, name, namespace, prefix);
+        let attr = Attr::new_inherited(local_name, value, name, namespace, prefix, owner);
         reflect_dom_object(~attr, window, AttrBinding::Wrap)
     }
 
     pub fn set_value(&mut self, value: DOMString) {
+        let node: JS<Node> = NodeCast::from(&self.owner);
+        let namespace_is_null = self.namespace == namespace::Null;
+
+        if namespace_is_null {
+            vtable_for(&node).before_remove_attr(self.local_name.clone(), self.value.clone());
+        }
+
         self.value = value;
+
+        if namespace_is_null {
+            vtable_for(&node).after_set_attr(self.local_name.clone(), self.value.clone());
+        }
     }
 
     pub fn value_ref<'a>(&'a self) -> &'a str {

src/components/script/dom/element.rs

@@ -265,23 +265,20 @@ impl AttributeHandlers for JS<Element> {
         let idx = self.get().attrs.iter().position(cb);
         match idx {
             Some(idx) => {
-                if namespace == namespace::Null {
-                    let old_value = self.get().attrs[idx].get().Value();
-                    vtable_for(&node).before_remove_attr(local_name.clone(), old_value);
-                }
                 self.get_mut().attrs[idx].get_mut().set_value(value.clone());
             }
 
             None => {
                 let doc = node.get().owner_doc().get();
                 let new_attr = Attr::new(&doc.window, local_name.clone(), value.clone(),
-                                         name, namespace.clone(), prefix);
+                                         name, namespace.clone(), prefix, self.clone());
                 self.get_mut().attrs.push(new_attr);
-            }
-        }
 
-        if namespace == namespace::Null {
+                // FIXME: This part should be handled in `Attr`.
-            vtable_for(&node).after_set_attr(local_name, value);
+                if namespace == namespace::Null {
+                    vtable_for(&node).after_set_attr(local_name, value);
+                }
+            }
         }
     }
 

src/components/script/dom/node.rs

@@ -1365,6 +1365,10 @@ impl Node {
                 let node_elem: JS<Element> = ElementCast::to(node).unwrap();
                 let node_elem = node_elem.get();
                 let mut copy_elem: JS<Element> = ElementCast::to(&copy).unwrap();
+
+                // XXX: to avoid double borrowing compile error. we might be able to fix this after #1854
+                let copy_elem_alias: JS<Element> = copy_elem.clone();
+
                 let copy_elem = copy_elem.get_mut();
                 // FIXME: https://github.com/mozilla/servo/issues/1737
                 copy_elem.namespace = node_elem.namespace.clone();
@@ -1373,7 +1377,7 @@ impl Node {
                     copy_elem.attrs.push(Attr::new(&document.get().window,
                                                    attr.local_name.clone(), attr.value.clone(),
                                                    attr.name.clone(), attr.namespace.clone(),
-                                                   attr.prefix.clone()));
+                                                   attr.prefix.clone(), copy_elem_alias.clone()));
                 }
             },
             _ => ()