From 0b802ab01882dc43ad5ec0f0e8e8699f22866e71 Mon Sep 17 00:00:00 2001 From: James Graham Date: Sun, 27 Jul 2014 22:44:13 +0100 Subject: [PATCH] Fix getElementsByTagName[NS] support to match the spec. --- src/components/script/dom/document.rs | 7 +-- src/components/script/dom/element.rs | 9 +-- src/components/script/dom/htmlcollection.rs | 64 ++++++++++++++++++--- src/components/script/dom/node.rs | 5 ++ src/test/content/test_htmlcollection.html | 2 +- 5 files changed, 65 insertions(+), 22 deletions(-) diff --git a/src/components/script/dom/document.rs b/src/components/script/dom/document.rs index 461ed9c69d5..bd2c6baf5ec 100644 --- a/src/components/script/dom/document.rs +++ b/src/components/script/dom/document.rs @@ -351,12 +351,7 @@ impl<'a> DocumentMethods for JSRef<'a, Document> { // http://dom.spec.whatwg.org/#dom-document-getelementsbytagnamens fn GetElementsByTagNameNS(&self, maybe_ns: Option, tag_name: DOMString) -> Temporary { let window = self.window.root(); - - let namespace = match maybe_ns { - Some(namespace) => Namespace::from_str(namespace.as_slice()), - None => Null - }; - HTMLCollection::by_tag_name_ns(&*window, NodeCast::from_ref(self), tag_name, namespace) + HTMLCollection::by_tag_name_ns(&*window, NodeCast::from_ref(self), tag_name, maybe_ns) } // http://dom.spec.whatwg.org/#dom-document-getelementsbyclassname diff --git a/src/components/script/dom/element.rs b/src/components/script/dom/element.rs index 5bf72216d96..a6b894c3b5b 100644 --- a/src/components/script/dom/element.rs +++ b/src/components/script/dom/element.rs @@ -224,9 +224,8 @@ pub trait ElementHelpers { impl<'a> ElementHelpers for JSRef<'a, Element> { fn html_element_in_html_document(&self) -> bool { - let is_html = self.namespace == namespace::HTML; let node: &JSRef = NodeCast::from_ref(self); - is_html && node.owner_doc().root().is_html_document + self.namespace == namespace::HTML && node.is_in_html_doc() } fn get_local_name<'a>(&'a self) -> &'a Atom { @@ -702,12 +701,8 @@ impl<'a> ElementMethods for JSRef<'a, Element> { fn GetElementsByTagNameNS(&self, maybe_ns: Option, localname: DOMString) -> Temporary { - let namespace = match maybe_ns { - Some(namespace) => Namespace::from_str(namespace.as_slice()), - None => Null - }; let window = window_from_node(self).root(); - HTMLCollection::by_tag_name_ns(&*window, NodeCast::from_ref(self), localname, namespace) + HTMLCollection::by_tag_name_ns(&*window, NodeCast::from_ref(self), localname, maybe_ns) } fn GetElementsByClassName(&self, classes: DOMString) -> Temporary { diff --git a/src/components/script/dom/htmlcollection.rs b/src/components/script/dom/htmlcollection.rs index 42712cf50b4..5bef6c56ff5 100644 --- a/src/components/script/dom/htmlcollection.rs +++ b/src/components/script/dom/htmlcollection.rs @@ -8,14 +8,16 @@ use dom::bindings::codegen::InheritTypes::{ElementCast, NodeCast}; use dom::bindings::global::Window; use dom::bindings::js::{JS, JSRef, Temporary}; use dom::bindings::utils::{Reflectable, Reflector, reflect_dom_object}; -use dom::element::{Element, AttributeHandlers}; +use dom::element::{Element, AttributeHandlers, ElementHelpers}; use dom::node::{Node, NodeHelpers}; use dom::window::Window; use servo_util::atom::Atom; +use servo_util::namespace; use servo_util::namespace::Namespace; use servo_util::str::{DOMString, split_html_space_chars}; use serialize::{Encoder, Encodable}; +use std::ascii::StrAsciiExt; pub trait CollectionFilter { fn filter(&self, elem: &JSRef, root: &JSRef) -> bool; @@ -59,36 +61,82 @@ impl HTMLCollection { HTMLCollection::new(window, Live(JS::from_rooted(root), filter)) } + fn all_elements(window: &JSRef, root: &JSRef, + namespace_filter: Option) -> Temporary { + struct AllElementFilter { + namespace_filter: Option + } + impl CollectionFilter for AllElementFilter { + fn filter(&self, elem: &JSRef, _root: &JSRef) -> bool { + match self.namespace_filter { + None => true, + Some(ref namespace) => elem.namespace == *namespace + } + } + } + let filter = AllElementFilter {namespace_filter: namespace_filter}; + HTMLCollection::create(window, root, box filter) + } + pub fn by_tag_name(window: &JSRef, root: &JSRef, tag: DOMString) -> Temporary { + if tag.as_slice() == "*" { + return HTMLCollection::all_elements(window, root, None); + } + struct TagNameFilter { - tag: Atom + tag: Atom, + ascii_lower_tag: Atom, } impl CollectionFilter for TagNameFilter { fn filter(&self, elem: &JSRef, _root: &JSRef) -> bool { - elem.deref().local_name == self.tag + if elem.html_element_in_html_document() { + elem.local_name == self.ascii_lower_tag + } else { + elem.local_name == self.tag + } } } let filter = TagNameFilter { - tag: Atom::from_slice(tag.as_slice()) + tag: Atom::from_slice(tag.as_slice()), + ascii_lower_tag: Atom::from_slice(tag.as_slice().to_ascii_lower().as_slice()), }; HTMLCollection::create(window, root, box filter) } pub fn by_tag_name_ns(window: &JSRef, root: &JSRef, tag: DOMString, - namespace: Namespace) -> Temporary { + maybe_ns: Option) -> Temporary { + let namespace_filter = match maybe_ns { + Some(namespace) => { + match namespace.as_slice() { + "*" => None, + ns => Some(Namespace::from_str(ns)), + } + }, + None => Some(namespace::Null), + }; + + if tag.as_slice() == "*" { + return HTMLCollection::all_elements(window, root, namespace_filter); + } struct TagNameNSFilter { tag: Atom, - namespace: Namespace + namespace_filter: Option } impl CollectionFilter for TagNameNSFilter { fn filter(&self, elem: &JSRef, _root: &JSRef) -> bool { - elem.deref().namespace == self.namespace && elem.deref().local_name == self.tag + let ns_match = match self.namespace_filter { + Some(ref namespace) => { + elem.deref().namespace == *namespace + }, + None => true + }; + ns_match && elem.deref().local_name == self.tag } } let filter = TagNameNSFilter { tag: Atom::from_slice(tag.as_slice()), - namespace: namespace + namespace_filter: namespace_filter }; HTMLCollection::create(window, root, box filter) } diff --git a/src/components/script/dom/node.rs b/src/components/script/dom/node.rs index 41a1815013a..9b3bf18a99a 100644 --- a/src/components/script/dom/node.rs +++ b/src/components/script/dom/node.rs @@ -389,6 +389,7 @@ pub trait NodeHelpers { fn owner_doc(&self) -> Temporary; fn set_owner_doc(&self, document: &JSRef); + fn is_in_html_doc(&self) -> bool; fn wait_until_safe_to_modify_dom(&self); @@ -672,6 +673,10 @@ impl<'a> NodeHelpers for JSRef<'a, Node> { self.owner_doc.assign(Some(document.clone())); } + fn is_in_html_doc(&self) -> bool { + self.owner_doc().root().is_html_document + } + fn children(&self) -> AbstractNodeChildrenIterator { AbstractNodeChildrenIterator { current_node: self.first_child.get().map(|node| (*node.root()).clone()), diff --git a/src/test/content/test_htmlcollection.html b/src/test/content/test_htmlcollection.html index c620b54815f..d7a0b44dce5 100644 --- a/src/test/content/test_htmlcollection.html +++ b/src/test/content/test_htmlcollection.html @@ -52,7 +52,7 @@ // test3: getElementsByTagName { - is(document.getElementsByTagName("DIV").length, 0); + is(document.getElementsByTagName("DIV").length, 5); is(document.getElementsByTagName("div").length, document.documentElement.getElementsByTagName("div").length);