From 75b83e38db7894b6397437171e06863a12a18a21 Mon Sep 17 00:00:00 2001
From: Tetsuharu OHZEKI <saneyuki.snyk@gmail.com>
Date: Thu, 3 Oct 2013 05:45:12 -0700
Subject: [PATCH 1/3] Define "isnot()" for script test.

---
 src/test/html/content/harness.js | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/test/html/content/harness.js b/src/test/html/content/harness.js
index 678d1ac4119..a7da6e7cd6e 100644
--- a/src/test/html/content/harness.js
+++ b/src/test/html/content/harness.js
@@ -13,6 +13,12 @@ function is(a, b, c) {
   f(a + " == " + b, m);
 }
 
+function isnot(a, b, c) {
+  let f = (a != b) ? _pass : _fail;
+  let m = !c ? "" : c;
+  f(a + " != " + b, m);
+}
+
 var _test_complete = false;
 var _test_timeout = 10000; //10 seconds
 function finish() {

From f9be872e614cc2ecf0487eb8878fedfd44525c53 Mon Sep 17 00:00:00 2001
From: Tetsuharu OHZEKI <saneyuki.snyk@gmail.com>
Date: Thu, 3 Oct 2013 16:42:18 +0900
Subject: [PATCH 2/3] Pass AbstractNode to Node<ScriptView>.add_to_doc().

---
 src/components/script/dom/node.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/components/script/dom/node.rs b/src/components/script/dom/node.rs
index 46ddc57ec36..293da68d0e4 100644
--- a/src/components/script/dom/node.rs
+++ b/src/components/script/dom/node.rs
@@ -460,7 +460,7 @@ impl Node<ScriptView> {
         node
     }
 
-    pub fn add_to_doc(&mut self, doc: AbstractDocument) {
+    pub fn add_to_doc(&mut self, abstract_self: AbstractNode<ScriptView>, doc: AbstractDocument) {
         let old_doc = self.owner_doc;
         self.owner_doc = doc;
         let mut cur_node = self.first_child;
@@ -753,7 +753,7 @@ impl Node<ScriptView> {
         node.parent_node().map(|parent| parent.remove_child(node));
         abstract_self.add_child(node);
         do node.with_mut_base |node| {
-            node.add_to_doc(self.owner_doc);
+            node.add_to_doc(abstract_self, self.owner_doc);
         }
         Ok(node)
     }

From db3b5c3c4cc10225d221b6579b82a13048adb35f Mon Sep 17 00:00:00 2001
From: Tetsuharu OHZEKI <saneyuki.snyk@gmail.com>
Date: Mon, 16 Sep 2013 23:00:18 +0900
Subject: [PATCH 3/3] Basic Implementation of document.getElementById(), #740

---
 src/components/script/dom/document.rs         | 53 +++++++++++++++--
 src/components/script/dom/element.rs          |  3 +
 src/components/script/dom/node.rs             | 16 +++++
 .../content/test_document_getElementById.html | 58 +++++++++++++++++++
 4 files changed, 126 insertions(+), 4 deletions(-)
 create mode 100644 src/test/html/content/test_document_getElementById.html

diff --git a/src/components/script/dom/document.rs b/src/components/script/dom/document.rs
index a8c417b986b..99c20d280c8 100644
--- a/src/components/script/dom/document.rs
+++ b/src/components/script/dom/document.rs
@@ -25,6 +25,8 @@ use js::jsapi::{JSTRACE_OBJECT, JSTracer, JS_CallTracer};
 use js::glue::RUST_OBJECT_TO_JSVAL;
 use servo_util::tree::TreeNodeRef;
 
+use std::hashmap::HashMap;
+
 use std::cast;
 use std::ptr;
 use std::str::eq_slice;
@@ -92,6 +94,9 @@ impl AbstractDocument {
         }));
         self.with_mut_base(|document| {
             document.root = Some(root);
+            // Register elements having "id" attribute to the owner doc.
+            document.register_nodes_with_id(&root);
+
             document.content_changed();
         });
     }
@@ -108,7 +113,8 @@ pub struct Document {
     reflector_: Reflector,
     window: Option<@mut Window>,
     doctype: DocumentType,
-    title: ~str
+    title: ~str,
+    idmap: HashMap<~str, AbstractNode<ScriptView>>
 }
 
 impl Document {
@@ -119,7 +125,8 @@ impl Document {
             reflector_: Reflector::new(),
             window: window,
             doctype: doctype,
-            title: ~""
+            title: ~"",
+            idmap: HashMap::new()
         }
     }
 
@@ -265,8 +272,11 @@ impl Document {
         HTMLCollection::new(~[], cx, scope)
     }
 
