From 7a9ecffaa0dcea56bf2431508b7e5a4d53214ad9 Mon Sep 17 00:00:00 2001 From: Tetsuharu OHZEKI Date: Fri, 3 Jan 2014 03:40:05 +0900 Subject: [PATCH] Implement Element.removeAttribute()/removeAttributeNS(). --- .../script/dom/bindings/codegen/Bindings.conf | 2 +- src/components/script/dom/element.rs | 84 ++++++++++++++++++- .../script/dom/htmliframeelement.rs | 6 ++ src/components/script/dom/htmlimageelement.rs | 11 +++ .../html/content/test_element_attribute.html | 10 +++ 5 files changed, 108 insertions(+), 5 deletions(-) diff --git a/src/components/script/dom/bindings/codegen/Bindings.conf b/src/components/script/dom/bindings/codegen/Bindings.conf index da82eae2a8c..9db9477850d 100644 --- a/src/components/script/dom/bindings/codegen/Bindings.conf +++ b/src/components/script/dom/bindings/codegen/Bindings.conf @@ -185,7 +185,7 @@ DOMInterfaces = { 'Element': { 'nativeType': 'AbstractNode', 'pointerType': '', - 'needsAbstract': ['getClientRects', 'getBoundingClientRect', 'setAttribute', 'setAttributeNS', 'id', 'attributes'] + 'needsAbstract': ['getClientRects', 'getBoundingClientRect', 'setAttribute', 'setAttributeNS', 'removeAttribute', 'removeAttributeNS', 'id', 'attributes'] }, 'Event': { diff --git a/src/components/script/dom/element.rs b/src/components/script/dom/element.rs index b1b3615f52e..25a9e3811b2 100644 --- a/src/components/script/dom/element.rs +++ b/src/components/script/dom/element.rs @@ -244,6 +244,76 @@ impl Element { _ => () } + self.notify_attribute_changed(abstract_self, local_name); + } + + pub fn remove_attribute(&mut self, + abstract_self: AbstractNode, + namespace: Namespace, + name: DOMString) -> ErrorResult { + let (_, local_name) = get_attribute_parts(name.clone()); + + self.node.wait_until_safe_to_modify_dom(); + + let idx = self.attrs.iter().position(|attr: &@mut Attr| -> bool { + attr.local_name == local_name + }); + + match idx { + None => (), + Some(idx) => { + let removed = self.attrs.remove(idx); + let removed_raw_value = Some(removed.Value()); + + if namespace == namespace::Null { + self.after_remove_attr(abstract_self, local_name, removed_raw_value); + } + } + }; + + Ok(()) + } + + fn after_remove_attr(&mut self, + abstract_self: AbstractNode, + local_name: DOMString, + old_value: Option) { + match local_name.as_slice() { + "style" => { + self.style_attribute = None + } + "id" => { + // XXX: this dual declaration are workaround to avoid the compile error: + // "borrowed value does not live long enough" + let doc = self.node.owner_doc(); + let doc = doc.mut_document(); + doc.update_idmap(abstract_self, None, old_value); + } + _ => () + } + + //XXXjdm We really need something like a vtable so we can call AfterSetAttr. + // This hardcoding is awful. + match abstract_self.type_id() { + ElementNodeTypeId(HTMLImageElementTypeId) => { + abstract_self.with_mut_image_element(|image| { + image.AfterRemoveAttr(local_name.clone()); + }); + } + ElementNodeTypeId(HTMLIframeElementTypeId) => { + abstract_self.with_mut_iframe_element(|iframe| { + iframe.AfterRemoveAttr(local_name.clone()); + }); + } + _ => () + } + + self.notify_attribute_changed(abstract_self, local_name); + } + + fn notify_attribute_changed(&self, + abstract_self: AbstractNode, + local_name: DOMString) { if abstract_self.is_in_doc() { let damage = match local_name.as_slice() { "style" | "id" | "class" => MatchSelectorsDocumentDamage, @@ -335,12 +405,18 @@ impl Element { self.set_attribute(abstract_self, namespace, name, value) } - pub fn RemoveAttribute(&self, _name: DOMString) -> ErrorResult { - Ok(()) + pub fn RemoveAttribute(&mut self, + abstract_self: AbstractNode, + name: DOMString) -> ErrorResult { + self.remove_attribute(abstract_self, namespace::Null, name) } - pub fn RemoveAttributeNS(&self, _namespace: Option, _localname: DOMString) -> ErrorResult { - Ok(()) + pub fn RemoveAttributeNS(&mut self, + abstract_self: AbstractNode, + namespace: Option, + localname: DOMString) -> ErrorResult { + let namespace = Namespace::from_str(namespace); + self.remove_attribute(abstract_self, namespace, localname) } pub fn HasAttribute(&self, name: DOMString) -> bool { diff --git a/src/components/script/dom/htmliframeelement.rs b/src/components/script/dom/htmliframeelement.rs index 5157ab03be7..7743901015f 100644 --- a/src/components/script/dom/htmliframeelement.rs +++ b/src/components/script/dom/htmliframeelement.rs @@ -112,6 +112,12 @@ impl HTMLIFrameElement { } } + pub fn AfterRemoveAttr(&mut self, name: DOMString) { + if "sandbox" == name { + self.sandbox = None; + } + } + pub fn AllowFullscreen(&self) -> bool { false } diff --git a/src/components/script/dom/htmlimageelement.rs b/src/components/script/dom/htmlimageelement.rs index 53686661c10..1966289e7e2 100644 --- a/src/components/script/dom/htmlimageelement.rs +++ b/src/components/script/dom/htmlimageelement.rs @@ -67,6 +67,17 @@ impl HTMLImageElement { } } + pub fn AfterRemoveAttr(&mut self, name: DOMString) { + // FIXME (#1469): + // This might not handle remove src attribute actually since + // `self.update_image()` will see the missing src attribute and return early. + if "src" == name { + let document = self.htmlelement.element.node.owner_doc(); + let window = document.document().window; + self.update_image(window.image_cache_task.clone(), None); + } + } + pub fn Alt(&self) -> DOMString { ~"" } diff --git a/src/test/html/content/test_element_attribute.html b/src/test/html/content/test_element_attribute.html index fccc92734d7..8631138c767 100644 --- a/src/test/html/content/test_element_attribute.html +++ b/src/test/html/content/test_element_attribute.html @@ -31,6 +31,16 @@ is(r, VALUE, "test3, attribute update by Element.setAttribute().") } + { + test.setAttribute("id", "bar"); + test.removeAttribute("id"); + + let r1 = test.hasAttribute("id"); + is(r1, false, "test4-0, Element.removeAttribute()."); + let r2 = test.getAttribute("id"); + is(r2, null, "test4-1, Element.removeAttribute()."); + } + finish();