From 592454defd39e0d186fe8b7e046f2b31e2454e0e Mon Sep 17 00:00:00 2001 From: Bruno de Oliveira Abinader Date: Fri, 8 Aug 2014 17:35:12 -0400 Subject: [PATCH 1/6] Fixed AttrValue::from_tokenlist indexes The following issues were found: - Single concatenated spaces were indexed as a single token; - Last token, if not followed by an HTML space character, was ignored; --- src/components/script/dom/attr.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/components/script/dom/attr.rs b/src/components/script/dom/attr.rs index 961089fa2a6..d31250fb0ee 100644 --- a/src/components/script/dom/attr.rs +++ b/src/components/script/dom/attr.rs @@ -37,10 +37,15 @@ impl AttrValue { pub fn from_tokenlist(list: DOMString) -> AttrValue { let mut indexes = vec![]; let mut last_index: uint = 0; + let length = list.len() - 1; for (index, ch) in list.as_slice().char_indices() { if HTML_SPACE_CHARACTERS.iter().any(|&space| space == ch) { - indexes.push((last_index, index)); + if last_index != index { + indexes.push((last_index, index)); + } last_index = index + 1; + } else if index == length { + indexes.push((last_index, index + 1)); } } return TokenListAttrValue(list, indexes); From e9f9afc32416b6452676ee668c12069afcbd7cd4 Mon Sep 17 00:00:00 2001 From: Bruno de Oliveira Abinader Date: Tue, 12 Aug 2014 10:58:18 -0400 Subject: [PATCH 2/6] Implemented Attribute's tokens() iterator --- src/components/script/dom/attr.rs | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/src/components/script/dom/attr.rs b/src/components/script/dom/attr.rs index d31250fb0ee..9f3fc9dc96e 100644 --- a/src/components/script/dom/attr.rs +++ b/src/components/script/dom/attr.rs @@ -16,9 +16,10 @@ use dom::virtualmethods::vtable_for; use servo_util::atom::Atom; use servo_util::namespace; use servo_util::namespace::Namespace; -use servo_util::str::{DOMString, HTML_SPACE_CHARACTERS}; +use servo_util::str::{DOMString, split_html_space_chars}; use std::cell::{Ref, RefCell}; use std::mem; +use std::slice::Items; pub enum AttrSettingType { FirstSetAttr, @@ -28,27 +29,16 @@ pub enum AttrSettingType { #[deriving(PartialEq, Clone, Encodable)] pub enum AttrValue { StringAttrValue(DOMString), - TokenListAttrValue(DOMString, Vec<(uint, uint)>), + TokenListAttrValue(DOMString, Vec), UIntAttrValue(DOMString, u32), AtomAttrValue(Atom), } impl AttrValue { - pub fn from_tokenlist(list: DOMString) -> AttrValue { - let mut indexes = vec![]; - let mut last_index: uint = 0; - let length = list.len() - 1; - for (index, ch) in list.as_slice().char_indices() { - if HTML_SPACE_CHARACTERS.iter().any(|&space| space == ch) { - if last_index != index { - indexes.push((last_index, index)); - } - last_index = index + 1; - } else if index == length { - indexes.push((last_index, index + 1)); - } - } - return TokenListAttrValue(list, indexes); + pub fn from_tokenlist(tokens: DOMString) -> AttrValue { + let atoms = split_html_space_chars(tokens.as_slice()) + .map(|token| Atom::from_slice(token)).collect(); + TokenListAttrValue(tokens, atoms) } pub fn from_u32(string: DOMString, default: u32) -> AttrValue { @@ -61,6 +51,12 @@ impl AttrValue { AtomAttrValue(value) } + pub fn tokens<'a>(&'a self) -> Option> { + match *self { + TokenListAttrValue(_, ref tokens) => Some(tokens.iter()), + _ => None + } + } } impl Str for AttrValue { From d17450106c3c55c42cbfc78fe7b13dc5e46c9060 Mon Sep 17 00:00:00 2001 From: Bruno de Oliveira Abinader Date: Fri, 22 Aug 2014 10:50:33 -0400 Subject: [PATCH 3/6] Use AttrValue::tokens() in Element::has_class() --- src/components/script/dom/element.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/components/script/dom/element.rs b/src/components/script/dom/element.rs index 425529fe6d4..7cd0fccbc7f 100644 --- a/src/components/script/dom/element.rs +++ b/src/components/script/dom/element.rs @@ -35,7 +35,7 @@ use style; use servo_util::atom::Atom; use servo_util::namespace; use servo_util::namespace::{Namespace, Null}; -use servo_util::str::{DOMString, null_str_as_empty_ref, split_html_space_chars}; +use servo_util::str::{DOMString, null_str_as_empty_ref}; use std::ascii::StrAsciiExt; use std::cell::{Cell, RefCell}; @@ -377,9 +377,11 @@ impl<'a> AttributeHandlers for JSRef<'a, Element> { } fn has_class(&self, name: &str) -> bool { - let class_names = self.get_string_attribute("class"); - let mut classes = split_html_space_chars(class_names.as_slice()); - classes.any(|class| name == class) + self.get_attribute(Null, "class").root().map(|attr| { + attr.deref().value().tokens().map(|mut tokens| { + tokens.any(|atom| atom.as_slice() == name) + }).unwrap_or(false) + }).unwrap_or(false) } fn set_atomic_attribute(&self, name: &str, value: DOMString) { From caa55bf9edd425abe58336248a45a89c10d81eb0 Mon Sep 17 00:00:00 2001 From: Bruno de Oliveira Abinader Date: Mon, 25 Aug 2014 13:35:44 -0400 Subject: [PATCH 4/6] Implement DOMTokenList.contains --- src/components/script/dom/domtokenlist.rs | 23 +++++++++- .../script/dom/webidls/DOMTokenList.webidl | 5 ++- src/test/content/test_domtokenlist.html | 45 +++++++++++++++++++ src/test/wpt/metadata/dom/interfaces.html.ini | 9 ---- .../dom/nodes/Element-classlist.html.ini | 39 ---------------- 5 files changed, 71 insertions(+), 50 deletions(-) create mode 100644 src/test/content/test_domtokenlist.html diff --git a/src/components/script/dom/domtokenlist.rs b/src/components/script/dom/domtokenlist.rs index cf44d5f7abc..3d252d36731 100644 --- a/src/components/script/dom/domtokenlist.rs +++ b/src/components/script/dom/domtokenlist.rs @@ -5,14 +5,16 @@ use dom::attr::{Attr, TokenListAttrValue}; use dom::bindings::codegen::Bindings::DOMTokenListBinding; use dom::bindings::codegen::Bindings::DOMTokenListBinding::DOMTokenListMethods; +use dom::bindings::error::{Fallible, InvalidCharacter, Syntax}; use dom::bindings::global::Window; use dom::bindings::js::{JS, JSRef, Temporary, OptionalRootable}; use dom::bindings::utils::{Reflector, Reflectable, reflect_dom_object}; use dom::element::{Element, AttributeHandlers}; use dom::node::window_from_node; +use servo_util::atom::Atom; use servo_util::namespace::Null; -use servo_util::str::DOMString; +use servo_util::str::{DOMString, HTML_SPACE_CHARACTERS}; #[deriving(Encodable)] pub struct DOMTokenList { @@ -47,6 +49,7 @@ impl Reflectable for DOMTokenList { trait PrivateDOMTokenListHelpers { fn attribute(&self) -> Option>; + fn check_token_exceptions<'a>(&self, token: &'a str) -> Fallible<&'a str>; } impl<'a> PrivateDOMTokenListHelpers for JSRef<'a, DOMTokenList> { @@ -54,6 +57,14 @@ impl<'a> PrivateDOMTokenListHelpers for JSRef<'a, DOMTokenList> { let element = self.element.root(); element.deref().get_attribute(Null, self.local_name) } + + fn check_token_exceptions<'a>(&self, token: &'a str) -> Fallible<&'a str> { + match token { + "" => Err(Syntax), + token if token.find(HTML_SPACE_CHARACTERS).is_some() => Err(InvalidCharacter), + token => Ok(token) + } + } } // http://dom.spec.whatwg.org/#domtokenlist @@ -92,4 +103,14 @@ impl<'a> DOMTokenListMethods for JSRef<'a, DOMTokenList> { *found = item.is_some(); item } + + // http://dom.spec.whatwg.org/#dom-domtokenlist-contains + fn Contains(&self, token: DOMString) -> Fallible { + self.check_token_exceptions(token.as_slice()).map(|slice| { + self.attribute().root().and_then(|attr| attr.value().tokens().map(|mut tokens| { + let atom = Atom::from_slice(slice); + tokens.any(|token| *token == atom) + })).unwrap_or(false) + }) + } } diff --git a/src/components/script/dom/webidls/DOMTokenList.webidl b/src/components/script/dom/webidls/DOMTokenList.webidl index 650d8abf6c6..bc32f4bf256 100644 --- a/src/components/script/dom/webidls/DOMTokenList.webidl +++ b/src/components/script/dom/webidls/DOMTokenList.webidl @@ -7,7 +7,10 @@ interface DOMTokenList { readonly attribute unsigned long length; getter DOMString? item(unsigned long index); - //boolean contains(DOMString token); + + [Throws] + boolean contains(DOMString token); + //void add(DOMString... tokens); //void remove(DOMString... tokens); //boolean toggle(DOMString token, optional boolean force); diff --git a/src/test/content/test_domtokenlist.html b/src/test/content/test_domtokenlist.html new file mode 100644 index 00000000000..bc32777ec0e --- /dev/null +++ b/src/test/content/test_domtokenlist.html @@ -0,0 +1,45 @@ + + + + + + + diff --git a/src/test/wpt/metadata/dom/interfaces.html.ini b/src/test/wpt/metadata/dom/interfaces.html.ini index f534ec23922..53d11f5d15f 100644 --- a/src/test/wpt/metadata/dom/interfaces.html.ini +++ b/src/test/wpt/metadata/dom/interfaces.html.ini @@ -1017,9 +1017,6 @@ [TreeWalker interface: document.createTreeWalker(document.body, NodeFilter.SHOW_ALL, null, false) must inherit property "nextNode" with the proper type (10)] expected: FAIL - [DOMTokenList interface: operation contains(DOMString)] - expected: FAIL - [DOMTokenList interface: operation add(DOMString)] expected: FAIL @@ -1029,12 +1026,6 @@ [DOMTokenList interface: operation toggle(DOMString,boolean)] expected: FAIL - [DOMTokenList interface: document.body.classList must inherit property "contains" with the proper type (2)] - expected: FAIL - - [DOMTokenList interface: calling contains(DOMString) on document.body.classList with too few arguments must throw TypeError] - expected: FAIL - [DOMTokenList interface: document.body.classList must inherit property "add" with the proper type (3)] expected: FAIL diff --git a/src/test/wpt/metadata/dom/nodes/Element-classlist.html.ini b/src/test/wpt/metadata/dom/nodes/Element-classlist.html.ini index 61f7cd11fbf..b1c1947cac2 100644 --- a/src/test/wpt/metadata/dom/nodes/Element-classlist.html.ini +++ b/src/test/wpt/metadata/dom/nodes/Element-classlist.html.ini @@ -6,24 +6,9 @@ [classList must be correct for an element that has classes] expected: FAIL - [classList.length must be 0 for an element that has no classes] - expected: FAIL - - [classList must not contain an undefined class] - expected: FAIL - - [classList.item() must return null for out-of-range index] - expected: FAIL - - [classList[index\] must be undefined for out-of-range index] - expected: FAIL - [empty classList should return the empty string since the ordered set parser skip the whitespaces] expected: FAIL - [.contains(empty_string) must throw a SYNTAX_ERR] - expected: FAIL - [.add(empty_string) must throw a SYNTAX_ERR] expected: FAIL @@ -33,9 +18,6 @@ [.toggle(empty_string) must throw a SYNTAX_ERR] expected: FAIL - [.contains(string_with_spaces) must throw an INVALID_CHARACTER_ERR] - expected: FAIL - [.add(string_with_spaces) must throw an INVALID_CHARACTER_ERR] expected: FAIL @@ -48,36 +30,18 @@ [computed style must update when setting .className] expected: FAIL - [classList.contains must update when .className is changed] - expected: FAIL - - [classList.contains must be case sensitive] - expected: FAIL - - [classList.contains must not match when punctuation characters are added] - expected: FAIL - [classList.add must not cause the CSS selector to stop matching] expected: FAIL - [classList.add must not remove existing classes] - expected: FAIL - [classList.contains case sensitivity must match a case-specific string] expected: FAIL [classList.length must correctly reflect the number of tokens] expected: FAIL - [classList.item(0) must return the first token] - expected: FAIL - [classList.item must return case-sensitive strings and preserve token order] expected: FAIL - [classList[0\] must return the first token] - expected: FAIL - [classList[index\] must return case-sensitive strings and preserve token order] expected: FAIL @@ -153,6 +117,3 @@ [classList.add should treat \\f as a space] expected: FAIL - [classList.length must be read-only] - expected: FAIL - From 400a31443b68df6dfd0f0b5520dbf67c1ff20c1b Mon Sep 17 00:00:00 2001 From: Bruno de Oliveira Abinader Date: Mon, 25 Aug 2014 13:37:19 -0400 Subject: [PATCH 5/6] Cleaned DOMTokenList code duplication --- src/components/script/dom/domtokenlist.rs | 29 ++++++----------------- 1 file changed, 7 insertions(+), 22 deletions(-) diff --git a/src/components/script/dom/domtokenlist.rs b/src/components/script/dom/domtokenlist.rs index 3d252d36731..11f7eaf59d0 100644 --- a/src/components/script/dom/domtokenlist.rs +++ b/src/components/script/dom/domtokenlist.rs @@ -2,7 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use dom::attr::{Attr, TokenListAttrValue}; +use dom::attr::Attr; use dom::bindings::codegen::Bindings::DOMTokenListBinding; use dom::bindings::codegen::Bindings::DOMTokenListBinding::DOMTokenListMethods; use dom::bindings::error::{Fallible, InvalidCharacter, Syntax}; @@ -71,31 +71,16 @@ impl<'a> PrivateDOMTokenListHelpers for JSRef<'a, DOMTokenList> { impl<'a> DOMTokenListMethods for JSRef<'a, DOMTokenList> { // http://dom.spec.whatwg.org/#dom-domtokenlist-length fn Length(&self) -> u32 { - let attribute = self.attribute().root(); - match attribute { - Some(attribute) => { - match *attribute.deref().value() { - TokenListAttrValue(_, ref indexes) => indexes.len() as u32, - _ => fail!("Expected a TokenListAttrValue"), - } - } - None => 0, - } + self.attribute().root().map(|attr| { + attr.value().tokens().map(|tokens| tokens.len()).unwrap_or(0) + }).unwrap_or(0) as u32 } // http://dom.spec.whatwg.org/#dom-domtokenlist-item fn Item(&self, index: u32) -> Option { - let attribute = self.attribute().root(); - attribute.and_then(|attribute| { - match *attribute.deref().value() { - TokenListAttrValue(ref value, ref indexes) => { - indexes.as_slice().get(index as uint).map(|&(start, end)| { - value.as_slice().slice(start, end).to_string() - }) - }, - _ => fail!("Expected a TokenListAttrValue"), - } - }) + self.attribute().root().and_then(|attr| attr.value().tokens().and_then(|mut tokens| { + tokens.idx(index as uint).map(|token| token.as_slice().to_string()) + })) } fn IndexedGetter(&self, index: u32, found: &mut bool) -> Option { From 23d1d1cf7b2e3f3940d2fd647aee669d2029c230 Mon Sep 17 00:00:00 2001 From: Bruno de Oliveira Abinader Date: Mon, 25 Aug 2014 13:57:38 -0400 Subject: [PATCH 6/6] Mark WPT tests as FAIL due to lack of DOMTokenList.toggle For now, these tests are being handled in test_element_classList.html until DOMTokenList.toggle gets implemented. Created issue #3138 to track DOMTokenList.toggle implementation. --- src/test/content/test_element_classList.html | 21 +++++++++++++++++++ .../dom/nodes/Element-classlist.html.ini | 6 ++++++ 2 files changed, 27 insertions(+) create mode 100644 src/test/content/test_element_classList.html diff --git a/src/test/content/test_element_classList.html b/src/test/content/test_element_classList.html new file mode 100644 index 00000000000..ade4aa6a5a1 --- /dev/null +++ b/src/test/content/test_element_classList.html @@ -0,0 +1,21 @@ + + + + + + + + + + diff --git a/src/test/wpt/metadata/dom/nodes/Element-classlist.html.ini b/src/test/wpt/metadata/dom/nodes/Element-classlist.html.ini index b1c1947cac2..c072735d502 100644 --- a/src/test/wpt/metadata/dom/nodes/Element-classlist.html.ini +++ b/src/test/wpt/metadata/dom/nodes/Element-classlist.html.ini @@ -102,6 +102,12 @@ [classList must stringify to an empty string when all classes have been removed] expected: FAIL + [classList.item(0) must return null when all classes have been removed] + expected: FAIL + + [classList[0\] must be undefined when all classes have been removed] + expected: FAIL + [classList.add should treat " " as a space] expected: FAIL