Auto merge of #14047 - mskrzypkows:getElementsByTagName, r=nox

fix getElementsByTagName()

<!-- Please describe your changes on the following line: -->
Improved implementation of getElementsByTagName() in Document, properly pass 3 cases of https://dom.spec.whatwg.org/#concept-getelementsbytagname

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix  #11596 (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14047)
<!-- Reviewable:end -->

components/script/dom/document.rs

@@ -2176,18 +2176,13 @@ impl DocumentMethods for Document {
     }
 
     // https://dom.spec.whatwg.org/#dom-document-getelementsbytagname
-    fn GetElementsByTagName(&self, tag_name: DOMString) -> Root<HTMLCollection> {
+    fn GetElementsByTagName(&self, qualified_name: DOMString) -> Root<HTMLCollection> {
-        let tag_atom = LocalName::from(&*tag_name);
+        let qualified_name = LocalName::from(&*qualified_name);
-        match self.tag_map.borrow_mut().entry(tag_atom.clone()) {
+        match self.tag_map.borrow_mut().entry(qualified_name.clone()) {
             Occupied(entry) => Root::from_ref(entry.get()),
             Vacant(entry) => {
-                let mut tag_copy = tag_name;
+                let result = HTMLCollection::by_qualified_name(
-                tag_copy.make_ascii_lowercase();
+                    &self.window, self.upcast(), qualified_name);
-                let ascii_lower_tag = LocalName::from(tag_copy);
-                let result = HTMLCollection::by_atomic_tag_name(&self.window,
-                                                                self.upcast(),
-                                                                tag_atom,
-                                                                ascii_lower_tag);
                 entry.insert(JS::from_ref(&*result));
                 result
             }

components/script/dom/element.rs

@@ -1441,7 +1441,7 @@ impl ElementMethods for Element {
     // https://dom.spec.whatwg.org/#dom-element-getelementsbytagname
     fn GetElementsByTagName(&self, localname: DOMString) -> Root<HTMLCollection> {
         let window = window_from_node(self);
-        HTMLCollection::by_tag_name(&window, self.upcast(), localname)
+        HTMLCollection::by_qualified_name(&window, self.upcast(), LocalName::from(&*localname))
     }
 
     // https://dom.spec.whatwg.org/#dom-element-getelementsbytagnamens

components/script/dom/htmlcollection.rs

@@ -15,7 +15,6 @@ use dom::node::Node;
 use dom::window::Window;
 use html5ever_atoms::{LocalName, QualName};
 use servo_atoms::Atom;
-use std::ascii::AsciiExt;
 use std::cell::Cell;
 use style::str::split_html_space_chars;
 
@@ -114,39 +113,52 @@ impl HTMLCollection {
         }
     }
 
-    pub fn by_tag_name(window: &Window, root: &Node, mut tag: DOMString)
+    // https://dom.spec.whatwg.org/#concept-getelementsbytagname
-                       -> Root<HTMLCollection> {
+    pub fn by_qualified_name(window: &Window, root: &Node, qualified_name: LocalName)
-        let tag_atom = LocalName::from(&*tag);
+                             -> Root<HTMLCollection> {
-        tag.make_ascii_lowercase();
+        // case 1
-        let ascii_lower_tag = LocalName::from(tag); // FIXME(ajeffrey): don't clone atom if it was already lowercased.
+        if qualified_name == local_name!("*") {
-        HTMLCollection::by_atomic_tag_name(window, root, tag_atom, ascii_lower_tag)
+            #[derive(JSTraceable, HeapSizeOf)]
-    }
+            struct AllFilter;
-
+            impl CollectionFilter for AllFilter {
-    pub fn by_atomic_tag_name(window: &Window, root: &Node, tag_atom: LocalName, ascii_lower_tag: LocalName)
+                fn filter(&self, _elem: &Element, _root: &Node) -> bool {
-                       -> Root<HTMLCollection> {
-        #[derive(JSTraceable, HeapSizeOf)]
-        struct TagNameFilter {
-            tag: LocalName,
-            ascii_lower_tag: LocalName,
-        }
-        impl CollectionFilter for TagNameFilter {
-            fn filter(&self, elem: &Element, _root: &Node) -> bool {
-                if self.tag == local_name!("*") {
                     true
-                } else if elem.html_element_in_html_document() {
+                }
-                    *elem.local_name() == self.ascii_lower_tag
+            }
-                } else {
+            return HTMLCollection::create(window, root, box AllFilter);
-                    *elem.local_name() == self.tag
+        }
+
+        #[derive(JSTraceable, HeapSizeOf)]
+        struct HtmlDocumentFilter {
+            qualified_name: LocalName,
+            ascii_lower_qualified_name: LocalName,
+        }
+        impl CollectionFilter for HtmlDocumentFilter {
+            fn filter(&self, elem: &Element, root: &Node) -> bool {
+                if root.is_in_html_doc() && elem.namespace() == &ns!(html) {    // case 2
+                    HTMLCollection::match_element(elem, &self.ascii_lower_qualified_name)
+                } else {    // case 2 and 3
+                    HTMLCollection::match_element(elem, &self.qualified_name)
                 }
             }
         }
-        let filter = TagNameFilter {
+
-            tag: tag_atom,
+        let filter = HtmlDocumentFilter {
-            ascii_lower_tag: ascii_lower_tag,
+            ascii_lower_qualified_name: qualified_name.to_ascii_lowercase(),
+            qualified_name: qualified_name,
         };
         HTMLCollection::create(window, root, box filter)
     }
 
+    fn match_element(elem: &Element, qualified_name: &LocalName) -> bool {
+        match *elem.prefix() {
+            None => elem.local_name() == qualified_name,
+            Some(ref prefix) => qualified_name.starts_with(prefix as &str) &&
+                qualified_name.find(":") == Some((prefix as &str).len()) &&
+                qualified_name.ends_with(elem.local_name() as &str),
+        }
+    }
+
     pub fn by_tag_name_ns(window: &Window, root: &Node, tag: DOMString,
                           maybe_ns: Option<DOMString>) -> Root<HTMLCollection> {
         let local = LocalName::from(tag);

components/script/dom/webidls/Document.webidl

@@ -28,8 +28,8 @@ interface Document : Node {
   readonly attribute DocumentType? doctype;
   [Pure]
   readonly attribute Element? documentElement;
-  HTMLCollection getElementsByTagName(DOMString localName);
+  HTMLCollection getElementsByTagName(DOMString qualifiedName);
-  HTMLCollection getElementsByTagNameNS(DOMString? namespace, DOMString localName);
+  HTMLCollection getElementsByTagNameNS(DOMString? namespace, DOMString qualifiedName);
   HTMLCollection getElementsByClassName(DOMString classNames);
 
   [NewObject, Throws]

tests/wpt/metadata/dom/nodes/Document-getElementsByTagName-xhtml.xhtml.ini

@@ -3,18 +3,8 @@
   [HTML element with uppercase tag name matches in XHTML documents]
     expected: FAIL
 
-  [Element in non-HTML namespace, prefix, lowercase name]
-    expected: FAIL
-
-  [Element in non-HTML namespace, prefix, uppercase name]
-    expected: FAIL
-
   [Element in HTML namespace, no prefix, non-ascii characters in name]
     expected: FAIL
 
   [Element in HTML namespace, prefix, non-ascii characters in name]
     expected: FAIL
-
-  [Element in non-HTML namespace, prefix, non-ascii characters in name]
-    expected: FAIL
-

tests/wpt/metadata/dom/nodes/Document-getElementsByTagName.html.ini

@@ -2,16 +2,3 @@
   type: testharness
   [Shouldn't be able to set unsigned properties on a HTMLCollection (strict mode)]
     expected: FAIL
-
-  [Element in non-HTML namespace, prefix, lowercase name]
-    expected: FAIL
-
-  [Element in non-HTML namespace, prefix, uppercase name]
-    expected: FAIL
-
-  [Element in HTML namespace, prefix, non-ascii characters in name]
-    expected: FAIL
-
-  [Element in non-HTML namespace, prefix, non-ascii characters in name]
-    expected: FAIL
-

tests/wpt/metadata/dom/nodes/Element-getElementsByTagName.html.ini

@@ -2,16 +2,3 @@
   type: testharness
   [Shouldn't be able to set unsigned properties on a HTMLCollection (strict mode)]
     expected: FAIL
-
-  [Element in non-HTML namespace, prefix, lowercase name]
-    expected: FAIL
-
-  [Element in non-HTML namespace, prefix, uppercase name]
-    expected: FAIL
-
-  [Element in HTML namespace, prefix, non-ascii characters in name]
-    expected: FAIL
-
-  [Element in non-HTML namespace, prefix, non-ascii characters in name]
-    expected: FAIL
-

tests/wpt/metadata/dom/nodes/case.html.ini

@@ -1,17 +0,0 @@
-[case.html]
-  type: testharness
-  [getElementsByTagName a:abc]
-    expected: FAIL
-
-  [getElementsByTagName a:Abc]
-    expected: FAIL
-
-  [getElementsByTagName a:ABC]
-    expected: FAIL
-
-  [getElementsByTagName a:ä]
-    expected: FAIL
-
-  [getElementsByTagName a:Ä]
-    expected: FAIL
-