From f2db7faf19c34165c08f1d5839a7461bc04c20a5 Mon Sep 17 00:00:00 2001 From: Tetsuharu OHZEKI Date: Sat, 26 Jul 2014 03:58:25 +0900 Subject: [PATCH 1/2] Port `id` attribute to use atom. --- src/components/script/dom/attr.rs | 7 +++++++ src/components/script/dom/element.rs | 11 ++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/components/script/dom/attr.rs b/src/components/script/dom/attr.rs index 2762910171f..f58631176f1 100644 --- a/src/components/script/dom/attr.rs +++ b/src/components/script/dom/attr.rs @@ -30,6 +30,7 @@ pub enum AttrValue { StringAttrValue(DOMString), TokenListAttrValue(DOMString, Vec<(uint, uint)>), UIntAttrValue(DOMString, u32), + AtomAttrValue(Atom), } impl AttrValue { @@ -50,11 +51,17 @@ impl AttrValue { UIntAttrValue(string, result) } + pub fn from_atomic(string: DOMString) -> AttrValue { + let value = Atom::from_slice(string.as_slice()); + AtomAttrValue(value) + } + pub fn as_slice<'a>(&'a self) -> &'a str { match *self { StringAttrValue(ref value) | TokenListAttrValue(ref value, _) | UIntAttrValue(ref value, _) => value.as_slice(), + AtomAttrValue(ref value) => value.as_slice(), } } } diff --git a/src/components/script/dom/element.rs b/src/components/script/dom/element.rs index 9301e61b20a..6fc0ba1b4c0 100644 --- a/src/components/script/dom/element.rs +++ b/src/components/script/dom/element.rs @@ -239,6 +239,8 @@ pub trait AttributeHandlers { fn notify_attribute_changed(&self, local_name: DOMString); fn has_class(&self, name: &str) -> bool; + fn set_atomic_attribute(&self, name: &str, value: DOMString); + // http://www.whatwg.org/html/#reflecting-content-attributes-in-idl-attributes fn get_url_attribute(&self, name: &str) -> DOMString; fn set_url_attribute(&self, name: &str, value: DOMString); @@ -362,6 +364,12 @@ impl<'a> AttributeHandlers for JSRef<'a, Element> { classes.any(|class| name == class) } + fn set_atomic_attribute(&self, name: &str, value: DOMString) { + assert!(name == name.to_ascii_lower().as_slice()); + let value = AttrValue::from_atomic(value); + self.set_attribute(name, value); + } + fn get_url_attribute(&self, name: &str) -> DOMString { // XXX Resolve URL. self.get_string_attribute(name) @@ -459,7 +467,7 @@ impl<'a> ElementMethods for JSRef<'a, Element> { // http://dom.spec.whatwg.org/#dom-element-id fn SetId(&self, id: DOMString) { - self.set_string_attribute("id", id); + self.set_atomic_attribute("id", id); } // http://dom.spec.whatwg.org/#dom-element-classname @@ -824,6 +832,7 @@ impl<'a> VirtualMethods for JSRef<'a, Element> { fn parse_plain_attribute(&self, name: &str, value: DOMString) -> AttrValue { match name { + "id" => AttrValue::from_atomic(value), "class" => AttrValue::from_tokenlist(value), _ => self.super_type().unwrap().parse_plain_attribute(name, value), } From 794ce9cd4d6058a8aa30120742f292fbc7d95e2a Mon Sep 17 00:00:00 2001 From: Tetsuharu OHZEKI Date: Sat, 26 Jul 2014 04:33:54 +0900 Subject: [PATCH 2/2] Use atom to match id selector. --- src/components/layout/wrapper.rs | 5 +++++ src/components/script/dom/attr.rs | 10 +++++++++ src/components/script/dom/element.rs | 26 ++++++++++++++++++++++- src/components/style/node.rs | 1 + src/components/style/selector_matching.rs | 7 +++--- src/components/style/selectors.rs | 12 ++++++----- 6 files changed, 51 insertions(+), 10 deletions(-) diff --git a/src/components/layout/wrapper.rs b/src/components/layout/wrapper.rs index adf0151749c..b0ecbbcd2fd 100644 --- a/src/components/layout/wrapper.rs +++ b/src/components/layout/wrapper.rs @@ -393,6 +393,11 @@ impl<'le> TElement for LayoutElement<'le> { self.element.node.get_hover_state_for_layout() } } + + #[inline] + fn get_id(&self) -> Option { + unsafe { self.element.get_attr_atom_for_layout(&namespace::Null, "id") } + } } fn get_content(content_list: &content::T) -> String { diff --git a/src/components/script/dom/attr.rs b/src/components/script/dom/attr.rs index f58631176f1..0e499ebf477 100644 --- a/src/components/script/dom/attr.rs +++ b/src/components/script/dom/attr.rs @@ -167,6 +167,7 @@ impl<'a> AttrMethods for JSRef<'a, Attr> { pub trait AttrHelpersForLayout { unsafe fn value_ref_forever(&self) -> &'static str; + unsafe fn value_atom_forever(&self) -> Option; } impl AttrHelpersForLayout for Attr { @@ -175,4 +176,13 @@ impl AttrHelpersForLayout for Attr { let value = mem::transmute::<&RefCell, &AttrValue>(self.value.deref()); value.as_slice() } + + unsafe fn value_atom_forever(&self) -> Option { + // cast to point to T in RefCell directly + let value = mem::transmute::<&RefCell, &AttrValue>(self.value.deref()); + match *value { + AtomAttrValue(ref val) => Some(val.clone()), + _ => None, + } + } } diff --git a/src/components/script/dom/element.rs b/src/components/script/dom/element.rs index 6fc0ba1b4c0..afc176b8a26 100644 --- a/src/components/script/dom/element.rs +++ b/src/components/script/dom/element.rs @@ -6,7 +6,7 @@ use cssparser::tokenize; use dom::attr::{Attr, ReplacedAttr, FirstSetAttr, AttrHelpersForLayout}; -use dom::attr::{AttrValue, StringAttrValue, UIntAttrValue}; +use dom::attr::{AttrValue, StringAttrValue, UIntAttrValue, AtomAttrValue}; use dom::attrlist::AttrList; use dom::bindings::codegen::Bindings::AttrBinding::AttrMethods; use dom::bindings::codegen::Bindings::ElementBinding; @@ -168,6 +168,7 @@ impl Element { pub trait RawLayoutElementHelpers { unsafe fn get_attr_val_for_layout(&self, namespace: &Namespace, name: &str) -> Option<&'static str>; + unsafe fn get_attr_atom_for_layout(&self, namespace: &Namespace, name: &str) -> Option; } impl RawLayoutElementHelpers for Element { @@ -184,6 +185,20 @@ impl RawLayoutElementHelpers for Element { (*attr).value_ref_forever() }) } + + #[inline] + unsafe fn get_attr_atom_for_layout(&self, namespace: &Namespace, name: &str) + -> Option { + // cast to point to T in RefCell directly + let attrs: *Vec> = mem::transmute(&self.attrs); + (*attrs).iter().find(|attr: & &JS| { + let attr = attr.unsafe_get(); + name == (*attr).local_name.as_slice() && (*attr).namespace == *namespace + }).and_then(|attr| { + let attr = attr.unsafe_get(); + (*attr).value_atom_forever() + }) + } } pub trait LayoutElementHelpers { @@ -901,4 +916,13 @@ impl<'a> style::TElement for JSRef<'a, Element> { let node: &JSRef = NodeCast::from_ref(self); node.get_hover_state() } + fn get_id<'a>(&self) -> Option { + self.get_attribute(namespace::Null, "id").map(|attr| { + let attr = attr.root(); + match *attr.value() { + AtomAttrValue(ref val) => val.clone(), + _ => fail!("`id` attribute should be AtomAttrValue"), + } + }) + } } diff --git a/src/components/style/node.rs b/src/components/style/node.rs index 5d00d34c746..62c588dffd4 100644 --- a/src/components/style/node.rs +++ b/src/components/style/node.rs @@ -26,5 +26,6 @@ pub trait TElement { fn get_local_name<'a>(&'a self) -> &'a Atom; fn get_namespace<'a>(&'a self) -> &'a Namespace; fn get_hover_state(&self) -> bool; + fn get_id(&self) -> Option; } diff --git a/src/components/style/selector_matching.rs b/src/components/style/selector_matching.rs index eeff8b3c60e..3b6de24c1c5 100644 --- a/src/components/style/selector_matching.rs +++ b/src/components/style/selector_matching.rs @@ -267,7 +267,7 @@ impl SelectorMap { match *ss { // TODO(pradeep): Implement case-sensitivity based on the document type and quirks // mode. - IDSelector(ref id) => return Some(id.clone()), + IDSelector(ref id) => return Some(id.as_slice().clone().to_string()), _ => {} } } @@ -691,9 +691,8 @@ fn matches_simple_selector { *shareable = false; let element = element.as_element(); - element.get_attr(&namespace::Null, "id") - .map_or(false, |attr| { - attr == id.as_slice() + element.get_id().map_or(false, |attr| { + attr == *id }) } // TODO: cache and intern class names on elements. diff --git a/src/components/style/selectors.rs b/src/components/style/selectors.rs index 951f85b8e20..8eadd371086 100644 --- a/src/components/style/selectors.rs +++ b/src/components/style/selectors.rs @@ -10,6 +10,7 @@ use sync::Arc; use cssparser::ast::*; use cssparser::parse_nth; +use servo_util::atom::Atom; use servo_util::namespace::Namespace; use servo_util::namespace; @@ -56,7 +57,7 @@ pub enum Combinator { #[deriving(PartialEq, Clone)] pub enum SimpleSelector { - IDSelector(String), + IDSelector(Atom), ClassSelector(String), LocalNameSelector(String), NamespaceSelector(Namespace), @@ -306,7 +307,7 @@ fn parse_one_simple_selector(iter: &mut Iter, namespaces: &NamespaceMap, inside_ -> SimpleSelectorParseResult { match iter.peek() { Some(&IDHash(_)) => match iter.next() { - Some(IDHash(id)) => SimpleSelectorResult(IDSelector(id)), + Some(IDHash(id)) => SimpleSelectorResult(IDSelector(Atom::from_slice(id.as_slice()))), _ => fail!("Implementation error, this should not happen."), }, Some(&Delim('.')) => { @@ -572,6 +573,7 @@ fn skip_whitespace(iter: &mut Iter) -> bool { mod tests { use sync::Arc; use cssparser; + use servo_util::atom::Atom; use servo_util::namespace; use namespaces::NamespaceMap; use super::*; @@ -611,7 +613,7 @@ mod tests { }))) assert!(parse("#bar") == Some(vec!(Selector{ compound_selectors: Arc::new(CompoundSelector { - simple_selectors: vec!(IDSelector("bar".to_string())), + simple_selectors: vec!(IDSelector(Atom::from_slice("bar"))), next: None, }), pseudo_element: None, @@ -621,7 +623,7 @@ mod tests { compound_selectors: Arc::new(CompoundSelector { simple_selectors: vec!(LocalNameSelector("e".to_string()), ClassSelector("foo".to_string()), - IDSelector("bar".to_string())), + IDSelector(Atom::from_slice("bar"))), next: None, }), pseudo_element: None, @@ -629,7 +631,7 @@ mod tests { }))) assert!(parse("e.foo #bar") == Some(vec!(Selector{ compound_selectors: Arc::new(CompoundSelector { - simple_selectors: vec!(IDSelector("bar".to_string())), + simple_selectors: vec!(IDSelector(Atom::from_slice("bar"))), next: Some((box CompoundSelector { simple_selectors: vec!(LocalNameSelector("e".to_string()), ClassSelector("foo".to_string())),