diff --git a/src/components/main/layout/wrapper.rs b/src/components/main/layout/wrapper.rs index ffd5d76721c..34bd613e957 100644 --- a/src/components/main/layout/wrapper.rs +++ b/src/components/main/layout/wrapper.rs @@ -19,12 +19,13 @@ use script::dom::element::{Element, HTMLAreaElementTypeId, HTMLAnchorElementType use script::dom::element::{HTMLLinkElementTypeId}; use script::dom::htmliframeelement::HTMLIFrameElement; use script::dom::htmlimageelement::HTMLImageElement; +use script::dom::namespace; use script::dom::namespace::Namespace; use script::dom::node::{AbstractNode, DocumentNodeTypeId, ElementNodeTypeId, Node, NodeTypeId}; use script::dom::text::Text; use servo_msg::constellation_msg::{PipelineId, SubpageId}; use std::cast; -use style::{PropertyDeclarationBlock, TElement, TNode}; +use style::{PropertyDeclarationBlock, TElement, TNode, AttrSelector}; /// A wrapper so that layout can access only the methods that it should have access to. Layout must /// only ever see these and must never see instances of `AbstractNode`. @@ -246,7 +247,7 @@ impl<'ln> TNode> for LayoutNode<'ln> { self.node.node().prev_sibling.map(|node| self.new_with_this_lifetime(node)) } } - + fn next_sibling(&self) -> Option> { unsafe { self.node.node().next_sibling.map(|node| self.new_with_this_lifetime(node)) @@ -280,6 +281,21 @@ impl<'ln> TNode> for LayoutNode<'ln> { } }) } + + fn match_attr(&self, attr: &AttrSelector, test: |&str| -> bool) -> bool { + self.with_element(|element| { + let name = if element.element.html_element_in_html_document() { + attr.lower_name.as_slice() + } else { + attr.name.as_slice() + }; + // FIXME: avoid .clone() here? See #1367 + match element.get_attr(attr.namespace.clone(), name) { + Some(value) => test(value), + None => false, + } + }) + } } pub struct LayoutNodeChildrenIterator<'a> { @@ -377,14 +393,17 @@ impl<'le> LayoutElement<'le> { } impl<'le> TElement for LayoutElement<'le> { + #[inline] fn get_local_name<'a>(&'a self) -> &'a str { self.element.tag_name.as_slice() } + #[inline] fn get_namespace_url<'a>(&'a self) -> &'a str { self.element.namespace.to_str().unwrap_or("") } + #[inline] fn get_attr(&self, ns_url: Option<~str>, name: &str) -> Option<&'static str> { let namespace = Namespace::from_str(ns_url); unsafe { self.element.get_attr_val_for_layout(namespace, name) } @@ -398,7 +417,8 @@ impl<'le> TElement for LayoutElement<'le> { ElementNodeTypeId(HTMLAnchorElementTypeId) | ElementNodeTypeId(HTMLAreaElementTypeId) | ElementNodeTypeId(HTMLLinkElementTypeId) => { - self.get_attr(None, "href").map(|val| val.to_owned()) + unsafe { self.element.get_attr_val_for_layout(namespace::Null, "href") } + .map(|val| val.to_owned()) } _ => None, } diff --git a/src/components/script/dom/element.rs b/src/components/script/dom/element.rs index 309204ee7a9..3d080a56de7 100644 --- a/src/components/script/dom/element.rs +++ b/src/components/script/dom/element.rs @@ -7,7 +7,7 @@ use dom::attr::Attr; use dom::attrlist::AttrList; use dom::bindings::utils::{Reflectable, DOMString, ErrorResult, Fallible, Reflector}; -use dom::bindings::utils::{null_str_as_empty, NamespaceError}; +use dom::bindings::utils::NamespaceError; use dom::bindings::utils::{InvalidCharacter, QName, Name, InvalidXMLName, xml_name_type}; use dom::htmlcollection::HTMLCollection; use dom::clientrect::ClientRect; @@ -137,30 +137,23 @@ impl Element { } } - pub fn normalize_attr_name(&self, name: Option) -> ~str { - //FIXME: Throw for XML-invalid names + pub fn html_element_in_html_document(&self) -> bool { let owner = self.node.owner_doc(); - if owner.document().doctype == document::HTML { // && self.namespace == Namespace::HTML - null_str_as_empty(&name).to_ascii_lower() - } else { - null_str_as_empty(&name) - } + self.namespace == namespace::HTML && + // FIXME: check that this matches what the spec calls "is in an HTML document" + owner.document().doctype == document::HTML } pub fn get_attribute(&self, namespace: Namespace, name: &str) -> Option<@mut Attr> { - // FIXME: only case-insensitive in the HTML namespace (as opposed to SVG, etc.) - let name = name.to_ascii_lower(); self.attrs.iter().find(|attr| { name == attr.local_name && attr.namespace == namespace }).map(|&x| x) } - pub unsafe fn get_attr_val_for_layout(&self, namespace: Namespace, name: &str) + pub unsafe fn get_attr_val_for_layout(&self, namespace: Namespace, name: &str) -> Option<&'static str> { - // FIXME: only case-insensitive in the HTML namespace (as opposed to SVG, etc.) - let name = name.to_ascii_lower(); self.attrs.iter().find(|attr: & &@mut Attr| { // unsafely avoid a borrow because this is accessed by many tasks // during parallel layout @@ -174,8 +167,6 @@ impl Element { pub fn set_attr(&mut self, abstract_self: AbstractNode, name: DOMString, value: DOMString) -> ErrorResult { - // FIXME: HTML-in-HTML only. - let name = name.to_ascii_lower(); self.set_attribute(abstract_self, namespace::Null, name, value) } @@ -402,6 +393,11 @@ impl Element { } pub fn GetAttribute(&self, name: DOMString) -> Option { + let name = if self.html_element_in_html_document() { + name.to_ascii_lower() + } else { + name + }; self.get_attribute(Null, name).map(|s| s.Value()) } @@ -413,6 +409,12 @@ impl Element { pub fn SetAttribute(&mut self, abstract_self: AbstractNode, name: DOMString, value: DOMString) -> ErrorResult { + // FIXME: If name does not match the Name production in XML, throw an "InvalidCharacterError" exception. + let name = if self.html_element_in_html_document() { + name.to_ascii_lower() + } else { + name + }; self.set_attr(abstract_self, name, value) } @@ -435,6 +437,11 @@ impl Element { pub fn RemoveAttribute(&mut self, abstract_self: AbstractNode, name: DOMString) -> ErrorResult { + let name = if self.html_element_in_html_document() { + name.to_ascii_lower() + } else { + name + }; self.remove_attribute(abstract_self, namespace::Null, name) } diff --git a/src/components/style/node.rs b/src/components/style/node.rs index df136a97be2..455e1d1e93d 100644 --- a/src/components/style/node.rs +++ b/src/components/style/node.rs @@ -5,6 +5,9 @@ //! Traits that nodes must implement. Breaks the otherwise-cyclic dependency between layout and //! style. +use selectors::AttrSelector; + + pub trait TNode : Clone { fn parent_node(&self) -> Option; fn prev_sibling(&self) -> Option; @@ -15,6 +18,8 @@ pub trait TNode : Clone { /// FIXME(pcwalton): This should not use the `with` pattern. fn with_element<'a, R>(&self, f: |&E| -> R) -> R; + + fn match_attr(&self, attr: &AttrSelector, test: |&str| -> bool) -> bool; } pub trait TElement { diff --git a/src/components/style/selector_matching.rs b/src/components/style/selector_matching.rs index 6e0ae833a1f..a3984f8603a 100644 --- a/src/components/style/selector_matching.rs +++ b/src/components/style/selector_matching.rs @@ -513,22 +513,22 @@ fn matches_simple_selector>(selector: &SimpleSelector, ele }) } - AttrExists(ref attr) => match_attribute(attr, element, |_| true), - AttrEqual(ref attr, ref value) => match_attribute(attr, element, |v| v == value.as_slice()), - AttrIncludes(ref attr, ref value) => match_attribute(attr, element, |attr_value| { + AttrExists(ref attr) => element.match_attr(attr, |_| true), + AttrEqual(ref attr, ref value) => element.match_attr(attr, |v| v == value.as_slice()), + AttrIncludes(ref attr, ref value) => element.match_attr(attr, |attr_value| { attr_value.split(SELECTOR_WHITESPACE).any(|v| v == value.as_slice()) }), AttrDashMatch(ref attr, ref value, ref dashing_value) - => match_attribute(attr, element, |attr_value| { + => element.match_attr(attr, |attr_value| { attr_value == value.as_slice() || attr_value.starts_with(dashing_value.as_slice()) }), - AttrPrefixMatch(ref attr, ref value) => match_attribute(attr, element, |attr_value| { + AttrPrefixMatch(ref attr, ref value) => element.match_attr(attr, |attr_value| { attr_value.starts_with(value.as_slice()) }), - AttrSubstringMatch(ref attr, ref value) => match_attribute(attr, element, |attr_value| { + AttrSubstringMatch(ref attr, ref value) => element.match_attr(attr, |attr_value| { attr_value.contains(value.as_slice()) }), - AttrSuffixMatch(ref attr, ref value) => match_attribute(attr, element, |attr_value| { + AttrSuffixMatch(ref attr, ref value) => element.match_attr(attr, |attr_value| { attr_value.ends_with(value.as_slice()) }), @@ -696,21 +696,6 @@ fn matches_last_child>(element: &N) -> bool { } } -#[inline] -fn match_attribute>( - attr: &AttrSelector, - element: &N, - f: |&str| -> bool) - -> bool { - element.with_element(|element: &E| { - // FIXME: avoid .clone() here? See #1367 - match element.get_attr(attr.namespace.clone(), attr.name) { - None => false, - Some(value) => f(value) - } - }) -} #[cfg(test)] mod tests { diff --git a/src/components/style/selectors.rs b/src/components/style/selectors.rs index 860c4a6d6bc..24b2d0e4b20 100644 --- a/src/components/style/selectors.rs +++ b/src/components/style/selectors.rs @@ -88,6 +88,7 @@ pub enum SimpleSelector { #[deriving(Eq, Clone)] pub struct AttrSelector { name: ~str, + lower_name: ~str, namespace: Option<~str>, } @@ -423,6 +424,7 @@ fn parse_attribute_selector(content: ~[ComponentValue], namespaces: &NamespaceMa QualifiedName(_, None) => fail!("Implementation error, this should not happen."), QualifiedName(namespace, Some(local_name)) => AttrSelector { namespace: namespace, + lower_name: local_name.to_ascii_lower(), name: local_name, }, }; diff --git a/src/components/style/style.rc b/src/components/style/style.rc index a8e39526fa1..c5ae2428064 100644 --- a/src/components/style/style.rc +++ b/src/components/style/style.rc @@ -23,7 +23,7 @@ pub use properties::{cascade, PropertyDeclaration, ComputedValues, computed_valu pub use properties::{PropertyDeclarationBlock, parse_style_attribute}; // Style attributes pub use errors::with_errors_silenced; pub use node::{TElement, TNode}; -pub use selectors::{PseudoElement, Before, After}; +pub use selectors::{PseudoElement, Before, After, AttrSelector}; mod stylesheets; mod errors;