diff --git a/src/components/script/dom/attr.rs b/src/components/script/dom/attr.rs index 40da74cfbd2..237806c07f8 100644 --- a/src/components/script/dom/attr.rs +++ b/src/components/script/dom/attr.rs @@ -3,12 +3,22 @@ * 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; +pub enum AttrSettingType { + FirstSetAttr, + ReplacedAttr, +} + #[deriving(Encodable)] pub struct Attr { reflector_: Reflector, @@ -16,7 +26,10 @@ pub struct Attr { value: DOMString, name: DOMString, namespace: Namespace, - prefix: Option + prefix: Option, + + /// the element that owns this attribute. + owner: JS, } impl Reflectable for Attr { @@ -32,26 +45,43 @@ impl Reflectable for Attr { impl Attr { fn new_inherited(local_name: DOMString, value: DOMString, name: DOMString, namespace: Namespace, - prefix: Option) -> Attr { + prefix: Option, owner: JS) -> 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, local_name: DOMString, value: DOMString, name: DOMString, namespace: Namespace, - prefix: Option) -> JS { - let attr = Attr::new_inherited(local_name, value, name, namespace, prefix); + prefix: Option, owner: JS) -> JS { + 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) { + pub fn set_value(&mut self, set_type: AttrSettingType, value: DOMString) { + let node: JS = NodeCast::from(&self.owner); + let namespace_is_null = self.namespace == namespace::Null; + + match set_type { + ReplacedAttr => { + if namespace_is_null { + vtable_for(&node).before_remove_attr(self.local_name.clone(), self.value.clone()); + } + } + FirstSetAttr => {} + } + 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 { @@ -69,7 +99,7 @@ impl Attr { } pub fn SetValue(&mut self, value: DOMString) { - self.value = value; + self.set_value(ReplacedAttr, value); } pub fn Name(&self) -> DOMString { diff --git a/src/components/script/dom/element.rs b/src/components/script/dom/element.rs index ace3f68bccc..d60181f5349 100644 --- a/src/components/script/dom/element.rs +++ b/src/components/script/dom/element.rs @@ -4,7 +4,7 @@ //! Element nodes. -use dom::attr::Attr; +use dom::attr::{Attr, AttrSettingType, ReplacedAttr, FirstSetAttr}; use dom::attrlist::AttrList; use dom::bindings::codegen::ElementBinding; use dom::bindings::codegen::InheritTypes::{ElementDerived, NodeCast}; @@ -263,26 +263,22 @@ impl AttributeHandlers for JS { prefix: Option, cb: |&JS| -> bool) { let node: JS = NodeCast::from(self); let idx = self.get().attrs.iter().position(cb); - match idx { + let (mut attr, set_type): (JS, AttrSettingType) = 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()); + let attr = self.get_mut().attrs[idx].clone(); + (attr, ReplacedAttr) } None => { let doc = node.get().owner_doc().get(); - let new_attr = Attr::new(&doc.window, local_name.clone(), value.clone(), - name, namespace.clone(), prefix); - self.get_mut().attrs.push(new_attr); + let attr = Attr::new(&doc.window, local_name.clone(), value.clone(), + name, namespace.clone(), prefix, self.clone()); + self.get_mut().attrs.push(attr.clone()); + (attr, FirstSetAttr) } - } + }; - if namespace == namespace::Null { - vtable_for(&node).after_set_attr(local_name, value); - } + attr.get_mut().set_value(set_type, value); } // http://dom.spec.whatwg.org/#dom-element-setattribute diff --git a/src/components/script/dom/node.rs b/src/components/script/dom/node.rs index 86137fef714..04910555552 100644 --- a/src/components/script/dom/node.rs +++ b/src/components/script/dom/node.rs @@ -1365,6 +1365,10 @@ impl Node { let node_elem: JS = ElementCast::to(node).unwrap(); let node_elem = node_elem.get(); let mut copy_elem: JS = ElementCast::to(©).unwrap(); + + // XXX: to avoid double borrowing compile error. we might be able to fix this after #1854 + let copy_elem_alias: JS = 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())); } }, _ => () diff --git a/src/test/content/test_document_getElementById.html b/src/test/content/test_document_getElementById.html index 97b506f58af..382c07574a7 100644 --- a/src/test/content/test_document_getElementById.html +++ b/src/test/content/test_document_getElementById.html @@ -92,6 +92,20 @@ is(document.getElementById("x"), null, "test 7-0"); } + // test 8 + { + let TEST_ID = "test-8" + let element = document.createElement("div"); + element.setAttribute("id", TEST_ID); + document.body.appendChild(element); + let target = document.getElementById(TEST_ID); + is_not(target, null, "test 8-0, getElementById is correct before changing the value"); + + element.attributes[0].value = TEST_ID + "-updated"; + let target2 = document.getElementById(TEST_ID); + is(target2, null, "test 8-1, should return null after updated id via Attr.value"); + } + finish();