diff options
author | bors-servo <lbergstrom+bors@mozilla.com> | 2016-11-04 12:41:22 -0500 |
---|---|---|
committer | GitHub <noreply@github.com> | 2016-11-04 12:41:22 -0500 |
commit | cfef68f92d1c8cdc928d3c0b60f22de4b40a97be (patch) | |
tree | 6d5b92c522c804d0f5a2846fdc3a7fc43beb0e20 | |
parent | 7c6793fc28423e518a0ceaae1176048fcb2caa3a (diff) | |
parent | af5d10969559ac193cf6071d37815580ab639cfe (diff) | |
download | servo-cfef68f92d1c8cdc928d3c0b60f22de4b40a97be.tar.gz servo-cfef68f92d1c8cdc928d3c0b60f22de4b40a97be.zip |
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 -->
-rw-r--r-- | components/script/dom/document.rs | 15 | ||||
-rw-r--r-- | components/script/dom/element.rs | 2 | ||||
-rw-r--r-- | components/script/dom/htmlcollection.rs | 60 | ||||
-rw-r--r-- | components/script/dom/webidls/Document.webidl | 4 | ||||
-rw-r--r-- | tests/wpt/metadata/dom/nodes/Document-getElementsByTagName-xhtml.xhtml.ini | 10 | ||||
-rw-r--r-- | tests/wpt/metadata/dom/nodes/Document-getElementsByTagName.html.ini | 13 | ||||
-rw-r--r-- | tests/wpt/metadata/dom/nodes/Element-getElementsByTagName.html.ini | 13 | ||||
-rw-r--r-- | tests/wpt/metadata/dom/nodes/case.html.ini | 17 |
8 files changed, 44 insertions, 90 deletions
diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 56ed097bce0..5f930f0814d 100644 --- a/components/script/dom/document.rs +++ b/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> { - let tag_atom = LocalName::from(&*tag_name); - match self.tag_map.borrow_mut().entry(tag_atom.clone()) { + fn GetElementsByTagName(&self, qualified_name: DOMString) -> Root<HTMLCollection> { + let qualified_name = LocalName::from(&*qualified_name); + 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; - tag_copy.make_ascii_lowercase(); - let ascii_lower_tag = LocalName::from(tag_copy); - let result = HTMLCollection::by_atomic_tag_name(&self.window, - self.upcast(), - tag_atom, - ascii_lower_tag); + let result = HTMLCollection::by_qualified_name( + &self.window, self.upcast(), qualified_name); entry.insert(JS::from_ref(&*result)); result } diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index f2123fbbec6..2217651f778 100644 --- a/components/script/dom/element.rs +++ b/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 diff --git a/components/script/dom/htmlcollection.rs b/components/script/dom/htmlcollection.rs index 3fd73103107..71fc6a1ecf1 100644 --- a/components/script/dom/htmlcollection.rs +++ b/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) - -> Root<HTMLCollection> { - let tag_atom = LocalName::from(&*tag); - tag.make_ascii_lowercase(); - let ascii_lower_tag = LocalName::from(tag); // FIXME(ajeffrey): don't clone atom if it was already lowercased. - HTMLCollection::by_atomic_tag_name(window, root, tag_atom, ascii_lower_tag) - } + // https://dom.spec.whatwg.org/#concept-getelementsbytagname + pub fn by_qualified_name(window: &Window, root: &Node, qualified_name: LocalName) + -> Root<HTMLCollection> { + // case 1 + if qualified_name == local_name!("*") { + #[derive(JSTraceable, HeapSizeOf)] + struct AllFilter; + impl CollectionFilter for AllFilter { + fn filter(&self, _elem: &Element, _root: &Node) -> bool { + true + } + } + return HTMLCollection::create(window, root, box AllFilter); + } - pub fn by_atomic_tag_name(window: &Window, root: &Node, tag_atom: LocalName, ascii_lower_tag: LocalName) - -> Root<HTMLCollection> { #[derive(JSTraceable, HeapSizeOf)] - struct TagNameFilter { - tag: LocalName, - ascii_lower_tag: LocalName, + struct HtmlDocumentFilter { + qualified_name: LocalName, + ascii_lower_qualified_name: 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 { - *elem.local_name() == self.tag + 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, - ascii_lower_tag: ascii_lower_tag, + + let filter = HtmlDocumentFilter { + 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); diff --git a/components/script/dom/webidls/Document.webidl b/components/script/dom/webidls/Document.webidl index 81a6c032df7..192c3d03714 100644 --- a/components/script/dom/webidls/Document.webidl +++ b/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 getElementsByTagNameNS(DOMString? namespace, DOMString localName); + HTMLCollection getElementsByTagName(DOMString qualifiedName); + HTMLCollection getElementsByTagNameNS(DOMString? namespace, DOMString qualifiedName); HTMLCollection getElementsByClassName(DOMString classNames); [NewObject, Throws] diff --git a/tests/wpt/metadata/dom/nodes/Document-getElementsByTagName-xhtml.xhtml.ini b/tests/wpt/metadata/dom/nodes/Document-getElementsByTagName-xhtml.xhtml.ini index e86af236483..bfa90316d52 100644 --- a/tests/wpt/metadata/dom/nodes/Document-getElementsByTagName-xhtml.xhtml.ini +++ b/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 - diff --git a/tests/wpt/metadata/dom/nodes/Document-getElementsByTagName.html.ini b/tests/wpt/metadata/dom/nodes/Document-getElementsByTagName.html.ini index 35ef59b4c8f..aa26e4b84da 100644 --- a/tests/wpt/metadata/dom/nodes/Document-getElementsByTagName.html.ini +++ b/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 - diff --git a/tests/wpt/metadata/dom/nodes/Element-getElementsByTagName.html.ini b/tests/wpt/metadata/dom/nodes/Element-getElementsByTagName.html.ini index 7a4d6ec4ec3..721558ef311 100644 --- a/tests/wpt/metadata/dom/nodes/Element-getElementsByTagName.html.ini +++ b/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 - diff --git a/tests/wpt/metadata/dom/nodes/case.html.ini b/tests/wpt/metadata/dom/nodes/case.html.ini deleted file mode 100644 index e7dd2689077..00000000000 --- a/tests/wpt/metadata/dom/nodes/case.html.ini +++ /dev/null @@ -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 - |