From fdafc3701f9a72e3e16b0580b838b2c541a33d04 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Sat, 25 Jan 2014 09:02:20 -0800 Subject: [PATCH] Make get_attr() take a &Namespace rather than Option<~str> --- src/components/main/layout/box_.rs | 3 ++- src/components/main/layout/wrapper.rs | 19 +++++++++++-------- src/components/script/dom/element.rs | 5 +++-- src/components/style/node.rs | 3 ++- src/components/style/selector_matching.rs | 10 ++++++---- 5 files changed, 24 insertions(+), 16 deletions(-) diff --git a/src/components/main/layout/box_.rs b/src/components/main/layout/box_.rs index 66125587be9..4c68e3b0511 100644 --- a/src/components/main/layout/box_.rs +++ b/src/components/main/layout/box_.rs @@ -22,6 +22,7 @@ use servo_net::local_image_cache::LocalImageCache; use servo_util::geometry::Au; use servo_util::geometry; use servo_util::range::*; +use servo_util::namespace; use std::cast; use std::cell::RefCell; use std::cmp::ApproxEq; @@ -127,7 +128,7 @@ impl ImageBoxInfo { fn convert_length(node: &LayoutNode, name: &str) -> Option { node.with_element(|element| { - element.get_attr(None, name).and_then(|string| { + element.get_attr(&namespace::Null, name).and_then(|string| { let n: Option = FromStr::from_str(string); n }).and_then(|pixels| Some(Au::from_px(pixels))) diff --git a/src/components/main/layout/wrapper.rs b/src/components/main/layout/wrapper.rs index c0c4c0bd754..852696705d8 100644 --- a/src/components/main/layout/wrapper.rs +++ b/src/components/main/layout/wrapper.rs @@ -15,7 +15,6 @@ //! onto these objects and cause use-after-free. use extra::url::Url; -use script::dom::bindings::utils::null_str_as_empty_ref; use script::dom::element::{Element, HTMLAreaElementTypeId, HTMLAnchorElementTypeId}; use script::dom::element::{HTMLLinkElementTypeId}; use script::dom::htmliframeelement::HTMLIFrameElement; @@ -290,10 +289,15 @@ impl<'ln> TNode> for LayoutNode<'ln> { } 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, + match attr.namespace { + Some(ref url) => { + match element.get_attr(&namespace::Namespace::from_str(url.as_slice()), name) { + Some(value) => test(value), + None => false, + } + }, + // FIXME: support `*|attr`, attribute selectors in any namespace + None => return false, } }) } @@ -405,8 +409,7 @@ impl<'le> TElement for LayoutElement<'le> { } #[inline] - fn get_attr(&self, ns_url: Option<~str>, name: &str) -> Option<&'static str> { - let namespace = Namespace::from_str(null_str_as_empty_ref(&ns_url)); + fn get_attr(&self, namespace: &Namespace, name: &str) -> Option<&'static str> { unsafe { self.element.get_attr_val_for_layout(namespace, name) } } @@ -418,7 +421,7 @@ impl<'le> TElement for LayoutElement<'le> { ElementNodeTypeId(HTMLAnchorElementTypeId) | ElementNodeTypeId(HTMLAreaElementTypeId) | ElementNodeTypeId(HTMLLinkElementTypeId) => { - unsafe { self.element.get_attr_val_for_layout(namespace::Null, "href") } + 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 f3c75db7a82..a834cb8e9c1 100644 --- a/src/components/script/dom/element.rs +++ b/src/components/script/dom/element.rs @@ -152,13 +152,14 @@ impl Element { }).map(|&x| x) } - pub unsafe fn get_attr_val_for_layout(&self, namespace: Namespace, name: &str) + #[inline] + pub unsafe fn get_attr_val_for_layout(&self, namespace: &Namespace, name: &str) -> Option<&'static str> { self.attrs.iter().find(|attr: & &@mut Attr| { // unsafely avoid a borrow because this is accessed by many tasks // during parallel layout let attr: ***Box = cast::transmute(attr); - name == (***attr).data.local_name && (***attr).data.namespace == namespace + name == (***attr).data.local_name && (***attr).data.namespace == *namespace }).map(|attr| { let attr: **Box = cast::transmute(attr); cast::transmute((**attr).data.value.as_slice()) diff --git a/src/components/style/node.rs b/src/components/style/node.rs index 455e1d1e93d..1d5b8355ed8 100644 --- a/src/components/style/node.rs +++ b/src/components/style/node.rs @@ -6,6 +6,7 @@ //! style. use selectors::AttrSelector; +use servo_util::namespace::Namespace; pub trait TNode : Clone { @@ -23,7 +24,7 @@ pub trait TNode : Clone { } pub trait TElement { - fn get_attr(&self, namespace: Option<~str>, attr: &str) -> Option<&'static str>; + fn get_attr(&self, namespace: &Namespace, attr: &str) -> Option<&'static str>; fn get_link(&self) -> Option<~str>; fn get_local_name<'a>(&'a self) -> &'a str; fn get_namespace_url<'a>(&'a self) -> &'a str; diff --git a/src/components/style/selector_matching.rs b/src/components/style/selector_matching.rs index a3984f8603a..57de671e761 100644 --- a/src/components/style/selector_matching.rs +++ b/src/components/style/selector_matching.rs @@ -7,6 +7,8 @@ use std::ascii::StrAsciiExt; use std::hashmap::HashMap; use std::str; +use servo_util::namespace; + use media_queries::{Device, Screen}; use node::{TElement, TNode}; use properties::{PropertyDeclaration, PropertyDeclarationBlock}; @@ -73,7 +75,7 @@ impl SelectorMap { // At the end, we're going to sort the rules that we added, so remember where we began. let init_len = matching_rules_list.len(); node.with_element(|element: &E| { - match element.get_attr(None, "id") { + match element.get_attr(&namespace::Null, "id") { Some(id) => { SelectorMap::get_matching_rules_from_hash(node, &self.id_hash, @@ -83,7 +85,7 @@ impl SelectorMap { None => {} } - match element.get_attr(None, "class") { + match element.get_attr(&namespace::Null, "class") { Some(ref class_attr) => { for class in class_attr.split(SELECTOR_WHITESPACE) { SelectorMap::get_matching_rules_from_hash( @@ -495,7 +497,7 @@ fn matches_simple_selector>(selector: &SimpleSelector, ele // TODO: cache and intern IDs on elements. IDSelector(ref id) => { element.with_element(|element: &E| { - match element.get_attr(None, "id") { + match element.get_attr(&namespace::Null, "id") { Some(attr) => str::eq_slice(attr, *id), None => false } @@ -504,7 +506,7 @@ fn matches_simple_selector>(selector: &SimpleSelector, ele // TODO: cache and intern classe names on elements. ClassSelector(ref class) => { element.with_element(|element: &E| { - match element.get_attr(None, "class") { + match element.get_attr(&namespace::Null, "class") { None => false, // TODO: case-sensitivity depends on the document type and quirks mode Some(ref class_attr)