diff --git a/src/components/script/dom/attr.rs b/src/components/script/dom/attr.rs index 3cf978d69c4..906e1b7f02c 100644 --- a/src/components/script/dom/attr.rs +++ b/src/components/script/dom/attr.rs @@ -9,8 +9,6 @@ use dom::window::Window; use servo_util::namespace::{Namespace, Null}; use servo_util::str::DOMString; -use std::util; - #[deriving(Encodable)] pub struct Attr { reflector_: Reflector, @@ -63,9 +61,8 @@ impl Attr { reflect_dom_object(~attr, window, AttrBinding::Wrap) } - pub fn set_value(&mut self, mut value: DOMString) -> DOMString { - util::swap(&mut self.value, &mut value); - value + pub fn set_value(&mut self, value: DOMString) { + self.value = value; } pub fn value_ref<'a>(&'a self) -> &'a str { diff --git a/src/components/script/dom/document.rs b/src/components/script/dom/document.rs index 9854bb7e386..3f917023c88 100644 --- a/src/components/script/dom/document.rs +++ b/src/components/script/dom/document.rs @@ -478,68 +478,30 @@ impl Document { self.window.get().wait_until_safe_to_modify_dom(); } - pub fn register_nodes_with_id(&mut self, root: &JS) { - foreach_ided_elements(root, |id: &DOMString, abstract_node: &JS| { - // 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.clone()); - }); + + /// Remove any existing association between the provided id and any elements in this document. + pub fn unregister_named_element(&mut self, + id: DOMString) { + self.idmap.remove(&id); } - pub fn unregister_nodes_with_id(&mut self, root: &JS) { - 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); + /// Associate an element present in this document with the provided id. + pub fn register_named_element(&mut self, + element: &JS, + id: DOMString) { + assert!({ + let node: JS = NodeCast::from(element); + node.is_in_doc() }); - } - pub fn update_idmap(&mut self, - abstract_self: &JS, - new_id: Option, - old_id: Option) { - // remove old ids: - // * if the old ones are not same as the new one, - // * OR if the new one is none. - match old_id { - Some(ref old_id) if new_id.is_none() || - (*new_id.get_ref() != *old_id) => { - self.idmap.remove(old_id); - } - _ => () - } - - match new_id { - Some(new_id) => { - // TODO: support the case if multiple elements - // which haves same id are in the same document. - self.idmap.mangle(new_id, abstract_self.clone(), - |_, new_node: JS| -> JS { - new_node - }, - |_, old_node: &mut JS, new_node: JS| { - *old_node = new_node; - }); - } - None => () - } - } -} - -#[inline(always)] -fn foreach_ided_elements(root: &JS, callback: |&DOMString, &JS|) { - let root: JS = NodeCast::from(root); - for node in root.traverse_preorder() { - if !node.is_element() { - continue; - } - - let element: JS = ElementCast::to(&node); - match element.get().get_attribute(Null, "id") { - Some(id) => { - callback(&id.get().Value(), &element); - } - None => () - } + // TODO: support the case if multiple elements + // which haves same id are in the same document. + self.idmap.mangle(id, element, + |_, new_element: &JS| -> JS { + new_element.clone() + }, + |_, old_element: &mut JS, new_element: &JS| { + *old_element = new_element.clone(); + }); } } diff --git a/src/components/script/dom/element.rs b/src/components/script/dom/element.rs index d7bbaa065a6..b3d5d06f38a 100644 --- a/src/components/script/dom/element.rs +++ b/src/components/script/dom/element.rs @@ -204,26 +204,31 @@ impl Element { self.node.wait_until_safe_to_modify_dom(); // FIXME: reduce the time of `value.clone()`. - let mut old_raw_value: Option = None; - for attr in self.attrs.mut_iter() { - let attr = attr.get_mut(); - if attr.local_name == local_name { - old_raw_value = Some(attr.set_value(value.clone())); - break; + let idx = self.attrs.iter().position(|attr| { + attr.get().local_name == local_name + }); + + match idx { + Some(idx) => { + if namespace == namespace::Null { + let old_value = self.attrs[idx].get().Value(); + self.before_remove_attr(abstract_self, local_name.clone(), + old_value); + } + self.attrs[idx].get_mut().set_value(value.clone()); + } + None => { + let doc = self.node.owner_doc(); + let doc = doc.get(); + let new_attr = Attr::new_ns(doc.window.get(), local_name.clone(), value.clone(), + name.clone(), namespace.clone(), + prefix); + self.attrs.push(new_attr); } } - if old_raw_value.is_none() { - let doc = self.node.owner_doc(); - let doc = doc.get(); - let new_attr = Attr::new_ns(doc.window.get(), local_name.clone(), value.clone(), - name.clone(), namespace.clone(), - prefix); - self.attrs.push(new_attr); - } - if namespace == namespace::Null { - self.after_set_attr(abstract_self, local_name, value, old_raw_value); + self.after_set_attr(abstract_self, local_name, value); } Ok(()) } @@ -231,8 +236,7 @@ impl Element { fn after_set_attr(&mut self, abstract_self: &JS, local_name: DOMString, - value: DOMString, - old_value: Option) { + value: DOMString) { match local_name.as_slice() { "style" => { @@ -247,7 +251,7 @@ impl Element { // "borrowed value does not live long enough" let mut doc = self.node.owner_doc(); let doc = doc.get_mut(); - doc.update_idmap(abstract_self, Some(value.clone()), old_value); + doc.register_named_element(abstract_self, value.clone()); } } _ => () @@ -289,22 +293,22 @@ impl Element { match idx { None => (), Some(idx) => { - let removed = self.attrs.remove(idx); - let removed_raw_value = Some(removed.get().Value()); - if namespace == namespace::Null { - self.after_remove_attr(abstract_self, local_name, removed_raw_value); + let removed_raw_value = self.attrs[idx].get().Value(); + self.before_remove_attr(abstract_self, local_name, removed_raw_value); } + + self.attrs.remove(idx); } }; Ok(()) } - fn after_remove_attr(&mut self, - abstract_self: &JS, - local_name: DOMString, - old_value: Option) { + fn before_remove_attr(&mut self, + abstract_self: &JS, + local_name: DOMString, + old_value: DOMString) { match local_name.as_slice() { "style" => { self.style_attribute = None @@ -316,7 +320,7 @@ impl Element { // "borrowed value does not live long enough" let mut doc = self.node.owner_doc(); let doc = doc.get_mut(); - doc.update_idmap(abstract_self, None, old_value); + doc.unregister_named_element(old_value); } } _ => () @@ -327,11 +331,11 @@ impl Element { match abstract_self.get().node.type_id { ElementNodeTypeId(HTMLImageElementTypeId) => { let mut elem: JS = HTMLImageElementCast::to(abstract_self); - elem.get_mut().AfterRemoveAttr(local_name.clone()); + elem.get_mut().BeforeRemoveAttr(local_name.clone()); } ElementNodeTypeId(HTMLIframeElementTypeId) => { let mut elem: JS = HTMLIFrameElementCast::to(abstract_self); - elem.get_mut().AfterRemoveAttr(local_name.clone()); + elem.get_mut().BeforeRemoveAttr(local_name.clone()); } _ => () } @@ -646,6 +650,33 @@ impl Element { } } +pub trait IElement { + fn bind_to_tree_impl(&self); + fn unbind_from_tree_impl(&self); +} + +impl IElement for JS { + fn bind_to_tree_impl(&self) { + match self.get().get_attribute(Null, "id") { + Some(attr) => { + let mut doc = self.get().node.owner_doc(); + doc.get_mut().register_named_element(self, attr.get().Value()); + } + _ => () + } + } + + fn unbind_from_tree_impl(&self) { + match self.get().get_attribute(Null, "id") { + Some(attr) => { + let mut doc = self.get().node.owner_doc(); + doc.get_mut().unregister_named_element(attr.get().Value()); + } + _ => () + } + } +} + fn get_attribute_parts(name: DOMString) -> (Option<~str>, ~str) { //FIXME: Throw for XML-invalid names //FIXME: Throw for XMLNS-invalid names diff --git a/src/components/script/dom/htmliframeelement.rs b/src/components/script/dom/htmliframeelement.rs index 92e0c63dfc7..4f25371772b 100644 --- a/src/components/script/dom/htmliframeelement.rs +++ b/src/components/script/dom/htmliframeelement.rs @@ -140,7 +140,7 @@ impl HTMLIFrameElement { } } - pub fn AfterRemoveAttr(&mut self, name: DOMString) { + pub fn BeforeRemoveAttr(&mut self, name: DOMString) { if "sandbox" == name { self.sandbox = None; } diff --git a/src/components/script/dom/htmlimageelement.rs b/src/components/script/dom/htmlimageelement.rs index 4f18597449c..1b336282b3d 100644 --- a/src/components/script/dom/htmlimageelement.rs +++ b/src/components/script/dom/htmlimageelement.rs @@ -96,7 +96,7 @@ impl HTMLImageElement { } } - pub fn AfterRemoveAttr(&mut self, name: DOMString) { + pub fn BeforeRemoveAttr(&mut self, name: DOMString) { if "src" == name { self.update_image(None, None); } diff --git a/src/components/script/dom/node.rs b/src/components/script/dom/node.rs index a0357c92ab4..419b02c844a 100644 --- a/src/components/script/dom/node.rs +++ b/src/components/script/dom/node.rs @@ -14,7 +14,7 @@ use dom::bindings::utils; use dom::characterdata::CharacterData; use dom::document::Document; use dom::documenttype::DocumentType; -use dom::element::{Element, ElementTypeId, HTMLAnchorElementTypeId}; +use dom::element::{Element, ElementTypeId, HTMLAnchorElementTypeId, IElement}; use dom::eventtarget::{EventTarget, NodeTargetTypeId}; use dom::nodelist::{NodeList}; use dom::text::Text; @@ -396,11 +396,13 @@ impl NodeHelpers for JS { // http://dom.spec.whatwg.org/#node-is-inserted fn node_inserted(&self) { assert!(self.parent_node().is_some()); - let mut document = self.get().owner_doc(); + let document = self.get().owner_doc(); - // Register elements having "id" attribute to the owner doc. - if self.is_element() { - document.get_mut().register_nodes_with_id(&ElementCast::to(self)); + for node in self.traverse_preorder() { + if node.is_element() { + let element: JS = ElementCast::to(&node); + element.bind_to_tree_impl(); + } } document.get().content_changed(); @@ -409,11 +411,13 @@ impl NodeHelpers for JS { // http://dom.spec.whatwg.org/#node-is-removed fn node_removed(&self) { assert!(self.parent_node().is_none()); - let mut document = self.get().owner_doc(); + let document = self.get().owner_doc(); - // Unregister elements having "id". - if self.is_element() { - document.get_mut().unregister_nodes_with_id(&ElementCast::to(self)); + for node in self.traverse_preorder() { + if node.is_element() { + let element: JS = ElementCast::to(&node); + element.unbind_from_tree_impl(); + } } document.get().content_changed();