From ae0cbda327b16e4f38b10e1913b04c3eaad0224e Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Sat, 25 Jan 2014 08:16:12 -0800 Subject: [PATCH 1/6] Change Namespace::from_str to take &str, fix #1367 --- src/components/main/layout/wrapper.rs | 3 ++- src/components/script/dom/element.rs | 8 ++++---- src/components/script/dom/namespace.rs | 7 +++---- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/components/main/layout/wrapper.rs b/src/components/main/layout/wrapper.rs index 34bd613e957..ca425072669 100644 --- a/src/components/main/layout/wrapper.rs +++ b/src/components/main/layout/wrapper.rs @@ -15,6 +15,7 @@ //! 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; @@ -405,7 +406,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(ns_url); + let namespace = Namespace::from_str(null_str_as_empty_ref(&ns_url)); unsafe { self.element.get_attr_val_for_layout(namespace, name) } } diff --git a/src/components/script/dom/element.rs b/src/components/script/dom/element.rs index 3d080a56de7..c2e790fd53d 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::NamespaceError; +use dom::bindings::utils::{null_str_as_empty_ref, NamespaceError}; use dom::bindings::utils::{InvalidCharacter, QName, Name, InvalidXMLName, xml_name_type}; use dom::htmlcollection::HTMLCollection; use dom::clientrect::ClientRect; @@ -402,7 +402,7 @@ impl Element { } pub fn GetAttributeNS(&self, namespace: Option, local_name: DOMString) -> Option { - let namespace = Namespace::from_str(namespace); + let namespace = Namespace::from_str(null_str_as_empty_ref(&namespace)); self.get_attribute(namespace, local_name) .map(|attr| attr.value.clone()) } @@ -430,7 +430,7 @@ impl Element { QName => {} } - let namespace = Namespace::from_str(namespace_url); + let namespace = Namespace::from_str(null_str_as_empty_ref(&namespace_url)); self.set_attribute(abstract_self, namespace, name, value) } @@ -449,7 +449,7 @@ impl Element { abstract_self: AbstractNode, namespace: Option, localname: DOMString) -> ErrorResult { - let namespace = Namespace::from_str(namespace); + let namespace = Namespace::from_str(null_str_as_empty_ref(&namespace)); self.remove_attribute(abstract_self, namespace, localname) } diff --git a/src/components/script/dom/namespace.rs b/src/components/script/dom/namespace.rs index c446b2969d6..354f95f69b2 100644 --- a/src/components/script/dom/namespace.rs +++ b/src/components/script/dom/namespace.rs @@ -2,8 +2,6 @@ * 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::bindings::utils::{DOMString, null_str_as_empty_ref}; - #[deriving(Eq, Clone)] pub enum Namespace { Null, @@ -17,8 +15,9 @@ pub enum Namespace { } impl Namespace { - pub fn from_str(url: Option) -> Namespace { - match null_str_as_empty_ref(&url) { + /// Empty string for "no namespace" + pub fn from_str(url: &str) -> Namespace { + match url { "http://www.w3.org/1999/xhtml" => HTML, "http://www.w3.org/XML/1998/namespace" => XML, "http://www.w3.org/2000/xmlns/" => XMLNS, From 624e2714d44d6d064aa507f8f6c806888a0f8ddc Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Sat, 25 Jan 2014 08:22:51 -0800 Subject: [PATCH 2/6] Move script::dom::namespace into util, in order to use it from style later. --- src/components/main/layout/wrapper.rs | 4 ++-- src/components/script/dom/attr.rs | 2 +- src/components/script/dom/document.rs | 2 +- src/components/script/dom/element.rs | 4 ++-- src/components/script/dom/htmldocument.rs | 2 +- src/components/script/dom/htmlelement.rs | 2 +- src/components/script/dom/htmlimageelement.rs | 2 +- src/components/script/dom/htmlserializer.rs | 2 +- src/components/script/html/hubbub_html_parser.rs | 2 +- src/components/script/script.rc | 1 - src/components/script/script_task.rs | 2 +- src/components/{script/dom => util}/namespace.rs | 0 src/components/util/util.rc | 2 +- 13 files changed, 13 insertions(+), 14 deletions(-) rename src/components/{script/dom => util}/namespace.rs (100%) diff --git a/src/components/main/layout/wrapper.rs b/src/components/main/layout/wrapper.rs index ca425072669..c0c4c0bd754 100644 --- a/src/components/main/layout/wrapper.rs +++ b/src/components/main/layout/wrapper.rs @@ -20,11 +20,11 @@ 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 servo_util::namespace; +use servo_util::namespace::Namespace; use std::cast; use style::{PropertyDeclarationBlock, TElement, TNode, AttrSelector}; diff --git a/src/components/script/dom/attr.rs b/src/components/script/dom/attr.rs index 3903e2d891d..e7e13853087 100644 --- a/src/components/script/dom/attr.rs +++ b/src/components/script/dom/attr.rs @@ -5,8 +5,8 @@ use dom::bindings::codegen::AttrBinding; use dom::bindings::utils::{Reflectable, Reflector, DOMString}; use dom::bindings::utils::reflect_dom_object; -use dom::namespace::{Namespace, Null}; use dom::window::Window; +use servo_util::namespace::{Namespace, Null}; use std::util; diff --git a/src/components/script/dom/document.rs b/src/components/script/dom/document.rs index 08979165375..092b3e63abe 100644 --- a/src/components/script/dom/document.rs +++ b/src/components/script/dom/document.rs @@ -16,7 +16,6 @@ use dom::event::{AbstractEvent, Event}; use dom::htmlcollection::HTMLCollection; use dom::htmldocument::HTMLDocument; use dom::mouseevent::MouseEvent; -use dom::namespace::Null; use dom::node::{AbstractNode, Node, ElementNodeTypeId, DocumentNodeTypeId}; use dom::text::Text; use dom::uievent::UIEvent; @@ -24,6 +23,7 @@ use dom::window::Window; use dom::htmltitleelement::HTMLTitleElement; use html::hubbub_html_parser::build_element_from_tag; use layout_interface::{DocumentDamageLevel, ContentChangedDocumentDamage}; +use servo_util::namespace::Null; use js::jsapi::{JSObject, JSContext, JSTracer}; use std::ascii::StrAsciiExt; diff --git a/src/components/script/dom/element.rs b/src/components/script/dom/element.rs index c2e790fd53d..f3c75db7a82 100644 --- a/src/components/script/dom/element.rs +++ b/src/components/script/dom/element.rs @@ -15,13 +15,13 @@ use dom::clientrectlist::ClientRectList; use dom::document::AbstractDocument; use dom::node::{AbstractNode, ElementNodeTypeId, Node, NodeIterator}; use dom::document; -use dom::namespace; -use dom::namespace::{Namespace, Null}; use dom::htmlserializer::serialize; use layout_interface::{ContentBoxQuery, ContentBoxResponse, ContentBoxesQuery}; use layout_interface::{ContentBoxesResponse, ContentChangedDocumentDamage}; use layout_interface::{MatchSelectorsDocumentDamage}; use style; +use servo_util::namespace; +use servo_util::namespace::{Namespace, Null}; use std::ascii::StrAsciiExt; use std::cast; diff --git a/src/components/script/dom/htmldocument.rs b/src/components/script/dom/htmldocument.rs index 144869392fe..0dad2905d89 100644 --- a/src/components/script/dom/htmldocument.rs +++ b/src/components/script/dom/htmldocument.rs @@ -6,8 +6,8 @@ use dom::bindings::codegen::HTMLDocumentBinding; use dom::bindings::utils::{Reflectable, Reflector, Traceable}; use dom::document::{AbstractDocument, Document, HTML}; use dom::htmlcollection::HTMLCollection; -use dom::namespace::Null; use dom::window::Window; +use servo_util::namespace::Null; use js::jsapi::JSTracer; use std::str::eq_slice; diff --git a/src/components/script/dom/htmlelement.rs b/src/components/script/dom/htmlelement.rs index f8703298926..19aecc6d947 100644 --- a/src/components/script/dom/htmlelement.rs +++ b/src/components/script/dom/htmlelement.rs @@ -9,7 +9,7 @@ use dom::element::{Element, ElementTypeId, HTMLElementTypeId}; use dom::node::{AbstractNode, Node}; use js::jsapi::{JSContext, JSVal}; use js::JSVAL_NULL; -use dom::namespace; +use servo_util::namespace; pub struct HTMLElement { element: Element diff --git a/src/components/script/dom/htmlimageelement.rs b/src/components/script/dom/htmlimageelement.rs index 2939de9e43a..dd0c5514472 100644 --- a/src/components/script/dom/htmlimageelement.rs +++ b/src/components/script/dom/htmlimageelement.rs @@ -7,7 +7,6 @@ use dom::bindings::utils::{DOMString, ErrorResult}; use dom::document::AbstractDocument; use dom::element::HTMLImageElementTypeId; use dom::htmlelement::HTMLElement; -use dom::namespace::Null; use dom::node::{AbstractNode, Node}; use extra::url::Url; use servo_util::geometry::to_px; @@ -15,6 +14,7 @@ use layout_interface::{ContentBoxQuery, ContentBoxResponse}; use servo_net::image_cache_task; use servo_net::image_cache_task::ImageCacheTask; use servo_util::url::make_url; +use servo_util::namespace::Null; pub struct HTMLImageElement { htmlelement: HTMLElement, diff --git a/src/components/script/dom/htmlserializer.rs b/src/components/script/dom/htmlserializer.rs index 3de36d831f2..8dbedcb2679 100644 --- a/src/components/script/dom/htmlserializer.rs +++ b/src/components/script/dom/htmlserializer.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::namespace; +use servo_util::namespace; use dom::attr::Attr; use dom::node::NodeIterator; use dom::node::{DoctypeNodeTypeId, DocumentFragmentNodeTypeId, CommentNodeTypeId, DocumentNodeTypeId, ElementNodeTypeId, TextNodeTypeId, AbstractNode}; diff --git a/src/components/script/html/hubbub_html_parser.rs b/src/components/script/html/hubbub_html_parser.rs index 7724dc80e30..d7179221bb5 100644 --- a/src/components/script/html/hubbub_html_parser.rs +++ b/src/components/script/html/hubbub_html_parser.rs @@ -8,7 +8,6 @@ use dom::htmlelement::HTMLElement; use dom::htmlheadingelement::{Heading1, Heading2, Heading3, Heading4, Heading5, Heading6}; use dom::htmliframeelement::IFrameSize; use dom::htmlformelement::HTMLFormElement; -use dom::namespace::Null; use dom::node::{AbstractNode, ElementNodeTypeId}; use dom::types::*; use html::cssparse::{InlineProvenance, StylesheetProvenance, UrlProvenance, spawn_css_parser}; @@ -22,6 +21,7 @@ use servo_net::image_cache_task::ImageCacheTask; use servo_net::resource_task::{Load, Payload, Done, ResourceTask, load_whole_resource}; use servo_util::url::make_url; use servo_util::task::spawn_named; +use servo_util::namespace::Null; use std::cast; use std::cell::RefCell; use std::comm::{Port, SharedChan}; diff --git a/src/components/script/script.rc b/src/components/script/script.rc index b7a642f4295..dd2eff22e95 100644 --- a/src/components/script/script.rc +++ b/src/components/script/script.rc @@ -136,7 +136,6 @@ pub mod dom { pub mod htmlunknownelement; pub mod location; pub mod mouseevent; - pub mod namespace; pub mod navigator; pub mod node; pub mod nodelist; diff --git a/src/components/script/script_task.rs b/src/components/script/script_task.rs index 362bdc9fc87..b226f05916b 100644 --- a/src/components/script/script_task.rs +++ b/src/components/script/script_task.rs @@ -13,7 +13,6 @@ use dom::event::{Event_, ResizeEvent, ReflowEvent, ClickEvent, MouseDownEvent, M use dom::event::Event; use dom::eventtarget::AbstractEventTarget; use dom::htmldocument::HTMLDocument; -use dom::namespace::Null; use dom::node::AbstractNode; use dom::window::{TimerData, TimerHandle, Window}; use html::hubbub_html_parser::HtmlParserResult; @@ -47,6 +46,7 @@ use servo_net::resource_task::ResourceTask; use servo_util::geometry::to_frac_px; use servo_util::url::make_url; use servo_util::task::spawn_named; +use servo_util::namespace::Null; use std::comm::{Port, SharedChan}; use std::ptr; use std::str::eq_slice; diff --git a/src/components/script/dom/namespace.rs b/src/components/util/namespace.rs similarity index 100% rename from src/components/script/dom/namespace.rs rename to src/components/util/namespace.rs diff --git a/src/components/util/util.rc b/src/components/util/util.rc index 1fb520c0c7b..8e7e8dfd2f2 100644 --- a/src/components/util/util.rc +++ b/src/components/util/util.rc @@ -21,4 +21,4 @@ pub mod debug; pub mod io; pub mod task; pub mod workqueue; - +pub mod namespace; From fdafc3701f9a72e3e16b0580b838b2c541a33d04 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Sat, 25 Jan 2014 09:02:20 -0800 Subject: [PATCH 3/6] 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) From e9ece24de9d85979ee9e74d6aa6a963b2f69d458 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Sat, 25 Jan 2014 09:17:54 -0800 Subject: [PATCH 4/6] Convert to Namespace during CSS parsing rather than selector matching. --- src/components/main/layout/wrapper.rs | 8 +++---- src/components/style/namespaces.rs | 21 ++++++++-------- src/components/style/node.rs | 2 +- src/components/style/selector_matching.rs | 6 ++--- src/components/style/selectors.rs | 29 +++++++++++++---------- 5 files changed, 36 insertions(+), 30 deletions(-) diff --git a/src/components/main/layout/wrapper.rs b/src/components/main/layout/wrapper.rs index 852696705d8..63205ed1da8 100644 --- a/src/components/main/layout/wrapper.rs +++ b/src/components/main/layout/wrapper.rs @@ -290,8 +290,8 @@ impl<'ln> TNode> for LayoutNode<'ln> { attr.name.as_slice() }; match attr.namespace { - Some(ref url) => { - match element.get_attr(&namespace::Namespace::from_str(url.as_slice()), name) { + Some(ref ns) => { + match element.get_attr(ns, name) { Some(value) => test(value), None => false, } @@ -404,8 +404,8 @@ impl<'le> TElement for LayoutElement<'le> { } #[inline] - fn get_namespace_url<'a>(&'a self) -> &'a str { - self.element.namespace.to_str().unwrap_or("") + fn get_namespace<'a>(&'a self) -> &'a Namespace { + &self.element.namespace } #[inline] diff --git a/src/components/style/namespaces.rs b/src/components/style/namespaces.rs index 77ae126fcd3..762b5f7ab0a 100644 --- a/src/components/style/namespaces.rs +++ b/src/components/style/namespaces.rs @@ -4,11 +4,12 @@ use std::hashmap::HashMap; use cssparser::ast::*; +use servo_util::namespace::Namespace; use errors::log_css_error; pub struct NamespaceMap { - default: Option<~str>, // Optional URL - prefix_map: HashMap<~str, ~str>, // prefix -> URL + default: Option, + prefix_map: HashMap<~str, Namespace>, } @@ -29,7 +30,7 @@ pub fn parse_namespace_rule(rule: AtRule, namespaces: &mut NamespaceMap) { ); if rule.block.is_some() { syntax_error!() } let mut prefix: Option<~str> = None; - let mut url: Option<~str> = None; + let mut ns: Option = None; let mut iter = rule.prelude.move_skip_whitespace(); for component_value in iter { match component_value { @@ -38,25 +39,25 @@ pub fn parse_namespace_rule(rule: AtRule, namespaces: &mut NamespaceMap) { prefix = Some(value); }, URL(value) | String(value) => { - if url.is_some() { syntax_error!() } - url = Some(value); + if ns.is_some() { syntax_error!() } + ns = Some(Namespace::from_str(value.as_slice())); break }, _ => syntax_error!(), } } if iter.next().is_some() { syntax_error!() } - match (prefix, url) { - (Some(prefix), Some(url)) => { - if namespaces.prefix_map.swap(prefix, url).is_some() { + match (prefix, ns) { + (Some(prefix), Some(ns)) => { + if namespaces.prefix_map.swap(prefix, ns).is_some() { log_css_error(location, "Duplicate @namespace rule"); } }, - (None, Some(url)) => { + (None, Some(ns)) => { if namespaces.default.is_some() { log_css_error(location, "Duplicate @namespace rule"); } - namespaces.default = Some(url); + namespaces.default = Some(ns); }, _ => syntax_error!() } diff --git a/src/components/style/node.rs b/src/components/style/node.rs index 1d5b8355ed8..2e22ac0cb89 100644 --- a/src/components/style/node.rs +++ b/src/components/style/node.rs @@ -27,6 +27,6 @@ pub trait TElement { 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; + fn get_namespace<'a>(&'a self) -> &'a Namespace; } diff --git a/src/components/style/selector_matching.rs b/src/components/style/selector_matching.rs index 57de671e761..a5f6b9cd3f9 100644 --- a/src/components/style/selector_matching.rs +++ b/src/components/style/selector_matching.rs @@ -488,9 +488,9 @@ fn matches_simple_selector>(selector: &SimpleSelector, ele element.get_local_name().eq_ignore_ascii_case(name.as_slice()) }) } - NamespaceSelector(ref url) => { + NamespaceSelector(ref namespace) => { element.with_element(|element: &E| { - element.get_namespace_url() == url.as_slice() + element.get_namespace() == namespace }) } // TODO: case-sensitivity depends on the document type and quirks mode @@ -626,7 +626,7 @@ fn matches_generic_nth_child<'a, element.with_element(|element: &E| { node.with_element(|node: &E| { if element.get_local_name() == node.get_local_name() && - element.get_namespace_url() == node.get_namespace_url() { + element.get_namespace() == node.get_namespace() { index += 1; } }) diff --git a/src/components/style/selectors.rs b/src/components/style/selectors.rs index 24b2d0e4b20..c15259917a4 100644 --- a/src/components/style/selectors.rs +++ b/src/components/style/selectors.rs @@ -9,6 +9,9 @@ use extra::arc::Arc; use cssparser::ast::*; use cssparser::parse_nth; +use servo_util::namespace::Namespace; +use servo_util::namespace; + use namespaces::NamespaceMap; @@ -55,7 +58,7 @@ pub enum SimpleSelector { IDSelector(~str), ClassSelector(~str), LocalNameSelector(~str), - NamespaceSelector(~str), + NamespaceSelector(Namespace), // Attribute selectors AttrExists(AttrSelector), // [foo] @@ -89,7 +92,8 @@ pub enum SimpleSelector { pub struct AttrSelector { name: ~str, lower_name: ~str, - namespace: Option<~str>, + /// None means "any namespace", `*|attr` + namespace: Option, } @@ -270,7 +274,7 @@ fn parse_type_selector(iter: &mut Iter, namespaces: &NamespaceMap) QualifiedName(namespace, local_name) => { let mut simple_selectors = ~[]; match namespace { - Some(url) => simple_selectors.push(NamespaceSelector(url)), + Some(ns) => simple_selectors.push(NamespaceSelector(ns)), None => (), } match local_name { @@ -357,7 +361,8 @@ fn parse_one_simple_selector(iter: &mut Iter, namespaces: &NamespaceMap, inside_ enum QualifiedNameParseResult { InvalidQualifiedName, NotAQualifiedName, - QualifiedName(Option<~str>, Option<~str>) // Namespace URL, local name. None means '*' + // Namespace URL, local name. None means '*' + QualifiedName(Option, Option<~str>) } fn parse_qualified_name(iter: &mut Iter, allow_universal: bool, namespaces: &NamespaceMap) @@ -365,22 +370,22 @@ fn parse_qualified_name(iter: &mut Iter, allow_universal: bool, namespaces: &Nam #[inline] fn default_namespace(namespaces: &NamespaceMap, local_name: Option<~str>) -> QualifiedNameParseResult { - QualifiedName(namespaces.default.as_ref().map(|url| url.to_owned()), local_name) + QualifiedName(namespaces.default.as_ref().map(|ns| ns.clone()), local_name) } #[inline] - fn explicit_namespace(iter: &mut Iter, allow_universal: bool, namespace_url: Option<~str>) + fn explicit_namespace(iter: &mut Iter, allow_universal: bool, namespace: Option) -> QualifiedNameParseResult { assert!(iter.next() == Some(Delim('|')), "Implementation error, this should not happen."); match iter.peek() { Some(&Delim('*')) if allow_universal => { iter.next(); - QualifiedName(namespace_url, None) + QualifiedName(namespace, None) }, Some(&Ident(_)) => { let local_name = get_next_ident(iter); - QualifiedName(namespace_url, Some(local_name)) + QualifiedName(namespace, Some(local_name)) }, _ => InvalidQualifiedName, } @@ -391,11 +396,11 @@ fn parse_qualified_name(iter: &mut Iter, allow_universal: bool, namespaces: &Nam let value = get_next_ident(iter); match iter.peek() { Some(&Delim('|')) => { - let namespace_url = match namespaces.prefix_map.find(&value) { + let namespace = match namespaces.prefix_map.find(&value) { None => return InvalidQualifiedName, // Undeclared namespace prefix - Some(ref url) => url.to_owned(), + Some(ref ns) => (*ns).clone(), }; - explicit_namespace(iter, allow_universal, Some(namespace_url)) + explicit_namespace(iter, allow_universal, Some(namespace)) }, _ => default_namespace(namespaces, Some(value)), } @@ -410,7 +415,7 @@ fn parse_qualified_name(iter: &mut Iter, allow_universal: bool, namespaces: &Nam }, } }, - Some(&Delim('|')) => explicit_namespace(iter, allow_universal, Some(~"")), + Some(&Delim('|')) => explicit_namespace(iter, allow_universal, Some(namespace::Null)), _ => NotAQualifiedName, } } From 5ae7aad6e2054e283b6219da9775fe4472379a26 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Sat, 25 Jan 2014 09:31:41 -0800 Subject: [PATCH 5/6] Make Namespace::to_str() return a string. --- src/components/script/dom/attr.rs | 5 ++++- src/components/util/namespace.rs | 18 +++++++++--------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/components/script/dom/attr.rs b/src/components/script/dom/attr.rs index e7e13853087..9ecf068774e 100644 --- a/src/components/script/dom/attr.rs +++ b/src/components/script/dom/attr.rs @@ -89,7 +89,10 @@ impl Attr { } pub fn GetNamespaceURI(&self) -> Option { - self.namespace.to_str().map(|s| s.to_owned()) + match self.namespace.to_str() { + "" => None, + url => Some(url.to_owned()), + } } pub fn GetPrefix(&self) -> Option { diff --git a/src/components/util/namespace.rs b/src/components/util/namespace.rs index 354f95f69b2..ff9eda169c9 100644 --- a/src/components/util/namespace.rs +++ b/src/components/util/namespace.rs @@ -28,16 +28,16 @@ impl Namespace { ns => Other(ns.to_owned()) } } - pub fn to_str<'a>(&'a self) -> Option<&'a str> { + pub fn to_str<'a>(&'a self) -> &'a str { match *self { - Null => None, - HTML => Some("http://www.w3.org/1999/xhtml"), - XML => Some("http://www.w3.org/XML/1998/namespace"), - XMLNS => Some("http://www.w3.org/2000/xmlns/"), - XLink => Some("http://www.w3.org/1999/xlink"), - SVG => Some("http://www.w3.org/2000/svg"), - MathML => Some("http://www.w3.org/1998/Math/MathML"), - Other(ref x) => Some(x.as_slice()) + Null => "", + HTML => "http://www.w3.org/1999/xhtml", + XML => "http://www.w3.org/XML/1998/namespace", + XMLNS => "http://www.w3.org/2000/xmlns/", + XLink => "http://www.w3.org/1999/xlink", + SVG => "http://www.w3.org/2000/svg", + MathML => "http://www.w3.org/1998/Math/MathML", + Other(ref x) => x.as_slice() } } } From 2782d1d07fcc68cacb488875aa07d20f8a7c6ef1 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Sat, 25 Jan 2014 09:48:21 -0800 Subject: [PATCH 6/6] Do not copy the result of get_link() --- src/components/main/layout/wrapper.rs | 3 +-- src/components/style/namespaces.rs | 2 +- src/components/style/node.rs | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/components/main/layout/wrapper.rs b/src/components/main/layout/wrapper.rs index 63205ed1da8..75bc94e5b85 100644 --- a/src/components/main/layout/wrapper.rs +++ b/src/components/main/layout/wrapper.rs @@ -413,7 +413,7 @@ impl<'le> TElement for LayoutElement<'le> { unsafe { self.element.get_attr_val_for_layout(namespace, name) } } - fn get_link(&self) -> Option<~str> { + fn get_link(&self) -> Option<&'static str> { // FIXME: This is HTML only. match self.element.node.type_id { // http://www.whatwg.org/specs/web-apps/current-work/multipage/selectors.html# @@ -422,7 +422,6 @@ impl<'le> TElement for LayoutElement<'le> { ElementNodeTypeId(HTMLAreaElementTypeId) | ElementNodeTypeId(HTMLLinkElementTypeId) => { unsafe { self.element.get_attr_val_for_layout(&namespace::Null, "href") } - .map(|val| val.to_owned()) } _ => None, } diff --git a/src/components/style/namespaces.rs b/src/components/style/namespaces.rs index 762b5f7ab0a..a7b2f79deac 100644 --- a/src/components/style/namespaces.rs +++ b/src/components/style/namespaces.rs @@ -40,7 +40,7 @@ pub fn parse_namespace_rule(rule: AtRule, namespaces: &mut NamespaceMap) { }, URL(value) | String(value) => { if ns.is_some() { syntax_error!() } - ns = Some(Namespace::from_str(value.as_slice())); + ns = Some(Namespace::from_str(value)); break }, _ => syntax_error!(), diff --git a/src/components/style/node.rs b/src/components/style/node.rs index 2e22ac0cb89..dc6df8427bd 100644 --- a/src/components/style/node.rs +++ b/src/components/style/node.rs @@ -25,7 +25,7 @@ pub trait TNode : Clone { pub trait TElement { fn get_attr(&self, namespace: &Namespace, attr: &str) -> Option<&'static str>; - fn get_link(&self) -> Option<~str>; + fn get_link(&self) -> Option<&'static str>; fn get_local_name<'a>(&'a self) -> &'a str; fn get_namespace<'a>(&'a self) -> &'a Namespace; }