Make sure getElementById always returns the first element with the given ID in tree order.(fixes #1822)

3 files changed, 77 insertions, 8 deletions

diff --git a/src/components/script/dom/document.rs b/src/components/script/dom/document.rs
index 16a96c0bb95..e81394e6f18 100644
--- a/src/components/script/dom/document.rs
+++ b/src/components/script/dom/document.rs
@@ -7,6 +7,7 @@ use dom::bindings::codegen::InheritTypes::{DocumentBase, NodeCast, DocumentCast}
 use dom::bindings::codegen::InheritTypes::{HTMLHeadElementCast, TextCast, ElementCast};
 use dom::bindings::codegen::InheritTypes::{DocumentTypeCast, HTMLHtmlElementCast};
 use dom::bindings::codegen::DocumentBinding;
+use dom::bindings::codegen::NodeBinding::NodeConstants::{DOCUMENT_POSITION_CONTAINS, DOCUMENT_POSITION_PRECEDING};
 use dom::bindings::js::JS;
 use dom::bindings::utils::{Reflectable, Reflector, reflect_dom_object};
 use dom::bindings::error::{ErrorResult, Fallible, NotSupported, InvalidCharacter, HierarchyRequest};
@@ -58,7 +59,7 @@ pub struct Document {
     node: Node,
     reflector_: Reflector,
     window: JS<Window>,
-    idmap: HashMap<DOMString, JS<Element>>,
+    idmap: HashMap<DOMString, ~[JS<Element>]>,
     implementation: Option<JS<DOMImplementation>>,
     content_type: DOMString,
     encoding_name: DOMString,
@@ -248,7 +249,7 @@ impl Document {
         // http://dom.spec.whatwg.org/#dom-document-getelementbyid.
         match self.idmap.find_equiv(&id) {
             None => None,
-            Some(node) => Some(node.clone()),
+            Some(ref elements) => Some(elements[0].clone()),
         }
     }
 
@@ -600,8 +601,22 @@ impl Document {
 
     /// Remove any existing association between the provided id and any elements in this document.
     pub fn unregister_named_element(&mut self,
+                                    abstract_self: &JS<Element>,
                                     id: DOMString) {
-        self.idmap.remove(&id);
+        let mut is_empty = false;
+        match self.idmap.find_mut(&id) {
+            None => {},
+            Some(elements) => {
+                let position = elements.iter()
+                                       .position(|element| element == abstract_self)
+                                       .expect("This element should be in registered.");
+                elements.remove(position);
+                is_empty = elements.is_empty();
+            }
+        }
+        if is_empty {
+            self.idmap.remove(&id);
+        }
     }
 
     /// Associate an element present in this document with the provided id.
@@ -618,12 +633,26 @@ impl Document {
         // FIXME https://github.com/mozilla/rust/issues/13195
         //       Use mangle() when it exists again.
         match self.idmap.find_mut(&id) {
-            Some(v) => {
-                *v = element.clone();
+            Some(elements) => {
+                let new_node = NodeCast::from(element);
+                let mut head : uint = 0u;
+                let mut tail : uint = elements.len();
+                while head < tail {
+                    let middle = ((head + tail) / 2) as int;
+                    let elem = &elements[middle];
+                    let js_node = NodeCast::from(elem);
+                    let position = elem.get().node.CompareDocumentPosition(&js_node, &new_node);
+                    if position == DOCUMENT_POSITION_PRECEDING || position == DOCUMENT_POSITION_PRECEDING + DOCUMENT_POSITION_CONTAINS {
+                        tail = middle as uint;
+                    } else {
+                        head = middle as uint + 1u;
+                    }
+                }
+                elements.insert(tail, element.clone());
                 return;
             },
             None => (),
         }
-        self.idmap.insert(id, element.clone());
+        self.idmap.insert(id, ~[element.clone()]);
     }
 }
diff --git a/src/components/script/dom/element.rs b/src/components/script/dom/element.rs
index 11b83684090..3d578a4a793 100644
--- a/src/components/script/dom/element.rs
+++ b/src/components/script/dom/element.rs
@@ -352,7 +352,7 @@ impl AttributeHandlers for JS<Element> {
                 // "borrowed value does not live long enough"
                 let mut doc = node.get().owner_doc().clone();
                 let doc = doc.get_mut();
-                doc.unregister_named_element(old_value);
+                doc.unregister_named_element(self, old_value);
             }
             _ => ()
         }
@@ -640,7 +640,7 @@ impl IElement for JS<Element> {
         match self.get_attribute(Null, "id") {
             Some(attr) => {
                 let mut doc = document_from_node(self);
-                doc.get_mut().unregister_named_element(attr.get().Value());
+                doc.get_mut().unregister_named_element(self, attr.get().Value());
             }
             _ => ()
         }
diff --git a/src/test/content/test_document_getElementById_tree_order.html b/src/test/content/test_document_getElementById_tree_order.html
new file mode 100644
index 00000000000..ccf0c37bb01
--- /dev/null
+++ b/src/test/content/test_document_getElementById_tree_order.html
@@ -0,0 +1,40 @@
+<html>
+  <head>
+    <script src="harness.js"></script>
+  </head>
+  <body>
+    <div id="a">
+    </div>
+    <div id="b">
+      <p id="b">P</p>
+      <input id="b" type="submit" value="Submit">
+    </div>
+    <script>
+      {
+        var b = document.getElementById("b");
+        is_a(b, HTMLDivElement);
+        var a = document.getElementById("a");
+        var p = document.createElement("p");
+        p.id = "b";
+        a.appendChild(p);
+        var newB = document.getElementById("b");
+        is_a(newB, HTMLParagraphElement);
+      }
+      {
+        var gbody = document.getElementsByTagName("body")[0];
+        var div = document.createElement("div");
+        div.setAttribute("id", "c");
+        var h1 = document.createElement("h1");
+        h1.setAttribute("id", "c");
+        gbody.appendChild(div);
+        gbody.appendChild(h1);
+        var c = document.getElementById("c");
+        is_a(c, HTMLDivElement);
+        gbody.removeChild(div);
+        var newC = document.getElementById("c");
+        is_a(newC, HTMLHeadingElement);
+      }
+      finish();
+    </script>
+  </body>
+</html>