-    pub fn GetElementById(&self, _id: &DOMString) -> Option<AbstractNode<ScriptView>> {
-        None
+    pub fn GetElementById(&self, id: &DOMString) -> Option<AbstractNode<ScriptView>> {
+        let key: &~str = &null_str_as_empty(id);
+        // TODO: "in tree order, within the context object's tree"
+        // http://dom.spec.whatwg.org/#dom-document-getelementbyid.
+        self.idmap.find_equiv(key).map(|node| **node)
     }
 
     pub fn CreateElement(&self, abstract_self: AbstractDocument, local_name: &DOMString) -> Fallible<AbstractNode<ScriptView>> {
@@ -513,6 +523,41 @@ impl Document {
             window.wait_until_safe_to_modify_dom();
         }
     }
+
+    pub fn register_nodes_with_id(&mut self, root: &AbstractNode<ScriptView>) {
+        foreach_ided_elements(root, |id: &~str, 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);
+        });
+    }
+
+    pub fn unregister_nodes_with_id(&mut self, root: &AbstractNode<ScriptView>) {
+        foreach_ided_elements(root, |id: &~str, _| {
+            // TODO: "in tree order, within the context object's tree"
+            // http://dom.spec.whatwg.org/#dom-document-getelementbyid.
+            self.idmap.pop(id);
+        });
+    }
+}
+
+#[inline(always)]
+fn foreach_ided_elements(root: &AbstractNode<ScriptView>,
+                         callback: &fn(&~str, &AbstractNode<ScriptView>)) {
+    for node in root.traverse_preorder() {
+        if !node.is_element() {
+            loop;
+        }
+
+        do node.with_imm_element |element| {
+            match element.get_attr("id") {
+                Some(id) => {
+                    callback(&id.to_str(), &node);
+                }
+                None => ()
+            }
+        }
+    }
 }
 
 impl Traceable for Document {
diff --git a/src/components/script/dom/element.rs b/src/components/script/dom/element.rs
index 51d7c609c98..4c4238ca5d6 100644
--- a/src/components/script/dom/element.rs
+++ b/src/components/script/dom/element.rs
@@ -170,6 +170,9 @@ impl<'self> Element {
                     null_str_as_empty_ref(raw_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() {
diff --git a/src/components/script/dom/node.rs b/src/components/script/dom/node.rs
index 293da68d0e4..b68f8ffff12 100644
--- a/src/components/script/dom/node.rs
+++ b/src/components/script/dom/node.rs
@@ -473,6 +473,16 @@ impl Node<ScriptView> {
             cur_node = cur_node.unwrap().next_sibling();
         }
 
+        // Unregister elements having "id' from the old doc.
+        do old_doc.with_mut_base |old_doc| {
+            old_doc.unregister_nodes_with_id(&abstract_self);
+        }
+
+        // Register elements having "id" attribute to the owner doc.
+        do doc.with_mut_base |doc| {
+            doc.register_nodes_with_id(&abstract_self);
+        }
+
         // Signal the old document that it needs to update its display
         if old_doc != doc {
             do old_doc.with_base |old_doc| {
@@ -779,6 +789,12 @@ impl Node<ScriptView> {
 
         self.wait_until_safe_to_modify_dom();
 
+        // Unregister elements having "id' from the owner doc.
+        // This need be called before target nodes are removed from tree.
+        do self.owner_doc.with_mut_base |doc| {
+            doc.unregister_nodes_with_id(&abstract_self);
+        }
+
         abstract_self.remove_child(node);
         // Signal the document that it needs to update its display.
         do self.owner_doc.with_base |document| {
diff --git a/src/test/html/content/test_document_getElementById.html b/src/test/html/content/test_document_getElementById.html
new file mode 100644
index 00000000000..20140274477
--- /dev/null
+++ b/src/test/html/content/test_document_getElementById.html
@@ -0,0 +1,58 @@
+<html>
+<head id="foo">
+  <title></title>
+  <script src="harness.js"></script>
+</head>
+<body>
+  <div id="bar"></div>
+  <script>
+    let gBody = document.getElementsByTagName("body")[0];
+
+    // test1: on static page
+    {
+      let foo = document.getElementById("foo");
+      isnot(foo, null, "test-1-0, on static page");
+      is(foo && foo.tagName, "HEAD", "test1-1, on static page");
+      is(foo instanceof HTMLHeadElement, true, "test1-2, on static page");
+
+      let bar = document.getElementById("bar");
+      isnot(bar, null, "test1-3, on static page");
+      is(bar && bar.tagName, "DIV", "test1-4, on static page");
+      is(bar instanceof HTMLDivElement, true, "test1-5, on static page");
+    }
+
+    // test2: scripted element
+    {
+      let TEST_ID = "test";
+      let test = document.createElement("div");
+      test.setAttribute("id", TEST_ID);
+      gBody.appendChild(test);
+
+      // test: appended element
+      let appended = document.getElementById(TEST_ID);
+      isnot(appended, null, "test2-0, appended element");
+      is(appended && appended.tagName, "DIV", "test2-1, appended element");
+      is(appended instanceof HTMLDivElement, true, "test2-2, appended element");
+
+      // test: removed element
+      gBody.removeChild(test);
+      let removed = document.getElementById(TEST_ID);
+      // `document.getElementById()` returns `null` if there is none.
+      // http://www.w3.org/TR/DOM-Level-3-Core/core.html#ID-getElBId
+      // http://dom.spec.whatwg.org/#dom-document-getelementbyid (2013-09-20)
+      is(removed, null, "test2-3, removed element");
+    }
+
+    // TODO:
+    // test3: update `id` attribute
+
+    // TODO:
+    // test4: "in tree order, within the context object's tree"
+    // http://dom.spec.whatwg.org/#dom-document-getelementbyid.
+
+    // TODO:
+    // test5: innerHTML
+    finish();
+  </script>
+</body>
+</html>