auto merge of #1302 : saneyuki/servo/getid, r=jdm

#740

src/components/script/dom/document.rs

@@ -92,7 +92,7 @@ pub struct Document {
     window: @mut Window,
     doctype: DocumentType,
     title: ~str,
-    idmap: HashMap<~str, AbstractNode<ScriptView>>
+    idmap: HashMap<DOMString, AbstractNode<ScriptView>>
 }
 
 impl Document {
@@ -326,7 +326,7 @@ impl Document {
     }
 
     pub fn register_nodes_with_id(&mut self, root: &AbstractNode<ScriptView>) {
-        foreach_ided_elements(root, |id: &~str, abstract_node: &AbstractNode<ScriptView>| {
+        foreach_ided_elements(root, |id: &DOMString, abstract_node: &AbstractNode<ScriptView>| {
             // TODO: "in tree order, within the context object's tree"
             // http://dom.spec.whatwg.org/#dom-document-getelementbyid.
             self.idmap.find_or_insert(id.clone(), *abstract_node);
@@ -334,17 +334,39 @@ impl Document {
     }
 
     pub fn unregister_nodes_with_id(&mut self, root: &AbstractNode<ScriptView>) {
-        foreach_ided_elements(root, |id: &~str, _| {
+        foreach_ided_elements(root, |id: &DOMString, _| {
             // TODO: "in tree order, within the context object's tree"
             // http://dom.spec.whatwg.org/#dom-document-getelementbyid.
             self.idmap.pop(id);
         });
     }
+
+    pub fn update_idmap(&mut self,
+                        abstract_self: AbstractNode<ScriptView>,
+                        new_id: DOMString,
+                        old_id: Option<DOMString>) {
+        // remove old ids if the old ones are not same as the new one.
+        match old_id {
+            Some(ref old_id) if new_id != *old_id => {
+                self.idmap.remove(old_id);
+            }
+            _ => ()
+        }
+
+        // TODO: support the case if multiple elements which haves same id are in the same document.
+        self.idmap.mangle(new_id, abstract_self,
+                         |_, new_node: AbstractNode<ScriptView>| -> AbstractNode<ScriptView> {
+                             new_node
+                         },
+                         |_, old_node: &mut AbstractNode<ScriptView>, new_node: AbstractNode<ScriptView>| {
+                             *old_node = new_node;
+                         });
+    }
 }
 
 #[inline(always)]
 fn foreach_ided_elements(root: &AbstractNode<ScriptView>,
-                         callback: &fn(&~str, &AbstractNode<ScriptView>)) {
+                         callback: &fn(&DOMString, &AbstractNode<ScriptView>)) {
     for node in root.traverse_preorder() {
         if !node.is_element() {
             continue;

src/components/script/dom/element.rs

@@ -216,6 +216,7 @@ impl<'self> Element {
         let win = self.node.owner_doc().document().window;
         let new_attr = Attr::new_ns(win, local_name.clone(), value.clone(),
                                     name.clone(), namespace.clone(), prefix);
+        let mut old_raw_value: Option<DOMString> = None;
         self.attrs.mangle(local_name.clone(), new_attr,
                           |new_name: &~str, new_value: @mut Attr| {
                               // register to the ordered list.
@@ -229,6 +230,7 @@ impl<'self> Element {
                               for attr in old_value.mut_iter() {
                                   if eq_slice(attr.local_name, *name) &&
                                      attr.namespace == new_value.namespace {
+                                      old_raw_value = Some(attr.Value());
                                       *attr = new_value;
                                       found = true;
                                       break;
@@ -242,7 +244,7 @@ impl<'self> Element {
                               }
                           });
 
-        self.after_set_attr(abstract_self, &namespace, local_name, value);
+        self.after_set_attr(abstract_self, &namespace, local_name, value, old_raw_value);
         Ok(())
     }
 
@@ -250,15 +252,21 @@ impl<'self> Element {
                       abstract_self: AbstractNode<ScriptView>,
                       namespace: &Namespace,
                       local_name: DOMString,
-                      value: DOMString) {
+                      value: DOMString,
+                      old_value: Option<DOMString>) {
 
-        if "style" == local_name && *namespace == namespace::Null {
-             self.style_attribute = Some(style::parse_style_attribute(value));
+        match local_name.as_slice() {
+            "style" if *namespace == namespace::Null => {
+                self.style_attribute = Some(style::parse_style_attribute(value))
+            }
+            "id" => {
+                let doc = self.node.owner_doc();
+                let doc = doc.mut_document();
+                doc.update_idmap(abstract_self, value.clone(), old_value);
+            }
+            _ => ()
         }
 
-        // TODO: update owner document's id hashmap for `document.getElementById()`
-        //       if `name` == "id".
-
         //XXXjdm We really need something like a vtable so we can call AfterSetAttr.
         //       This hardcoding is awful.
         match abstract_self.type_id() {

src/test/html/content/test_document_getElementById.html

@@ -43,8 +43,23 @@
       is(removed, null, "test2-3, removed element");
     }
 
-    // TODO:
     // test3: update `id` attribute
+    {
+      // setup fixtures.
+      let TEST_ID = "test3";
+      let test = document.createElement("div");
+      test.setAttribute("id", TEST_ID);
+      gBody.appendChild(test);
+
+      // update id
+      let UPDATED_ID = "test3-updated";
+      test.setAttribute("id", UPDATED_ID);
+      let e = document.getElementById(UPDATED_ID);
+      is(e, test, "test3-0, update 'id' attribute.");
+
+      let old = document.getElementById(TEST_ID);
+      is(old, null, "test3-1, the method shouldn't get the element by the old id.");
+    }
 
     // TODO:
     // test4: "in tree order, within the context object's tree"