Merge pull request #3078 from SimonSapin/style-cleanup

Various refactoring and fixes in the style crate

Makefile.in

@@ -348,7 +348,7 @@ RFLAGS_style = $(addprefix -L $(B)src/,$(DEPS_SUBMODULES)) -L $(B)src/components
 MAKO_ZIP = $(S)src/components/style/Mako-0.9.1.zip
 MAKO_style = $(S)src/components/style/properties/mod.rs
 MAKO_SRC_style = $(MAKO_style).mako
-SRC_style = $(call rwildcard,$(S)src/components/style/,*.rs) $(call rwildcard,$(S)src/compontents/style/properties/*.rs) $(MAKO_style)
+SRC_style = $(call rwildcard,$(S)src/components/style/,*.rs) $(call rwildcard,$(S)src/compontents/style/properties/*.rs) $(MAKO_style) $(S)src/components/style/user-agent.css
 CRATE_style = $(S)src/components/style/style.rs
 DONE_style = $(B)src/components/style/libstyle.dummy
 
@@ -364,7 +364,7 @@ DEPS_layout_traits = $(CRATE_layout_traits) $(SRC_layout_traits) $(DONE_script_t
 
 RFLAGS_layout = $(addprefix -L $(B)src/,$(DEPS_SUBMODULES)) -L $(B)src/components/gfx -L $(B)src/components/util -L $(B)src/components/net -L $(B)src/components/script -L $(B)src/components/style -L $(B)src/components/msg -L$(B)src/components/macros -L$(B)src/components/layout_traits -L $(B)src/components/script_traits
 
-SRC_layout = $(call rwildcard,$(S)src/components/layout/,*.rs) $(S)src/components/layout/css/user-agent.css
+SRC_layout = $(call rwildcard,$(S)src/components/layout/,*.rs)
 CRATE_layout = $(S)src/components/layout/layout.rs
 DONE_layout = $(B)src/components/layout/liblayout.dummy
 

mk/check.mk

@@ -29,7 +29,7 @@ servo-test-$(1): $$(DEPS_$(1))
 .PHONY: check-servo-$(1)
 check-servo-$(1): servo-test-$(1)
 	@$$(call E, check: $(1))
-	$$(Q)./servo-test-$(1)
+	$$(Q)./servo-test-$(1) $(TESTNAME)
 endef
 
 $(foreach lib_crate,$(SERVO_LIB_CRATES),\

src/components/gfx/font_cache_task.rs

@@ -70,7 +70,7 @@ impl FontFamily {
 /// Commands that the FontContext sends to the font cache task.
 pub enum Command {
     GetFontTemplate(String, FontTemplateDescriptor, Sender<Reply>),
-    AddWebFont(Vec<Url>, String, Sender<()>),
+    AddWebFont(String, Url, Sender<()>),
     Exit(Sender<()>),
 }
 
@@ -105,8 +105,7 @@ impl FontCache {
 
                     result.send(GetFontTemplateReply(font_template));
                 }
-                AddWebFont(urls, family_name, result) => {
+                AddWebFont(family_name, url, result) => {
-                    for url in urls.iter() {
                     let maybe_resource = load_whole_resource(&self.resource_task, url.clone());
                     match maybe_resource {
                         Ok((_, bytes)) => {
@@ -121,7 +120,6 @@ impl FontCache {
                             fail!("{}: url={}", msg, url);
                         }
                     }
-                    }
                     result.send(());
                 }
                 Exit(result) => {
@@ -264,9 +262,9 @@ impl FontCacheTask {
         }
     }
 
-    pub fn add_web_font(&mut self, urls: Vec<Url>, family: &str) {
+    pub fn add_web_font(&mut self, family: String, url: Url) {
         let (response_chan, response_port) = channel();
-        self.chan.send(AddWebFont(urls, family.to_string(), response_chan));
+        self.chan.send(AddWebFont(family, url, response_chan));
         response_port.recv();
     }
 

src/components/layout/css/matching.rs

@@ -19,13 +19,13 @@ use servo_util::str::DOMString;
 use std::mem;
 use std::hash::{Hash, sip};
 use std::slice::Items;
-use style::{After, Before, ComputedValues, MatchedProperty, Stylist, TElement, TNode, cascade};
+use style::{After, Before, ComputedValues, DeclarationBlock, Stylist, TElement, TNode, cascade};
 use sync::Arc;
 
 pub struct ApplicableDeclarations {
-    pub normal: SmallVec16<MatchedProperty>,
+    pub normal: SmallVec16<DeclarationBlock>,
-    pub before: Vec<MatchedProperty>,
+    pub before: Vec<DeclarationBlock>,
-    pub after: Vec<MatchedProperty>,
+    pub after: Vec<DeclarationBlock>,
 
     /// Whether the `normal` declarations are shareable with other nodes.
     pub normal_shareable: bool,
@@ -51,11 +51,11 @@ impl ApplicableDeclarations {
 
 #[deriving(Clone)]
 pub struct ApplicableDeclarationsCacheEntry {
-    pub declarations: Vec<MatchedProperty>,
+    pub declarations: Vec<DeclarationBlock>,
 }
 
 impl ApplicableDeclarationsCacheEntry {
-    fn new(slice: &[MatchedProperty]) -> ApplicableDeclarationsCacheEntry {
+    fn new(slice: &[DeclarationBlock]) -> ApplicableDeclarationsCacheEntry {
         let mut entry_declarations = Vec::new();
         for declarations in slice.iter() {
             entry_declarations.push(declarations.clone());
@@ -81,11 +81,11 @@ impl Hash for ApplicableDeclarationsCacheEntry {
 }
 
 struct ApplicableDeclarationsCacheQuery<'a> {
-    declarations: &'a [MatchedProperty],
+    declarations: &'a [DeclarationBlock],
 }
 
 impl<'a> ApplicableDeclarationsCacheQuery<'a> {
-    fn new(declarations: &'a [MatchedProperty]) -> ApplicableDeclarationsCacheQuery<'a> {
+    fn new(declarations: &'a [DeclarationBlock]) -> ApplicableDeclarationsCacheQuery<'a> {
         ApplicableDeclarationsCacheQuery {
             declarations: declarations,
         }
@@ -141,14 +141,14 @@ impl ApplicableDeclarationsCache {
         }
     }
 
-    fn find(&self, declarations: &[MatchedProperty]) -> Option<Arc<ComputedValues>> {
+    fn find(&self, declarations: &[DeclarationBlock]) -> Option<Arc<ComputedValues>> {
         match self.cache.find_equiv(&ApplicableDeclarationsCacheQuery::new(declarations)) {
             None => None,
             Some(ref values) => Some((*values).clone()),
         }
     }
 
-    fn insert(&mut self, declarations: &[MatchedProperty], style: Arc<ComputedValues>) {
+    fn insert(&mut self, declarations: &[DeclarationBlock], style: Arc<ComputedValues>) {
         self.cache.insert(ApplicableDeclarationsCacheEntry::new(declarations), style)
     }
 }
@@ -309,7 +309,7 @@ pub trait MatchMethods {
 trait PrivateMatchMethods {
     fn cascade_node_pseudo_element(&self,
                                    parent_style: Option<&Arc<ComputedValues>>,
-                                   applicable_declarations: &[MatchedProperty],
+                                   applicable_declarations: &[DeclarationBlock],
                                    style: &mut Option<Arc<ComputedValues>>,
                                    applicable_declarations_cache: &mut
                                    ApplicableDeclarationsCache,
@@ -324,7 +324,7 @@ trait PrivateMatchMethods {
 impl<'ln> PrivateMatchMethods for LayoutNode<'ln> {
     fn cascade_node_pseudo_element(&self,
                                    parent_style: Option<&Arc<ComputedValues>>,
-                                   applicable_declarations: &[MatchedProperty],
+                                   applicable_declarations: &[DeclarationBlock],
                                    style: &mut Option<Arc<ComputedValues>>,
                                    applicable_declarations_cache: &mut
                                    ApplicableDeclarationsCache,

src/components/layout/css/select.rs

@@ -1,18 +0,0 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * 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 style::{Stylesheet, Stylist, UserAgentOrigin, with_errors_silenced};
-use url::Url;
-
-
-pub fn new_stylist() -> Stylist {
-    let mut stylist = Stylist::new();
-    let ua_stylesheet = with_errors_silenced(|| Stylesheet::from_bytes(
-        include_bin!("user-agent.css"),
-        Url::parse("chrome:///user-agent.css").unwrap(),
-        None,
-        None));
-    stylist.add_stylesheet(ua_stylesheet, UserAgentOrigin);
-    stylist
-}

src/components/layout/layout.rs

@@ -62,7 +62,6 @@ pub mod extra;
 pub mod css {
     mod node_util;
 
-    pub mod select;
     pub mod matching;
     pub mod node_style;
 }

src/components/layout/layout_task.rs

@@ -6,7 +6,6 @@
 //! rendered.
 
 use css::matching::{ApplicableDeclarations, MatchMethods};
-use css::select::new_stylist;
 use css::node_style::StyledNode;
 use construct::{FlowConstructionResult, NoConstructionResult};
 use context::{LayoutContext, SharedLayoutContext};
@@ -57,7 +56,7 @@ use std::comm::{channel, Sender, Receiver, Select};
 use std::mem;
 use std::ptr;
 use style::{AuthorOrigin, Stylesheet, Stylist};
-use style::CSSFontFaceRule;
+use style::iter_font_face_rules;
 use sync::{Arc, Mutex};
 use url::Url;
 
@@ -347,7 +346,7 @@ impl LayoutTask {
             screen_size: screen_size,
 
             display_list: None,
-            stylist: box new_stylist(),
+            stylist: box Stylist::new(),
             parallel_traversal: parallel_traversal,
             time_profiler_chan: time_profiler_chan,
             opts: opts.clone(),
@@ -491,22 +490,9 @@ impl LayoutTask {
     fn handle_add_stylesheet(&mut self, sheet: Stylesheet) {
         // Find all font-face rules and notify the font cache of them.
         // GWTODO: Need to handle unloading web fonts (when we handle unloading stylesheets!)
-        // GWTODO: Need to handle font-face nested within media rules.
+        iter_font_face_rules(&sheet, |family, url| {
-        for rule in sheet.rules.iter() {
+            self.font_cache_task.add_web_font(family.to_string(), url.clone());
-            match rule {
+        });
-                &CSSFontFaceRule(ref font_face_rule) => {
-                    let mut font_urls = vec!();
-                    for source_line in font_face_rule.source_lines.iter() {
-                        for source in source_line.sources.iter() {
-                            font_urls.push(source.url.clone());
-                        }
-                    }
-                    self.font_cache_task.add_web_font(font_urls, font_face_rule.family.as_slice());
-                },
-                _ => {}
-            }
-        }
-
         self.stylist.add_stylesheet(sheet, AuthorOrigin);
     }
 

src/components/layout/wrapper.rs

@@ -265,13 +265,11 @@ impl<'ln> TNode<LayoutElement<'ln>> for LayoutNode<'ln> {
     }
 
     fn match_attr(&self, attr: &AttrSelector, test: |&str| -> bool) -> bool {
-        let name = unsafe {
+        assert!(self.is_element())
-            let element: JS<Element> = self.node.transmute_copy();
+        let name = if self.is_html_element_in_html_document() {
-            if element.html_element_in_html_document_for_layout() {
             attr.lower_name.as_slice()
         } else {
             attr.name.as_slice()
-            }
         };
         match attr.namespace {
             SpecificNamespace(ref ns) => {
@@ -283,6 +281,15 @@ impl<'ln> TNode<LayoutElement<'ln>> for LayoutNode<'ln> {
             AnyNamespace => false,
         }
     }
+
+    fn is_html_element_in_html_document(&self) -> bool {
+        unsafe {
+            self.is_element() && {
+                let element: JS<Element> = self.node.transmute_copy();
+                element.html_element_in_html_document_for_layout()
+            }
+        }
+    }
 }
 
 pub struct LayoutNodeChildrenIterator<'a> {

src/components/script/dom/element.rs

@@ -4,7 +4,6 @@
 
 //! Element nodes.
 
-use cssparser::tokenize;
 use dom::attr::{Attr, ReplacedAttr, FirstSetAttr, AttrHelpersForLayout};
 use dom::attr::{AttrValue, StringAttrValue, UIntAttrValue, AtomAttrValue};
 use dom::attrlist::AttrList;
@@ -31,7 +30,7 @@ use dom::nodelist::NodeList;
 use dom::virtualmethods::{VirtualMethods, vtable_for};
 use layout_interface::ContentChangedDocumentDamage;
 use layout_interface::MatchSelectorsDocumentDamage;
-use style::{matches_compound_selector, NamespaceMap, parse_selector_list};
+use style::{matches, parse_selector_list_from_str};
 use style;
 use servo_util::atom::Atom;
 use servo_util::namespace;
@@ -775,23 +774,15 @@ impl<'a> ElementMethods for JSRef<'a, Element> {
 
     // http://dom.spec.whatwg.org/#dom-element-matches
     fn Matches(&self, selectors: DOMString) -> Fallible<bool> {
-        let namespace = NamespaceMap::new();
+        match parse_selector_list_from_str(selectors.as_slice()) {
-        match parse_selector_list(tokenize(selectors.as_slice()).map(|(token, _)| token).collect(),
+            Err(()) => Err(Syntax),
-                                  &namespace) {
+            Ok(ref selectors) => {
-            None => return Err(Syntax),
-            Some(ref selectors) => {
                 let root: &JSRef<Node> = NodeCast::from_ref(self);
-                for selector in selectors.iter() {
+                Ok(matches(selectors, root))
-                    let mut shareable = false;
-                    if matches_compound_selector(&*selector.compound_selectors, root, &mut shareable) {
-                        return Ok(true);
             }
         }
     }
 }
-        Ok(false)
-    }
-}
 
 pub fn get_attribute_parts<'a>(name: &'a str) -> (Option<&'a str>, &'a str) {
     //FIXME: Throw for XML-invalid names

src/components/script/dom/node.rs

@@ -4,7 +4,6 @@
 
 //! The core DOM types. Defines the basic DOM hierarchy as well as all the HTML elements.
 
-use cssparser::tokenize;
 use dom::attr::Attr;
 use dom::bindings::codegen::Bindings::AttrBinding::AttrMethods;
 use dom::bindings::codegen::Bindings::CharacterDataBinding::CharacterDataMethods;
@@ -48,7 +47,7 @@ use layout_interface::{ContentBoxQuery, ContentBoxResponse, ContentBoxesQuery, C
                        LayoutChan, ReapLayoutDataMsg, TrustedNodeAddress, UntrustedNodeAddress};
 use servo_util::geometry::Au;
 use servo_util::str::{DOMString, null_str_as_empty};
-use style::{parse_selector_list, matches_compound_selector, NamespaceMap};
+use style::{parse_selector_list_from_str, matches};
 
 use js::jsapi::{JSContext, JSObject, JSRuntime};
 use js::jsfriendapi;
@@ -611,48 +610,35 @@ impl<'m, 'n> NodeHelpers<'m, 'n> for JSRef<'n, Node> {
     // http://dom.spec.whatwg.org/#dom-parentnode-queryselector
     fn query_selector(&self, selectors: DOMString) -> Fallible<Option<Temporary<Element>>> {
         // Step 1.
-        let namespace = NamespaceMap::new();
+        match parse_selector_list_from_str(selectors.as_slice()) {
-        match parse_selector_list(tokenize(selectors.as_slice()).map(|(token, _)| token).collect(), &namespace) {
             // Step 2.
-            None => return Err(Syntax),
+            Err(()) => return Err(Syntax),
             // Step 3.
-            Some(ref selectors) => {
+            Ok(ref selectors) => {
                 let root = self.ancestors().last().unwrap_or(self.clone());
-                for selector in selectors.iter() {
+                for node in root.traverse_preorder() {
-                    assert!(selector.pseudo_element.is_none());
+                    if node.is_element() && matches(selectors, &node) {
-                    for node in root.traverse_preorder().filter(|node| node.is_element()) {
-                        let mut _shareable: bool = false;
-                        if matches_compound_selector(selector.compound_selectors.deref(), &node, &mut _shareable) {
                         let elem: &JSRef<Element> = ElementCast::to_ref(&node).unwrap();
                         return Ok(Some(Temporary::from_rooted(elem)));
                     }
                 }
             }
         }
-        }
         Ok(None)
     }
 
     // http://dom.spec.whatwg.org/#dom-parentnode-queryselectorall
     fn query_selector_all(&self, selectors: DOMString) -> Fallible<Temporary<NodeList>> {
         // Step 1.
-        let mut nodes = vec!();
+        let nodes;
         let root = self.ancestors().last().unwrap_or(self.clone());
-        let namespace = NamespaceMap::new();
+        match parse_selector_list_from_str(selectors.as_slice()) {
-        match parse_selector_list(tokenize(selectors.as_slice()).map(|(token, _)| token).collect(), &namespace) {
             // Step 2.
-            None => return Err(Syntax),
+            Err(()) => return Err(Syntax),
             // Step 3.
-            Some(ref selectors) => {
+            Ok(ref selectors) => {
-                for selector in selectors.iter() {
+                nodes = root.traverse_preorder().filter(
-                    assert!(selector.pseudo_element.is_none());
+                    |node| node.is_element() && matches(selectors, node)).collect()
-                    for node in root.traverse_preorder().filter(|node| node.is_element()) {
-                        let mut _shareable: bool = false;
-                        if matches_compound_selector(selector.compound_selectors.deref(), &node, &mut _shareable) {
-                            nodes.push(node.clone())
-                        }
-                    }
-                }
             }
         }
         let window = window_from_node(self).root();
@@ -1997,29 +1983,32 @@ impl<'a> style::TNode<JSRef<'a, Element>> for JSRef<'a, Node> {
     fn parent_node(&self) -> Option<JSRef<'a, Node>> {
         (self as &NodeHelpers).parent_node().map(|node| *node.root())
     }
+
     fn prev_sibling(&self) -> Option<JSRef<'a, Node>> {
         (self as &NodeHelpers).prev_sibling().map(|node| *node.root())
     }
+
     fn next_sibling(&self) -> Option<JSRef<'a, Node>> {
         (self as &NodeHelpers).next_sibling().map(|node| *node.root())
     }
+
     fn is_document(&self) -> bool {
         (self as &NodeHelpers).is_document()
     }
+
     fn is_element(&self) -> bool {
         (self as &NodeHelpers).is_element()
     }
+
     fn as_element(&self) -> JSRef<'a, Element> {
         let elem: Option<&JSRef<'a, Element>> = ElementCast::to_ref(self);
         assert!(elem.is_some());
         *elem.unwrap()
     }
+
     fn match_attr(&self, attr: &style::AttrSelector, test: |&str| -> bool) -> bool {
         let name = {
-            let elem: Option<&JSRef<'a, Element>> = ElementCast::to_ref(self);
+            if self.is_html_element_in_html_document() {
-            assert!(elem.is_some());
-            let elem: &ElementHelpers = elem.unwrap() as &ElementHelpers;
-            if elem.html_element_in_html_document() {
                 attr.lower_name.as_slice()
             } else {
                 attr.name.as_slice()
@@ -2034,6 +2023,13 @@ impl<'a> style::TNode<JSRef<'a, Element>> for JSRef<'a, Node> {
             style::AnyNamespace => false,
         }
     }
+
+    fn is_html_element_in_html_document(&self) -> bool {
+        let elem: Option<&JSRef<'a, Element>> = ElementCast::to_ref(self);
+        assert!(elem.is_some());
+        let elem: &ElementHelpers = elem.unwrap() as &ElementHelpers;
+        elem.html_element_in_html_document()
+    }
 }
 
 pub trait DisabledStateHelpers {

src/components/style/errors.rs

@@ -27,21 +27,6 @@ impl<T, I: Iterator<Result<T, SyntaxError>>> Iterator<T> for ErrorLoggerIterator
 /// Set a `RUST_LOG=style::errors` environment variable
 /// to log CSS parse errors to stderr.
 pub fn log_css_error(location: SourceLocation, message: &str) {
-    // Check this first as it’s cheaper than local_data.
-    if log_enabled!(::log::INFO) {
-        if silence_errors.get().is_none() {
     // TODO eventually this will got into a "web console" or something.
     info!("{:u}:{:u} {:s}", location.line, location.column, message)
 }
-    }
-}
-
-
-local_data_key!(silence_errors: ())
-
-pub fn with_errors_silenced<T>(f: || -> T) -> T {
-    silence_errors.replace(Some(()));
-    let result = f();
-    silence_errors.replace(None);
-    result
-}

src/components/style/font_face.rs

@@ -6,37 +6,54 @@ use cssparser::ast::*;
 use cssparser::parse_declaration_list;
 use errors::{ErrorLoggerIterator, log_css_error};
 use std::ascii::StrAsciiExt;
-use parsing_utils::one_component_value;
+use parsing_utils::{BufferedIter, ParserIter, parse_slice_comma_separated};
-use stylesheets::{CSSRule, CSSFontFaceRule};
+use properties::longhands::font_family::parse_one_family;
+use properties::computed_values::font_family::FamilyName;
+use stylesheets::{CSSRule, CSSFontFaceRule, CSSStyleRule, CSSMediaRule};
+use media_queries::{Device, Screen};
 use url::{Url, UrlParser};
 
-#[deriving(PartialEq)]
+
-pub enum FontFaceFormat {
+static SUPPORTED_FORMATS: &'static [&'static str] = &["truetype", "opentype"];
-    UnknownFormat,
+
-    WoffFormat,
+
-    TtfFormat,
+pub fn iter_font_face_rules_inner(rules: &[CSSRule], callback: |family: &str, source: &Url|) {
-    SvgFormat,
+    let device = &Device { media_type: Screen };  // TODO, use Print when printing
-    EotFormat,
+    for rule in rules.iter() {
+        match *rule {
+            CSSStyleRule(_) => {},
+            CSSMediaRule(ref rule) => if rule.media_queries.evaluate(device) {
+                iter_font_face_rules_inner(rule.rules.as_slice(), |f, s| callback(f, s))
+            },
+            CSSFontFaceRule(ref rule) => {
+                for source in rule.sources.iter() {
+                    if source.format_hints.is_empty() || source.format_hints.iter().any(
+                            |f| SUPPORTED_FORMATS.iter().any(
+                                |s| f.as_slice().eq_ignore_ascii_case(*s))) {
+                        callback(rule.family.as_slice(), &source.url)
+                    }
+                }
+            },
+        }
+    }
 }
 
-pub struct FontFaceSource {
+enum Source {
+    UrlSource(UrlSource),
+    LocalSource(String),
+}
+
+pub struct UrlSource {
     pub url: Url,
-    pub format_hints: Vec<FontFaceFormat>,
+    pub format_hints: Vec<String>,
-}
-
-pub struct FontFaceSourceLine {
-    pub sources: Vec<FontFaceSource>
 }
 
 pub struct FontFaceRule {
     pub family: String,
-    pub source_lines: Vec<FontFaceSourceLine>,
+    pub sources: Vec<UrlSource>,  // local() is not supported yet
 }
 
 pub fn parse_font_face_rule(rule: AtRule, parent_rules: &mut Vec<CSSRule>, base_url: &Url) {
-    let mut maybe_family = None;
-    let mut source_lines = vec!();
-
     if rule.prelude.as_slice().skip_whitespace().next().is_some() {
         log_css_error(rule.location, "@font-face prelude contains unexpected characters");
         return;
@@ -50,144 +67,36 @@ pub fn parse_font_face_rule(rule: AtRule, parent_rules: &mut Vec<CSSRule>, base_
         }
     };
 
+    let mut maybe_family = None;
+    let mut maybe_sources = None;
+
     for item in ErrorLoggerIterator(parse_declaration_list(block.move_iter())) {
         match item {
             DeclAtRule(rule) => log_css_error(
                 rule.location, format!("Unsupported at-rule in declaration list: @{:s}", rule.name).as_slice()),
-            Declaration(Declaration{ location: location, name: name, value: value, important: _}) => {
+            Declaration(Declaration{ location, name, value, important }) => {
-
+                if important {
+                    log_css_error(location, "!important is not allowed on @font-face descriptors");
+                    continue
+                }
                 let name_lower = name.as_slice().to_ascii_lower();
                 match name_lower.as_slice() {
                     "font-family" => {
-                        // FIXME(#2802): Share code with the font-family parser.
+                        let iter = &mut BufferedIter::new(value.as_slice().skip_whitespace());
-                        match one_component_value(value.as_slice()) {
+                        match parse_one_family(iter) {
-                            Some(&String(ref string_value)) => {
+                            Ok(FamilyName(name)) => {
-                                maybe_family = Some(string_value.clone());
+                                maybe_family = Some(name);
                             },
-                            _ => {
+                            // This also includes generic family names:
-                                log_css_error(location, format!("Unsupported font-family string {:s}", name).as_slice());
+                            _ => log_css_error(location, "Invalid font-family in @font-face"),
-                            }
                         }
                     },
                     "src" => {
-                        let mut iter = value.as_slice().skip_whitespace();
+                        match parse_slice_comma_separated(
-                        let mut sources = vec!();
+                                value.as_slice(), |iter| parse_one_url_src(iter, base_url)) {
-                        let mut syntax_error = false;
+                            Ok(sources) => maybe_sources = Some(sources),
-
+                            Err(()) => log_css_error(location, "Invalid src in @font-face"),
-                        'outer: loop {
-
-                            // url() or local() should be next
-                            let maybe_url = match iter.next() {
-                                Some(&URL(ref string_value)) => {
-                                    let maybe_url = UrlParser::new().base_url(base_url).parse(string_value.as_slice());
-                                    let url = maybe_url.unwrap_or_else(|_| Url::parse("about:invalid").unwrap());
-                                    Some(url)
-                                },
-                                Some(&Function(ref string_value, ref _values)) => {
-                                    match string_value.as_slice() {
-                                        "local" => {
-                                            log_css_error(location, "local font face is not supported yet - skipping");
-                                            None
-                                        },
-                                        _ => {
-                                            log_css_error(location, format!("Unexpected token {}", string_value).as_slice());
-                                            syntax_error = true;
-                                            break;
-                                        }
-                                    }
-                                },
-                                _ => {
-                                    log_css_error(location, "Unsupported declaration type");
-                                    syntax_error = true;
-                                    break;
-                                }
                         };
-
-                            let mut next_token = iter.next();
-
-                            match maybe_url {
-                                Some(url) => {
-                                    let mut source = FontFaceSource {
-                                        url: url,
-                                        format_hints: vec!(),
-                                    };
-
-                                    // optional format, or comma to start loop again
-                                    match next_token {
-                                        Some(&Function(ref string_value, ref values)) => {
-                                            match string_value.as_slice() {
-                                                "format" => {
-                                                    let mut format_iter = values.as_slice().skip_whitespace();
-
-                                                    loop {
-                                                        let fmt_token = format_iter.next();
-                                                        match fmt_token {
-                                                            Some(&String(ref format_hint)) => {
-                                                                let hint = match format_hint.as_slice() {
-                                                                    "embedded-opentype" => EotFormat,
-                                                                    "woff" => WoffFormat,
-                                                                    "truetype" | "opentype" => TtfFormat,
-                                                                    "svg" => SvgFormat,
-                                                                    _ => UnknownFormat,
-                                                                };
-                                                                source.format_hints.push(hint);
-                                                            },
-                                                            _ => {
-                                                                log_css_error(location, "Unexpected token");
-                                                                syntax_error = true;
-                                                                break 'outer;
-                                                            }
-                                                        }
-
-                                                        let comma_token = format_iter.next();
-                                                        match comma_token {
-                                                            Some(&Comma) => {},
-                                                            None => {
-                                                                break;
-                                                            }
-                                                            _ => {
-                                                                log_css_error(location, "Unexpected token");
-                                                                syntax_error = true;
-                                                                break 'outer;
-                                                            }
-                                                        }
-                                                    }
-                                                },
-                                                _ => {
-                                                    log_css_error(location,
-                                                                    format!("Unsupported token {}", string_value).as_slice());
-                                                    syntax_error = true;
-                                                    break;
-                                                }
-                                            }
-                                            next_token = iter.next();
-                                        },
-                                        _ => {}
-                                    }
-
-                                    sources.push(source);
-                                },
-                                None => {},
-                            }
-
-                            // after url or optional format, comes comma or end
-                            match next_token {
-                                Some(&Comma) => {},
-                                None => break,
-                                _ => {
-                                    log_css_error(location, "Unexpected token type");
-                                    syntax_error = true;
-                                    break;
-                                }
-                            }
-                        }
-
-                        if !syntax_error && sources.len() > 0 {
-                            let source_line = FontFaceSourceLine {
-                                sources: sources
-                            };
-                            source_lines.push(source_line);
-                        }
                     },
                     _ => {
                         log_css_error(location, format!("Unsupported declaration {:s}", name).as_slice());
@@ -197,11 +106,78 @@ pub fn parse_font_face_rule(rule: AtRule, parent_rules: &mut Vec<CSSRule>, base_
         }
     }
 
-    if maybe_family.is_some() && source_lines.len() > 0 {
+    match (maybe_family, maybe_sources) {
-        let font_face_rule = FontFaceRule {
+        (Some(family), Some(sources)) => parent_rules.push(CSSFontFaceRule(FontFaceRule {
-            family: maybe_family.unwrap(),
+            family: family,
-            source_lines: source_lines,
+            sources: sources,
+        })),
+        (None, _) => log_css_error(rule.location, "@font-face without a font-family descriptor"),
+        _ => log_css_error(rule.location, "@font-face without an src descriptor"),
+    }
+}
+
+
+/// local() is not supported yet
+fn parse_one_url_src(iter: ParserIter, base_url: &Url) -> Result<UrlSource, ()> {
+    match parse_one_src(iter, base_url) {
+        Ok(UrlSource(source)) => Ok(source),
+        _ => Err(())
+    }
+}
+
+
+fn parse_one_src(iter: ParserIter, base_url: &Url) -> Result<Source, ()> {
+    let url = match iter.next() {
+        // Parsing url()
+        Some(&URL(ref url)) => {
+            UrlParser::new().base_url(base_url).parse(url.as_slice()).unwrap_or_else(
+                |_error| Url::parse("about:invalid").unwrap())
+        },
+        // Parsing local() with early return()
+        Some(&Function(ref name, ref arguments)) => {
+            if name.as_slice().eq_ignore_ascii_case("local") {
+                let iter = &mut BufferedIter::new(arguments.as_slice().skip_whitespace());
+                match parse_one_family(iter) {
+                    Ok(FamilyName(name)) => return Ok(LocalSource(name)),
+                    _ => return Err(())
+                }
+            }
+            return Err(())
+        },
+        _ => return Err(())
     };
-        parent_rules.push(CSSFontFaceRule(font_face_rule));
+
+    // Parsing optional format()
+    let format_hints = match iter.next() {
+        Some(&Function(ref name, ref arguments)) => {
+            if !name.as_slice().eq_ignore_ascii_case("format") {
+                return Err(())
+            }
+            try!(parse_slice_comma_separated(arguments.as_slice(), parse_one_format))
+        }
+        Some(component_value) => {
+            iter.push_back(component_value);
+            vec![]
+        }
+        None => vec![],
+    };
+
+    Ok(UrlSource(UrlSource {
+        url: url,
+        format_hints: format_hints,
+    }))
+}
+
+
+fn parse_one_format(iter: ParserIter) -> Result<String, ()> {
+    match iter.next() {
+        Some(&String(ref value)) => {
+            if iter.next().is_none() {
+                Ok(value.clone())
+            } else {
+                Err(())
+            }
+        }
+        _ => Err(())
     }
 }

src/components/style/node.rs

@@ -18,6 +18,7 @@ pub trait TNode<E:TElement> : Clone {
     fn is_element(&self) -> bool;
     fn as_element(&self) -> E;
     fn match_attr(&self, attr: &AttrSelector, test: |&str| -> bool) -> bool;
+    fn is_html_element_in_html_document(&self) -> bool;
 }
 
 pub trait TElement {

src/components/style/parsing_utils.rs

@@ -4,18 +4,78 @@
 
 
 use std::ascii::StrAsciiExt;
-use cssparser::ast::{ComponentValue, Ident, SkipWhitespaceIterable};
+use cssparser::ast::{ComponentValue, Ident, Comma, SkipWhitespaceIterable, SkipWhitespaceIterator};
 
 
-pub fn one_component_value<'a>(input: &'a [ComponentValue]) -> Option<&'a ComponentValue> {
+pub fn one_component_value<'a>(input: &'a [ComponentValue]) -> Result<&'a ComponentValue, ()> {
     let mut iter = input.skip_whitespace();
-    iter.next().filtered(|_| iter.next().is_none())
+    match iter.next() {
+        Some(value) => if iter.next().is_none() { Ok(value) } else { Err(()) },
+        None => Err(())
+    }
 }
 
 
-pub fn get_ident_lower(component_value: &ComponentValue) -> Option<String> {
+pub fn get_ident_lower(component_value: &ComponentValue) -> Result<String, ()> {
     match component_value {
-        &Ident(ref value) => Some(value.as_slice().to_ascii_lower()),
+        &Ident(ref value) => Ok(value.as_slice().to_ascii_lower()),
-        _ => None,
+        _ => Err(()),
     }
 }
+
+
+pub struct BufferedIter<E, I> {
+    iter: I,
+    buffer: Option<E>,
+}
+
+impl<E, I: Iterator<E>> BufferedIter<E, I> {
+    pub fn new(iter: I) -> BufferedIter<E, I> {
+        BufferedIter {
+            iter: iter,
+            buffer: None,
+        }
+    }
+
+    #[inline]
+    pub fn push_back(&mut self, value: E) {
+        assert!(self.buffer.is_none());
+        self.buffer = Some(value);
+    }
+}
+
+impl<E, I: Iterator<E>> Iterator<E> for BufferedIter<E, I> {
+    #[inline]
+    fn next(&mut self) -> Option<E> {
+        if self.buffer.is_some() {
+            self.buffer.take()
+        }
+        else {
+            self.iter.next()
+        }
+    }
+}
+
+pub type ParserIter<'a, 'b> = &'a mut BufferedIter<&'b ComponentValue, SkipWhitespaceIterator<'b>>;
+
+
+#[inline]
+pub fn parse_slice_comma_separated<T>(input: &[ComponentValue],
+                                      parse_one: |ParserIter| -> Result<T, ()>)
+                                      -> Result<Vec<T>, ()> {
+    parse_comma_separated(&mut BufferedIter::new(input.skip_whitespace()), parse_one)
+}
+
+#[inline]
+pub fn parse_comma_separated<T>(iter: ParserIter,
+                                parse_one: |ParserIter| -> Result<T, ()>)
+                                -> Result<Vec<T>, ()> {
+    let mut values = vec![try!(parse_one(iter))];
+    for component_value in iter {
+        match component_value {
+            &Comma => values.push(try!(parse_one(iter))),
+            _ => return Err(())
+        }
+    }
+    Ok(values)
+}

src/components/style/properties/common_types.rs

@@ -40,32 +40,32 @@ pub mod specified {
     static AU_PER_PC: CSSFloat = AU_PER_PT * 12.;
     impl Length {
         #[inline]
-        fn parse_internal(input: &ComponentValue, negative_ok: bool) -> Option<Length> {
+        fn parse_internal(input: &ComponentValue, negative_ok: bool) -> Result<Length, ()> {
             match input {
                 &Dimension(ref value, ref unit) if negative_ok || value.value >= 0.
                 => Length::parse_dimension(value.value, unit.as_slice()),
-                &Number(ref value) if value.value == 0. =>  Some(Au_(Au(0))),
+                &Number(ref value) if value.value == 0. =>  Ok(Au_(Au(0))),
-                _ => None
+                _ => Err(())
             }
         }
         #[allow(dead_code)]
-        pub fn parse(input: &ComponentValue) -> Option<Length> {
+        pub fn parse(input: &ComponentValue) -> Result<Length, ()> {
             Length::parse_internal(input, /* negative_ok = */ true)
         }
-        pub fn parse_non_negative(input: &ComponentValue) -> Option<Length> {
+        pub fn parse_non_negative(input: &ComponentValue) -> Result<Length, ()> {
             Length::parse_internal(input, /* negative_ok = */ false)
         }
-        pub fn parse_dimension(value: CSSFloat, unit: &str) -> Option<Length> {
+        pub fn parse_dimension(value: CSSFloat, unit: &str) -> Result<Length, ()> {
             match unit.to_ascii_lower().as_slice() {
-                "px" => Some(Length::from_px(value)),
+                "px" => Ok(Length::from_px(value)),
-                "in" => Some(Au_(Au((value * AU_PER_IN) as i32))),
+                "in" => Ok(Au_(Au((value * AU_PER_IN) as i32))),
-                "cm" => Some(Au_(Au((value * AU_PER_CM) as i32))),
+                "cm" => Ok(Au_(Au((value * AU_PER_CM) as i32))),
-                "mm" => Some(Au_(Au((value * AU_PER_MM) as i32))),
+                "mm" => Ok(Au_(Au((value * AU_PER_MM) as i32))),
-                "pt" => Some(Au_(Au((value * AU_PER_PT) as i32))),
+                "pt" => Ok(Au_(Au((value * AU_PER_PT) as i32))),
-                "pc" => Some(Au_(Au((value * AU_PER_PC) as i32))),
+                "pc" => Ok(Au_(Au((value * AU_PER_PC) as i32))),
-                "em" => Some(Em(value)),
+                "em" => Ok(Em(value)),
-                "ex" => Some(Ex(value)),
+                "ex" => Ok(Ex(value)),
-                _ => None
+                _ => Err(())
             }
         }
         #[inline]
@@ -81,23 +81,23 @@ pub mod specified {
     }
     impl LengthOrPercentage {
         fn parse_internal(input: &ComponentValue, negative_ok: bool)
-                              -> Option<LengthOrPercentage> {
+                              -> Result<LengthOrPercentage, ()> {
             match input {
                 &Dimension(ref value, ref unit) if negative_ok || value.value >= 0.
                 => Length::parse_dimension(value.value, unit.as_slice()).map(LP_Length),
                 &ast::Percentage(ref value) if negative_ok || value.value >= 0.
-                => Some(LP_Percentage(value.value / 100.)),
+                => Ok(LP_Percentage(value.value / 100.)),
-                &Number(ref value) if value.value == 0. =>  Some(LP_Length(Au_(Au(0)))),
+                &Number(ref value) if value.value == 0. =>  Ok(LP_Length(Au_(Au(0)))),
-                _ => None
+                _ => Err(())
             }
         }
         #[allow(dead_code)]
         #[inline]
-        pub fn parse(input: &ComponentValue) -> Option<LengthOrPercentage> {
+        pub fn parse(input: &ComponentValue) -> Result<LengthOrPercentage, ()> {
             LengthOrPercentage::parse_internal(input, /* negative_ok = */ true)
         }
         #[inline]
-        pub fn parse_non_negative(input: &ComponentValue) -> Option<LengthOrPercentage> {
+        pub fn parse_non_negative(input: &ComponentValue) -> Result<LengthOrPercentage, ()> {
             LengthOrPercentage::parse_internal(input, /* negative_ok = */ false)
         }
     }
@@ -110,23 +110,23 @@ pub mod specified {
     }
     impl LengthOrPercentageOrAuto {
         fn parse_internal(input: &ComponentValue, negative_ok: bool)
-                     -> Option<LengthOrPercentageOrAuto> {
+                     -> Result<LengthOrPercentageOrAuto, ()> {
             match input {
                 &Dimension(ref value, ref unit) if negative_ok || value.value >= 0.
                 => Length::parse_dimension(value.value, unit.as_slice()).map(LPA_Length),
                 &ast::Percentage(ref value) if negative_ok || value.value >= 0.
-                => Some(LPA_Percentage(value.value / 100.)),
+                => Ok(LPA_Percentage(value.value / 100.)),
-                &Number(ref value) if value.value == 0. => Some(LPA_Length(Au_(Au(0)))),
+                &Number(ref value) if value.value == 0. => Ok(LPA_Length(Au_(Au(0)))),
-                &Ident(ref value) if value.as_slice().eq_ignore_ascii_case("auto") => Some(LPA_Auto),
+                &Ident(ref value) if value.as_slice().eq_ignore_ascii_case("auto") => Ok(LPA_Auto),
-                _ => None
+                _ => Err(())
             }
         }
         #[inline]
-        pub fn parse(input: &ComponentValue) -> Option<LengthOrPercentageOrAuto> {
+        pub fn parse(input: &ComponentValue) -> Result<LengthOrPercentageOrAuto, ()> {
             LengthOrPercentageOrAuto::parse_internal(input, /* negative_ok = */ true)
         }
         #[inline]
-        pub fn parse_non_negative(input: &ComponentValue) -> Option<LengthOrPercentageOrAuto> {
+        pub fn parse_non_negative(input: &ComponentValue) -> Result<LengthOrPercentageOrAuto, ()> {
             LengthOrPercentageOrAuto::parse_internal(input, /* negative_ok = */ false)
         }
     }
@@ -139,24 +139,24 @@ pub mod specified {
     }
     impl LengthOrPercentageOrNone {
         fn parse_internal(input: &ComponentValue, negative_ok: bool)
-                     -> Option<LengthOrPercentageOrNone> {
+                     -> Result<LengthOrPercentageOrNone, ()> {
             match input {
                 &Dimension(ref value, ref unit) if negative_ok || value.value >= 0.
                 => Length::parse_dimension(value.value, unit.as_slice()).map(LPN_Length),
                 &ast::Percentage(ref value) if negative_ok || value.value >= 0.
-                => Some(LPN_Percentage(value.value / 100.)),
+                => Ok(LPN_Percentage(value.value / 100.)),
-                &Number(ref value) if value.value == 0. => Some(LPN_Length(Au_(Au(0)))),
+                &Number(ref value) if value.value == 0. => Ok(LPN_Length(Au_(Au(0)))),
-                &Ident(ref value) if value.as_slice().eq_ignore_ascii_case("none") => Some(LPN_None),
+                &Ident(ref value) if value.as_slice().eq_ignore_ascii_case("none") => Ok(LPN_None),
-                _ => None
+                _ => Err(())
             }
         }
         #[allow(dead_code)]
         #[inline]
-        pub fn parse(input: &ComponentValue) -> Option<LengthOrPercentageOrNone> {
+        pub fn parse(input: &ComponentValue) -> Result<LengthOrPercentageOrNone, ()> {
             LengthOrPercentageOrNone::parse_internal(input, /* negative_ok = */ true)
         }
         #[inline]
-        pub fn parse_non_negative(input: &ComponentValue) -> Option<LengthOrPercentageOrNone> {
+        pub fn parse_non_negative(input: &ComponentValue) -> Result<LengthOrPercentageOrNone, ()> {
             LengthOrPercentageOrNone::parse_internal(input, /* negative_ok = */ false)
         }
     }

src/components/style/properties/mod.rs.mako

@@ -18,7 +18,7 @@ pub use geom::SideOffsets2D;
 use errors::{ErrorLoggerIterator, log_css_error};
 pub use parsing_utils::*;
 pub use self::common_types::*;
-use selector_matching::MatchedProperty;
+use selector_matching::DeclarationBlock;
 
 
 pub use self::property_bit_field::PropertyBitField;
@@ -118,12 +118,13 @@ pub mod longhands {
             ${caller.body()}
             % if derived_from is None:
                 pub fn parse_declared(input: &[ComponentValue], base_url: &Url)
-                                   -> Option<DeclaredValue<SpecifiedValue>> {
+                                   -> Result<DeclaredValue<SpecifiedValue>, ()> {
                     match CSSWideKeyword::parse(input) {
-                        Some(Some(keyword)) => Some(CSSWideKeyword(keyword)),
+                        Ok(InheritKeyword) => Ok(Inherit),
-                        Some(None) => Some(CSSWideKeyword(${
+                        Ok(InitialKeyword) => Ok(Initial),
-                            "Inherit" if THIS_STYLE_STRUCT.inherited else "Initial"})),
+                        Ok(UnsetKeyword) => Ok(${
-                        None => parse_specified(input, base_url),
+                            "Inherit" if THIS_STYLE_STRUCT.inherited else "Initial"}),
+                        Err(()) => parse_specified(input, base_url),
                     }
                 }
             % endif
@@ -136,7 +137,7 @@ pub mod longhands {
             ${caller.body()}
             % if derived_from is None:
                 pub fn parse_specified(_input: &[ComponentValue], _base_url: &Url)
-                                   -> Option<DeclaredValue<SpecifiedValue>> {
+                                   -> Result<DeclaredValue<SpecifiedValue>, ()> {
                     parse(_input, _base_url).map(super::SpecifiedValue)
                 }
             % endif
@@ -147,7 +148,7 @@ pub mod longhands {
         <%self:longhand name="${name}" derived_from="${derived_from}"
                         experimental="${experimental}">
             ${caller.body()}
-            pub fn parse(input: &[ComponentValue], base_url: &Url) -> Option<SpecifiedValue> {
+            pub fn parse(input: &[ComponentValue], base_url: &Url) -> Result<SpecifiedValue, ()> {
                 one_component_value(input).and_then(|c| from_component_value(c, base_url))
             }
         </%self:longhand>
@@ -170,13 +171,13 @@ pub mod longhands {
                 ${to_rust_ident(values.split()[0])}
             }
             pub fn from_component_value(v: &ComponentValue, _base_url: &Url)
-                                        -> Option<SpecifiedValue> {
+                                        -> Result<SpecifiedValue, ()> {
                 get_ident_lower(v).and_then(|keyword| {
                     match keyword.as_slice() {
                         % for value in values.split():
-                            "${value}" => Some(${to_rust_ident(value)}),
+                            "${value}" => Ok(${to_rust_ident(value)}),
                         % endfor
-                        _ => None,
+                        _ => Err(()),
                     }
                 })
             }
@@ -201,7 +202,7 @@ pub mod longhands {
             }
             #[inline] pub fn get_initial_value() -> computed_value::T { ${initial_value} }
             #[inline] pub fn from_component_value(v: &ComponentValue, _base_url: &Url)
-                                                  -> Option<SpecifiedValue> {
+                                                  -> Result<SpecifiedValue, ()> {
                 specified::${type}::${parse_method}(v)
             }
         </%self:single_component_value>
@@ -244,14 +245,14 @@ pub mod longhands {
     % endfor
 
     pub fn parse_border_width(component_value: &ComponentValue, _base_url: &Url)
-                              -> Option<specified::Length> {
+                              -> Result<specified::Length, ()> {
         match component_value {
             &Ident(ref value) => {
                 match value.as_slice().to_ascii_lower().as_slice() {
-                    "thin" => Some(specified::Length::from_px(1.)),
+                    "thin" => Ok(specified::Length::from_px(1.)),
-                    "medium" => Some(specified::Length::from_px(3.)),
+                    "medium" => Ok(specified::Length::from_px(3.)),
-                    "thick" => Some(specified::Length::from_px(5.)),
+                    "thick" => Ok(specified::Length::from_px(5.)),
-                    _ => None
+                    _ => Err(())
                 }
             },
             _ => specified::Length::parse_non_negative(component_value)
@@ -267,7 +268,7 @@ pub mod longhands {
             #[inline] pub fn get_initial_value() -> computed_value::T {
                 Au::from_px(3)  // medium
             }
-            pub fn parse(input: &[ComponentValue], base_url: &Url) -> Option<SpecifiedValue> {
+            pub fn parse(input: &[ComponentValue], base_url: &Url) -> Result<SpecifiedValue, ()> {
                 one_component_value(input).and_then(|c| parse_border_width(c, base_url))
             }
             #[inline]
@@ -346,7 +347,7 @@ pub mod longhands {
         pub fn get_initial_value() -> computed_value::T { computed::LPA_Auto }
         #[inline]
         pub fn from_component_value(v: &ComponentValue, _base_url: &Url)
-                                              -> Option<SpecifiedValue> {
+                                              -> Result<SpecifiedValue, ()> {
             specified::LengthOrPercentageOrAuto::parse_non_negative(v)
         }
         pub fn to_computed_value(value: SpecifiedValue, context: &computed::Context)
@@ -387,18 +388,18 @@ pub mod longhands {
         }
         /// normal | <number> | <length> | <percentage>
         pub fn from_component_value(input: &ComponentValue, _base_url: &Url)
-                                    -> Option<SpecifiedValue> {
+                                    -> Result<SpecifiedValue, ()> {
             match input {
                 &ast::Number(ref value) if value.value >= 0.
-                => Some(SpecifiedNumber(value.value)),
+                => Ok(SpecifiedNumber(value.value)),
                 &ast::Percentage(ref value) if value.value >= 0.
-                => Some(SpecifiedLength(specified::Em(value.value / 100.))),
+                => Ok(SpecifiedLength(specified::Em(value.value / 100.))),
                 &Dimension(ref value, ref unit) if value.value >= 0.
                 => specified::Length::parse_dimension(value.value, unit.as_slice())
                     .map(SpecifiedLength),
                 &Ident(ref value) if value.as_slice().eq_ignore_ascii_case("normal")
-                => Some(SpecifiedNormal),
+                => Ok(SpecifiedNormal),
-                _ => None,
+                _ => Err(()),
             }
         }
         pub mod computed_value {
@@ -474,14 +475,14 @@ pub mod longhands {
         /// baseline | sub | super | top | text-top | middle | bottom | text-bottom
         /// | <percentage> | <length>
         pub fn from_component_value(input: &ComponentValue, _base_url: &Url)
-                                    -> Option<SpecifiedValue> {
+                                    -> Result<SpecifiedValue, ()> {
             match input {
                 &Ident(ref value) => {
                     match value.as_slice().to_ascii_lower().as_slice() {
                         % for keyword in vertical_align_keywords:
-                        "${keyword}" => Some(Specified_${to_rust_ident(keyword)}),
+                        "${keyword}" => Ok(Specified_${to_rust_ident(keyword)}),
                         % endfor
-                        _ => None,
+                        _ => Err(()),
                     }
                 },
                 _ => specified::LengthOrPercentage::parse_non_negative(input)
@@ -552,12 +553,12 @@ pub mod longhands {
 
             // normal | none | [ <string> ]+
             // TODO: <uri>, <counter>, attr(<identifier>), open-quote, close-quote, no-open-quote, no-close-quote
-            pub fn parse(input: &[ComponentValue], _base_url: &Url) -> Option<SpecifiedValue> {
+            pub fn parse(input: &[ComponentValue], _base_url: &Url) -> Result<SpecifiedValue, ()> {
                 match one_component_value(input) {
-                    Some(&Ident(ref keyword)) => {
+                    Ok(&Ident(ref keyword)) => {
                         match keyword.as_slice().to_ascii_lower().as_slice() {
-                            "normal" => return Some(normal),
+                            "normal" => return Ok(normal),
-                            "none" => return Some(none),
+                            "none" => return Ok(none),
                             _ => ()
                         }
                     },
@@ -568,10 +569,10 @@ pub mod longhands {
                     match component_value {
                         &String(ref value)
                         => content.push(StringContent(value.clone())),
-                        _ => return None  // invalid/unsupported value
+                        _ => return Err(())  // invalid/unsupported value
                     }
                 }
-                Some(Content(content))
+                Ok(Content(content))
             }
     </%self:longhand>
     // CSS 2.1, Section 13 - Paged media
@@ -593,14 +594,16 @@ pub mod longhands {
             #[inline] pub fn get_initial_value() -> SpecifiedValue {
                 None
             }
-            pub fn from_component_value(component_value: &ComponentValue, base_url: &Url) -> Option<SpecifiedValue> {
+            pub fn from_component_value(component_value: &ComponentValue, base_url: &Url)
+                                        -> Result<SpecifiedValue, ()> {
                 match component_value {
                     &ast::URL(ref url) => {
                         let image_url = parse_url(url.as_slice(), base_url);
-                        Some(Some(image_url))
+                        Ok(Some(image_url))
                     },
-                    &ast::Ident(ref value) if value.as_slice().eq_ignore_ascii_case("none") => Some(None),
+                    &ast::Ident(ref value) if value.as_slice().eq_ignore_ascii_case("none")
-                    _ => None,
+                    => Ok(None),
+                    _ => Err(()),
                 }
             }
     </%self:single_component_value>
@@ -643,36 +646,29 @@ pub mod longhands {
 
             // FIXME(#1997, pcwalton): Support complete CSS2 syntax.
             pub fn parse_horizontal_and_vertical(horiz: &ComponentValue, vert: &ComponentValue)
-                                                 -> Option<SpecifiedValue> {
+                                                 -> Result<SpecifiedValue, ()> {
-                let horiz = match specified::LengthOrPercentage::parse_non_negative(horiz) {
+                let horiz = try!(specified::LengthOrPercentage::parse_non_negative(horiz));
-                    None => return None,
+                let vert = try!(specified::LengthOrPercentage::parse_non_negative(vert));
-                    Some(value) => value,
-                };
 
-                let vert = match specified::LengthOrPercentage::parse_non_negative(vert) {
+                Ok(SpecifiedValue {
-                    None => return None,
-                    Some(value) => value,
-                };
-
-                Some(SpecifiedValue {
                     horizontal: horiz,
                     vertical: vert,
                 })
             }
 
-            pub fn parse(input: &[ComponentValue], _: &Url) -> Option<SpecifiedValue> {
+            pub fn parse(input: &[ComponentValue], _: &Url) -> Result<SpecifiedValue, ()> {
                 let mut input_iter = input.skip_whitespace();
                 let horizontal = input_iter.next();
                 let vertical = input_iter.next();
                 if input_iter.next().is_some() {
-                    return None
+                    return Err(())
                 }
 
                 match (horizontal, vertical) {
                     (Some(horizontal), Some(vertical)) => {
                         parse_horizontal_and_vertical(horizontal, vertical)
                     }
-                    _ => None
+                    _ => Err(())
                 }
             }
     </%self:longhand>
@@ -693,11 +689,11 @@ pub mod longhands {
             RGBA { red: 0., green: 0., blue: 0., alpha: 1. }  /* black */
         }
         pub fn parse_specified(input: &[ComponentValue], _base_url: &Url)
-                               -> Option<DeclaredValue<SpecifiedValue>> {
+                               -> Result<DeclaredValue<SpecifiedValue>, ()> {
             match one_component_value(input).and_then(Color::parse) {
-                Some(RGBA(rgba)) => Some(SpecifiedValue(rgba)),
+                Ok(RGBA(rgba)) => Ok(SpecifiedValue(rgba)),
-                Some(CurrentColor) => Some(CSSWideKeyword(Inherit)),
+                Ok(CurrentColor) => Ok(Inherit),
-                None => None,
+                Err(()) => Err(()),
             }
         }
     </%self:raw_longhand>
@@ -722,61 +718,48 @@ pub mod longhands {
             pub type T = Vec<FontFamily>;
         }
         pub type SpecifiedValue = computed_value::T;
-        #[inline] pub fn get_initial_value() -> computed_value::T { vec!(FamilyName("serif".to_string())) }
+
+        #[inline]
+        pub fn get_initial_value() -> computed_value::T {
+            vec![FamilyName("serif".to_string())]
+        }
         /// <familiy-name>#
         /// <familiy-name> = <string> | [ <ident>+ ]
         /// TODO: <generic-familiy>
-        pub fn parse(input: &[ComponentValue], _base_url: &Url) -> Option<SpecifiedValue> {
+        pub fn parse(input: &[ComponentValue], _base_url: &Url) -> Result<SpecifiedValue, ()> {
-            from_iter(input.skip_whitespace())
+            parse_slice_comma_separated(input, parse_one_family)
         }
-        pub fn from_iter<'a>(mut iter: SkipWhitespaceIterator<'a>) -> Option<SpecifiedValue> {
+        pub fn parse_one_family<'a>(iter: ParserIter) -> Result<FontFamily, ()> {
-            let mut result = vec!();
-            macro_rules! add(
-                ($value: expr, $b: expr) => {
-                    {
-                        result.push($value);
-                        match iter.next() {
-                            Some(&Comma) => (),
-                            None => $b,
-                            _ => return None,
-                        }
-                    }
-                }
-            )
-            'outer: loop {
-                match iter.next() {
             // TODO: avoid copying strings?
-                    Some(&String(ref value)) => add!(FamilyName(value.clone()), break 'outer),
+            let mut idents = match iter.next() {
+                Some(&String(ref value)) => return Ok(FamilyName(value.clone())),
                 Some(&Ident(ref value)) => {
-                        match value.as_slice().to_ascii_lower().as_slice() {
+//                    match value.as_slice().to_ascii_lower().as_slice() {
-//                            "serif" => add!(Serif, break 'outer),
+//                        "serif" => return Ok(Serif),
-//                            "sans-serif" => add!(SansSerif, break 'outer),
+//                        "sans-serif" => return Ok(SansSerif),
-//                            "cursive" => add!(Cursive, break 'outer),
+//                        "cursive" => return Ok(Cursive),
-//                            "fantasy" => add!(Fantasy, break 'outer),
+//                        "fantasy" => return Ok(Fantasy),
-//                            "monospace" => add!(Monospace, break 'outer),
+//                        "monospace" => return Ok(Monospace),
+//                        _ => {
+                            vec![value.as_slice()]
+//                        }
+//                    }
+                }
+                _ => return Err(())
+            };
+            for component_value in iter {
+                match component_value {
+                    &Ident(ref value) => {
+                        idents.push(value.as_slice());
+                        iter.next();
+                    },
                     _ => {
-                                let mut idents = vec!(value.as_slice());
+                        iter.push_back(component_value);
-                                loop {
-                                    match iter.next() {
-                                        Some(&Ident(ref value)) => idents.push(value.as_slice()),
-                                        Some(&Comma) => {
-                                            result.push(FamilyName(idents.connect(" ")));
                         break
-                                        },
-                                        None => {
-                                            result.push(FamilyName(idents.connect(" ")));
-                                            break 'outer
-                                        },
-                                        _ => return None,
                     }
                 }
             }
-                        }
+            Ok(FamilyName(idents.connect(" ")))
-                    }
-                    _ => return None,
-                }
-            }
-            Some(result)
         }
     </%self:longhand>
 
@@ -795,30 +778,30 @@ pub mod longhands {
         }
         /// normal | bold | bolder | lighter | 100 | 200 | 300 | 400 | 500 | 600 | 700 | 800 | 900
         pub fn from_component_value(input: &ComponentValue, _base_url: &Url)
-                                    -> Option<SpecifiedValue> {
+                                    -> Result<SpecifiedValue, ()> {
             match input {
                 &Ident(ref value) => {
                     match value.as_slice().to_ascii_lower().as_slice() {
-                        "bold" => Some(SpecifiedWeight700),
+                        "bold" => Ok(SpecifiedWeight700),
-                        "normal" => Some(SpecifiedWeight400),
+                        "normal" => Ok(SpecifiedWeight400),
-                        "bolder" => Some(Bolder),
+                        "bolder" => Ok(Bolder),
-                        "lighter" => Some(Lighter),
+                        "lighter" => Ok(Lighter),
-                        _ => None,
+                        _ => Err(()),
                     }
                 },
                 &Number(ref value) => match value.int_value {
-                    Some(100) => Some(SpecifiedWeight100),
+                    Some(100) => Ok(SpecifiedWeight100),
-                    Some(200) => Some(SpecifiedWeight200),
+                    Some(200) => Ok(SpecifiedWeight200),
-                    Some(300) => Some(SpecifiedWeight300),
+                    Some(300) => Ok(SpecifiedWeight300),
-                    Some(400) => Some(SpecifiedWeight400),
+                    Some(400) => Ok(SpecifiedWeight400),
-                    Some(500) => Some(SpecifiedWeight500),
+                    Some(500) => Ok(SpecifiedWeight500),
-                    Some(600) => Some(SpecifiedWeight600),
+                    Some(600) => Ok(SpecifiedWeight600),
-                    Some(700) => Some(SpecifiedWeight700),
+                    Some(700) => Ok(SpecifiedWeight700),
-                    Some(800) => Some(SpecifiedWeight800),
+                    Some(800) => Ok(SpecifiedWeight800),
-                    Some(900) => Some(SpecifiedWeight900),
+                    Some(900) => Ok(SpecifiedWeight900),
-                    _ => None,
+                    _ => Err(()),
                 },
-                _ => None
+                _ => Err(())
             }
         }
         pub mod computed_value {
@@ -890,7 +873,7 @@ pub mod longhands {
         /// <length> | <percentage>
         /// TODO: support <absolute-size> and <relative-size>
         pub fn from_component_value(input: &ComponentValue, _base_url: &Url)
-                                    -> Option<SpecifiedValue> {
+                                    -> Result<SpecifiedValue, ()> {
             specified::LengthOrPercentage::parse_non_negative(input).map(|value| {
                 match value {
                     specified::LP_Length(value) => value,
@@ -927,30 +910,34 @@ pub mod longhands {
             none
         }
         /// none | [ underline || overline || line-through || blink ]
-        pub fn parse(input: &[ComponentValue], _base_url: &Url) -> Option<SpecifiedValue> {
+        pub fn parse(input: &[ComponentValue], _base_url: &Url) -> Result<SpecifiedValue, ()> {
             let mut result = SpecifiedValue {
                 underline: false, overline: false, line_through: false,
             };
+            match one_component_value(input) {
+                Ok(&Ident(ref value))
+                if value.as_slice().eq_ignore_ascii_case("none") => return Ok(result),
+                _ => {}
+            }
             let mut blink = false;
             let mut empty = true;
             for component_value in input.skip_whitespace() {
                 match get_ident_lower(component_value) {
-                    None => return None,
+                    Err(()) => return Err(()),
-                    Some(keyword) => match keyword.as_slice() {
+                    Ok(keyword) => match keyword.as_slice() {
-                        "underline" => if result.underline { return None }
+                        "underline" => if result.underline { return Err(()) }
                                       else { empty = false; result.underline = true },
-                        "overline" => if result.overline { return None }
+                        "overline" => if result.overline { return Err(()) }
                                       else { empty = false; result.overline = true },
-                        "line-through" => if result.line_through { return None }
+                        "line-through" => if result.line_through { return Err(()) }
                                           else { empty = false; result.line_through = true },
-                        "blink" => if blink { return None }
+                        "blink" => if blink { return Err(()) }
                                    else { empty = false; blink = true },
-                        "none" => return if empty { Some(result) } else { None },
+                        _ => return Err(()),
-                        _ => return None,
                     }
                 }
             }
-            if !empty { Some(result) } else { None }
+            if !empty { Ok(result) } else { Err(()) }
         }
     </%self:longhand>
 
@@ -1070,7 +1057,7 @@ pub mod shorthands {
                     pub ${sub_property.ident}: Option<${sub_property.ident}::SpecifiedValue>,
                 % endfor
             }
-            pub fn parse(input: &[ComponentValue], base_url: &Url) -> Option<Longhands> {
+            pub fn parse(input: &[ComponentValue], base_url: &Url) -> Result<Longhands, ()> {
                 ${caller.body()}
             }
         }
@@ -1080,7 +1067,7 @@ pub mod shorthands {
         <%self:shorthand name="${name}" sub_properties="${
                 ' '.join(sub_property_pattern % side
                          for side in ['top', 'right', 'bottom', 'left'])}">
-            let mut iter = input.skip_whitespace().map(|c| ${parser_function}(c, base_url));
+            let mut iter = input.skip_whitespace().map(|c| ${parser_function}(c, base_url).ok());
             // zero or more than four values is invalid.
             // one value sets them all
             // two values set (top, bottom) and (left, right)
@@ -1092,13 +1079,13 @@ pub mod shorthands {
             let left = iter.next().unwrap_or(right);
             if top.is_some() && right.is_some() && bottom.is_some() && left.is_some()
             && iter.next().is_none() {
-                Some(Longhands {
+                Ok(Longhands {
                     % for side in ["top", "right", "bottom", "left"]:
                         ${to_rust_ident(sub_property_pattern % side)}: ${side},
                     % endfor
                 })
             } else {
-                None
+                Err(())
             }
         </%self:shorthand>
     </%def>
@@ -1116,46 +1103,46 @@ pub mod shorthands {
                 for component_value in input.skip_whitespace() {
                     if color.is_none() {
                         match background_color::from_component_value(component_value, base_url) {
-                            Some(v) => {
+                            Ok(v) => {
                                 color = Some(v);
                                 any = true;
                                 continue
                             },
-                            None => ()
+                            Err(()) => ()
                         }
                     }
 
                     if image.is_none() {
                         match background_image::from_component_value(component_value, base_url) {
-                            Some(v) => {
+                            Ok(v) => {
                                 image = Some(v);
                                 any = true;
                                 continue
                             },
-                            None => (),
+                            Err(()) => (),
                         }
                     }
 
                     if repeat.is_none() {
                         match background_repeat::from_component_value(component_value, base_url) {
-                            Some(v) => {
+                            Ok(v) => {
                                 repeat = Some(v);
                                 any = true;
                                 continue
                             },
-                            None => ()
+                            Err(()) => ()
                         }
                     }
 
                     if attachment.is_none() {
                         match background_attachment::from_component_value(component_value,
                                                                           base_url) {
-                            Some(v) => {
+                            Ok(v) => {
                                 attachment = Some(v);
                                 any = true;
                                 continue
                             },
-                            None => ()
+                            Err(()) => ()
                         }
                     }
 
@@ -1165,17 +1152,17 @@ pub mod shorthands {
                                 match background_position::parse_horizontal_and_vertical(
                                         saved_component_value,
                                         component_value) {
-                                    Some(v) => {
+                                    Ok(v) => {
                                         position = Some(v);
                                         any = true;
                                         continue
                                     },
-                                    None => (),
+                                    Err(()) => (),
                                 }
                             }
 
                             // If we get here, parsing failed.
-                            return None
+                            return Err(())
                         }
                         None => {
                             // Save the component value.
@@ -1185,7 +1172,7 @@ pub mod shorthands {
                 }
 
                 if any && last_component_value.is_none() {
-                    Some(Longhands {
+                    Ok(Longhands {
                         background_color: color,
                         background_image: image,
                         background_position: position,
@@ -1193,14 +1180,14 @@ pub mod shorthands {
                         background_attachment: attachment,
                     })
                 } else {
-                    None
+                    Err(())
                 }
     </%self:shorthand>
 
     ${four_sides_shorthand("margin", "margin-%s", "margin_top::from_component_value")}
     ${four_sides_shorthand("padding", "padding-%s", "padding_top::from_component_value")}
 
-    pub fn parse_color(value: &ComponentValue, _base_url: &Url) -> Option<specified::CSSColor> {
+    pub fn parse_color(value: &ComponentValue, _base_url: &Url) -> Result<specified::CSSColor, ()> {
         specified::CSSColor::parse(value)
     }
     ${four_sides_shorthand("border-color", "border-%s-color", "parse_color")}
@@ -1209,9 +1196,9 @@ pub mod shorthands {
     ${four_sides_shorthand("border-width", "border-%s-width", "parse_border_width")}
 
     pub fn parse_border(input: &[ComponentValue], base_url: &Url)
-                     -> Option<(Option<specified::CSSColor>,
+                     -> Result<(Option<specified::CSSColor>,
                                 Option<border_top_style::SpecifiedValue>,
-                                Option<specified::Length>)> {
+                                Option<specified::Length>), ()> {
         let mut color = None;
         let mut style = None;
         let mut width = None;
@@ -1219,25 +1206,25 @@ pub mod shorthands {
         for component_value in input.skip_whitespace() {
             if color.is_none() {
                 match specified::CSSColor::parse(component_value) {
-                    Some(c) => { color = Some(c); any = true; continue },
+                    Ok(c) => { color = Some(c); any = true; continue },
-                    None => ()
+                    Err(()) => ()
                 }
             }
             if style.is_none() {
                 match border_top_style::from_component_value(component_value, base_url) {
-                    Some(s) => { style = Some(s); any = true; continue },
+                    Ok(s) => { style = Some(s); any = true; continue },
-                    None => ()
+                    Err(()) => ()
                 }
             }
             if width.is_none() {
                 match parse_border_width(component_value, base_url) {
-                    Some(w) => { width = Some(w); any = true; continue },
+                    Ok(w) => { width = Some(w); any = true; continue },
-                    None => ()
+                    Err(()) => ()
                 }
             }
-            return None
+            return Err(())
         }
-        if any { Some((color, style, width)) } else { None }
+        if any { Ok((color, style, width)) } else { Err(()) }
     }
 
 
@@ -1286,7 +1273,7 @@ pub mod shorthands {
             // font-style, font-weight and font-variant.
             // Leaves the values to None, 'normal' is the initial value for each of them.
             match get_ident_lower(component_value) {
-                Some(ref ident) if ident.as_slice().eq_ignore_ascii_case("normal") => {
+                Ok(ref ident) if ident.as_slice().eq_ignore_ascii_case("normal") => {
                     nb_normals += 1;
                     continue;
                 }
@@ -1294,25 +1281,25 @@ pub mod shorthands {
             }
             if style.is_none() {
                 match font_style::from_component_value(component_value, base_url) {
-                    Some(s) => { style = Some(s); continue },
+                    Ok(s) => { style = Some(s); continue },
-                    None => ()
+                    Err(()) => ()
                 }
             }
             if weight.is_none() {
                 match font_weight::from_component_value(component_value, base_url) {
-                    Some(w) => { weight = Some(w); continue },
+                    Ok(w) => { weight = Some(w); continue },
-                    None => ()
+                    Err(()) => ()
                 }
             }
             if variant.is_none() {
                 match font_variant::from_component_value(component_value, base_url) {
-                    Some(v) => { variant = Some(v); continue },
+                    Ok(v) => { variant = Some(v); continue },
-                    None => ()
+                    Err(()) => ()
                 }
             }
             match font_size::from_component_value(component_value, base_url) {
-                Some(s) => { size = Some(s); break },
+                Ok(s) => { size = Some(s); break },
-                None => return None
+                Err(()) => return Err(())
             }
         }
         #[inline]
@@ -1323,29 +1310,29 @@ pub mod shorthands {
             }
         }
         if size.is_none() || (count(&style) + count(&weight) + count(&variant) + nb_normals) > 3 {
-            return None
+            return Err(())
         }
         let mut copied_iter = iter.clone();
         match copied_iter.next() {
             Some(&Delim('/')) => {
                 iter = copied_iter;
                 line_height = match iter.next() {
-                    Some(v) => line_height::from_component_value(v, base_url),
+                    Some(v) => line_height::from_component_value(v, base_url).ok(),
-                    _ => return None,
+                    _ => return Err(()),
                 };
-                if line_height.is_none() { return None }
+                if line_height.is_none() { return Err(()) }
             }
             _ => ()
         }
-        let family = font_family::from_iter(iter);
+        let family = try!(parse_comma_separated(
-        if family.is_none() { return None }
+            &mut BufferedIter::new(iter), font_family::parse_one_family));
-        Some(Longhands {
+        Ok(Longhands {
             font_style: style,
             font_variant: variant,
             font_weight: weight,
             font_size: size,
             line_height: line_height,
-            font_family: family
+            font_family: Some(family)
         })
     </%self:shorthand>
 
@@ -1459,20 +1446,20 @@ pub fn parse_property_declaration_list<I: Iterator<Node>>(input: I, base_url: &U
 }
 
 
-#[deriving(Clone)]
 pub enum CSSWideKeyword {
-    Initial,
+    InitialKeyword,
-    Inherit,
+    InheritKeyword,
+    UnsetKeyword,
 }
 
 impl CSSWideKeyword {
-    pub fn parse(input: &[ComponentValue]) -> Option<Option<CSSWideKeyword>> {
+    pub fn parse(input: &[ComponentValue]) -> Result<CSSWideKeyword, ()> {
         one_component_value(input).and_then(get_ident_lower).and_then(|keyword| {
             match keyword.as_slice() {
-                "initial" => Some(Some(Initial)),
+                "initial" => Ok(InitialKeyword),
-                "inherit" => Some(Some(Inherit)),
+                "inherit" => Ok(InheritKeyword),
-                "unset" => Some(None),
+                "unset" => Ok(UnsetKeyword),
-                _ => None
+                _ => Err(())
             }
         })
     }
@@ -1482,7 +1469,11 @@ impl CSSWideKeyword {
 #[deriving(Clone)]
 pub enum DeclaredValue<T> {
     SpecifiedValue(T),
-    CSSWideKeyword(CSSWideKeyword),
+    Initial,
+    Inherit,
+    // There is no Unset variant here.
+    // The 'unset' keyword is represented as either Initial or Inherit,
+    // depending on whether the property is inherited.
 }
 
 #[deriving(Clone)]
@@ -1521,12 +1512,12 @@ impl PropertyDeclaration {
                             return ValidOrIgnoredDeclaration
                         }
                         match longhands::${property.ident}::parse_declared(value, base_url) {
-                            Some(value) => {
+                            Ok(value) => {
                                 seen.set_${property.ident}();
                                 result_list.push(${property.camel_case}Declaration(value));
                                 ValidOrIgnoredDeclaration
                             },
-                            None => InvalidValue,
+                            Err(()) => InvalidValue,
                         }
                     },
                 % else:
@@ -1540,45 +1531,53 @@ impl PropertyDeclaration {
                         return ValidOrIgnoredDeclaration
                     }
                     match CSSWideKeyword::parse(value) {
-                        Some(Some(keyword)) => {
+                        Ok(InheritKeyword) => {
                             % for sub_property in shorthand.sub_properties:
                                 if !seen.get_${sub_property.ident}() {
                                     seen.set_${sub_property.ident}();
-                                    result_list.push(${sub_property.camel_case}Declaration(
+                                    result_list.push(
-                                        CSSWideKeyword(keyword)));
+                                        ${sub_property.camel_case}Declaration(Inherit));
                                 }
                             % endfor
                             ValidOrIgnoredDeclaration
                         },
-                        Some(None) => {
+                        Ok(InitialKeyword) => {
+                            % for sub_property in shorthand.sub_properties:
+                                if !seen.get_${sub_property.ident}() {
+                                    seen.set_${sub_property.ident}();
+                                    result_list.push(
+                                        ${sub_property.camel_case}Declaration(Initial));
+                                }
+                            % endfor
+                            ValidOrIgnoredDeclaration
+                        },
+                        Ok(UnsetKeyword) => {
                             % for sub_property in shorthand.sub_properties:
                                 if !seen.get_${sub_property.ident}() {
                                     seen.set_${sub_property.ident}();
                                     result_list.push(${sub_property.camel_case}Declaration(
-                                        CSSWideKeyword(
                                         ${"Inherit" if sub_property.style_struct.inherited else "Initial"}
-                                        )
                                     ));
                                 }
                             % endfor
                             ValidOrIgnoredDeclaration
                         },
-                        None => match shorthands::${shorthand.ident}::parse(value, base_url) {
+                        Err(()) => match shorthands::${shorthand.ident}::parse(value, base_url) {
-                            Some(result) => {
+                            Ok(result) => {
                                 % for sub_property in shorthand.sub_properties:
                                     if !seen.get_${sub_property.ident}() {
                                         seen.set_${sub_property.ident}();
                                         result_list.push(${sub_property.camel_case}Declaration(
                                             match result.${sub_property.ident} {
                                                 Some(value) => SpecifiedValue(value),
-                                                None => CSSWideKeyword(Initial),
+                                                None => Initial,
                                             }
                                         ));
                                     }
                                 % endfor
                                 ValidOrIgnoredDeclaration
                             },
-                            None => InvalidValue,
+                            Err(()) => InvalidValue,
                         }
                     }
                 },
@@ -1788,7 +1787,7 @@ impl<T: Send + Share + Clone> ArcExperimental<T> for Arc<T> {
 }
 
 /// Fast path for the function below. Only computes new inherited styles.
-fn cascade_with_cached_declarations(applicable_declarations: &[MatchedProperty],
+fn cascade_with_cached_declarations(applicable_declarations: &[DeclarationBlock],
                                     shareable: bool,
                                     parent_style: &ComputedValues,
                                     cached_style: &ComputedValues,
@@ -1824,9 +1823,9 @@ fn cascade_with_cached_declarations(applicable_declarations: &[MatchedProperty],
                                             (*specified_value).clone(),
                                             context
                                         ),
-                                        CSSWideKeyword(Initial)
+                                        Initial
                                         => longhands::${property.ident}::get_initial_value(),
-                                        CSSWideKeyword(Inherit) => {
+                                        Inherit => {
                                             // This is a bit slow, but this is rare so it shouldn't
                                             // matter.
                                             //
@@ -1893,7 +1892,7 @@ fn cascade_with_cached_declarations(applicable_declarations: &[MatchedProperty],
 ///     this is ignored.
 ///
 /// Returns the computed values and a boolean indicating whether the result is cacheable.
-pub fn cascade(applicable_declarations: &[MatchedProperty],
+pub fn cascade(applicable_declarations: &[DeclarationBlock],
                shareable: bool,
                parent_style: Option< &ComputedValues >,
                cached_style: Option< &ComputedValues >)
@@ -1934,8 +1933,8 @@ pub fn cascade(applicable_declarations: &[MatchedProperty],
         ($style_struct_getter: ident, $property: ident, $declared_value: expr) => {
             match *$declared_value {
                 SpecifiedValue(specified_value) => specified_value,
-                CSSWideKeyword(Initial) => longhands::$property::get_initial_value(),
+                Initial => longhands::$property::get_initial_value(),
-                CSSWideKeyword(Inherit) => inherited_style.$style_struct_getter().$property.clone(),
+                Inherit => inherited_style.$style_struct_getter().$property.clone(),
             }
         };
     )
@@ -1950,8 +1949,8 @@ pub fn cascade(applicable_declarations: &[MatchedProperty],
                     context.font_size = match *value {
                         SpecifiedValue(specified_value) => computed::compute_Au_with_font_size(
                             specified_value, context.inherited_font_size),
-                        CSSWideKeyword(Initial) => longhands::font_size::get_initial_value(),
+                        Initial => longhands::font_size::get_initial_value(),
-                        CSSWideKeyword(Inherit) => context.inherited_font_size,
+                        Inherit => context.inherited_font_size,
                     }
                 }
                 ColorDeclaration(ref value) => {
@@ -2031,9 +2030,9 @@ pub fn cascade(applicable_declarations: &[MatchedProperty],
                                         (*specified_value).clone(),
                                         &context
                                     ),
-                                    CSSWideKeyword(Initial)
+                                    Initial
                                     => longhands::${property.ident}::get_initial_value(),
-                                    CSSWideKeyword(Inherit) => {
+                                    Inherit => {
                                         // This is a bit slow, but this is rare so it shouldn't
                                         // matter.
                                         //

src/components/style/selector_matching.rs

@@ -3,10 +3,12 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use std::collections::hashmap::HashMap;
-use std::ascii::StrAsciiExt;
+use std::hash::Hash;
 use std::num::div_rem;
 use sync::Arc;
 
+use url::Url;
+
 use servo_util::atom::Atom;
 use servo_util::namespace;
 use servo_util::smallvec::VecLike;
@@ -16,7 +18,7 @@ use media_queries::{Device, Screen};
 use node::{TElement, TNode};
 use properties::{PropertyDeclaration, PropertyDeclarationBlock};
 use selectors::*;
-use stylesheets::{Stylesheet, iter_style_rules};
+use stylesheets::{Stylesheet, iter_stylesheet_style_rules};
 
 pub enum StylesheetOrigin {
     UserAgentOrigin,
@@ -50,7 +52,10 @@ struct SelectorMap {
     // TODO: Tune the initial capacity of the HashMap
     id_hash: HashMap<Atom, Vec<Rule>>,
     class_hash: HashMap<Atom, Vec<Rule>>,
-    element_hash: HashMap<Atom, Vec<Rule>>,
+    local_name_hash: HashMap<Atom, Vec<Rule>>,
+    /// Same as local_name_hash, but keys are lower-cased.
+    /// For HTML elements in HTML documents.
+    lower_local_name_hash: HashMap<Atom, Vec<Rule>>,
     // For Rules that don't have ID, class, or element selectors.
     universal_rules: Vec<Rule>,
     /// Whether this hash is empty.
@@ -62,7 +67,8 @@ impl SelectorMap {
         SelectorMap {
             id_hash: HashMap::new(),
             class_hash: HashMap::new(),
-            element_hash: HashMap::new(),
+            local_name_hash: HashMap::new(),
+            lower_local_name_hash: HashMap::new(),
             universal_rules: vec!(),
             empty: true,
         }
@@ -74,7 +80,7 @@ impl SelectorMap {
     /// Sort the Rules at the end to maintain cascading order.
     fn get_all_matching_rules<E:TElement,
                               N:TNode<E>,
-                              V:VecLike<MatchedProperty>>(
+                              V:VecLike<DeclarationBlock>>(
                               &self,
                               node: &N,
                               matching_rules_list: &mut V,
@@ -111,11 +117,14 @@ impl SelectorMap {
             None => {}
         }
 
-        // HTML elements in HTML documents must be matched case-insensitively.
+        let local_name_hash = if node.is_html_element_in_html_document() {
-        // TODO(pradeep): Case-sensitivity depends on the document type.
+            &self.lower_local_name_hash
-        SelectorMap::get_matching_rules_from_hash_ignoring_case(node,
+        } else {
-                                                                &self.element_hash,
+            &self.local_name_hash
-                                                                element.get_local_name().as_slice(),
+        };
+        SelectorMap::get_matching_rules_from_hash(node,
+                                                  local_name_hash,
+                                                  element.get_local_name(),
                                                   matching_rules_list,
                                                   shareable);
 
@@ -125,12 +134,16 @@ impl SelectorMap {
                                         shareable);
 
         // Sort only the rules we just added.
-        sort::quicksort(matching_rules_list.vec_mut_slice_from(init_len));
+        sort::quicksort_by(matching_rules_list.vec_mut_slice_from(init_len), compare);
+
+        fn compare(a: &DeclarationBlock, b: &DeclarationBlock) -> Ordering {
+            (a.specificity, a.source_order).cmp(&(b.specificity, b.source_order))
+        }
     }
 
     fn get_matching_rules_from_hash<E:TElement,
                                     N:TNode<E>,
-                                    V:VecLike<MatchedProperty>>(
+                                    V:VecLike<DeclarationBlock>>(
                                     node: &N,
                                     hash: &HashMap<Atom, Vec<Rule>>,
                                     key: &Atom,
@@ -144,83 +157,45 @@ impl SelectorMap {
         }
     }
 
-    fn get_matching_rules_from_hash_ignoring_case<E:TElement,
-                                                  N:TNode<E>,
-                                                  V:VecLike<MatchedProperty>>(
-                                                  node: &N,
-                                                  hash: &HashMap<Atom, Vec<Rule>>,
-                                                  key: &str,
-                                                  matching_rules: &mut V,
-                                                  shareable: &mut bool) {
-        // FIXME: Precache the lower case version as an atom.
-        match hash.find(&Atom::from_slice(key.to_ascii_lower().as_slice())) {
-            Some(rules) => {
-                SelectorMap::get_matching_rules(node, rules.as_slice(), matching_rules, shareable)
-            }
-            None => {}
-        }
-    }
-
     /// Adds rules in `rules` that match `node` to the `matching_rules` list.
     fn get_matching_rules<E:TElement,
                           N:TNode<E>,
-                          V:VecLike<MatchedProperty>>(
+                          V:VecLike<DeclarationBlock>>(
                           node: &N,
                           rules: &[Rule],
                           matching_rules: &mut V,
                           shareable: &mut bool) {
         for rule in rules.iter() {
             if matches_compound_selector(&*rule.selector, node, shareable) {
-                // TODO(pradeep): Is the cloning inefficient?
+                matching_rules.vec_push(rule.declarations.clone());
-                matching_rules.vec_push(rule.property.clone());
             }
         }
     }
 
     /// Insert rule into the correct hash.
-    /// Order in which to try: id_hash, class_hash, element_hash, universal_rules.
+    /// Order in which to try: id_hash, class_hash, local_name_hash, universal_rules.
     fn insert(&mut self, rule: Rule) {
         self.empty = false;
 
         match SelectorMap::get_id_name(&rule) {
             Some(id_name) => {
-                match self.id_hash.find_mut(&id_name) {
+                self.id_hash.find_push(id_name, rule);
-                    Some(rules) => {
-                        rules.push(rule);
-                        return;
-                    }
-                    None => {}
-                }
-                self.id_hash.insert(id_name, vec!(rule));
                 return;
             }
             None => {}
         }
         match SelectorMap::get_class_name(&rule) {
             Some(class_name) => {
-                match self.class_hash.find_mut(&class_name) {
+                self.class_hash.find_push(class_name, rule);
-                    Some(rules) => {
-                        rules.push(rule);
-                        return;
-                    }
-                    None => {}
-                }
-                self.class_hash.insert(class_name, vec!(rule));
                 return;
             }
             None => {}
         }
 
-        match SelectorMap::get_element_name(&rule) {
+        match SelectorMap::get_local_name(&rule) {
-            Some(element_name) => {
+            Some(LocalNameSelector { name, lower_name }) => {
-                match self.element_hash.find_mut(&element_name) {
+                self.local_name_hash.find_push(name, rule.clone());
-                    Some(rules) => {
+                self.lower_local_name_hash.find_push(lower_name, rule);
-                        rules.push(rule);
-                        return;
-                    }
-                    None => {}
-                }
-                self.element_hash.insert(element_name, vec!(rule));
                 return;
             }
             None => {}
@@ -258,14 +233,12 @@ impl SelectorMap {
     }
 
     /// Retrieve the name if it is a type selector, or None otherwise.
-    fn get_element_name(rule: &Rule) -> Option<Atom> {
+    fn get_local_name(rule: &Rule) -> Option<LocalNameSelector> {
         let simple_selector_sequence = &rule.selector.simple_selectors;
         for ss in simple_selector_sequence.iter() {
             match *ss {
-                // HTML elements in HTML documents must be matched case-insensitively
-                // TODO: case-sensitivity depends on the document type
                 LocalNameSelector(ref name) => {
-                    return Some(Atom::from_slice(name.as_slice().to_ascii_lower().as_slice()));
+                    return Some(name.clone())
                 }
                 _ => {}
             }
@@ -284,12 +257,19 @@ pub struct Stylist {
 impl Stylist {
     #[inline]
     pub fn new() -> Stylist {
-        Stylist {
+        let mut stylist = Stylist {
             element_map: PerPseudoElementSelectorMap::new(),
             before_map: PerPseudoElementSelectorMap::new(),
             after_map: PerPseudoElementSelectorMap::new(),
             rules_source_order: 0u,
-        }
+        };
+        let ua_stylesheet = Stylesheet::from_bytes(
+            include_bin!("user-agent.css"),
+            Url::parse("chrome:///user-agent.css").unwrap(),
+            None,
+            None);
+        stylist.add_stylesheet(ua_stylesheet, UserAgentOrigin);
+        stylist
     }
 
     pub fn add_stylesheet(&mut self, stylesheet: Stylesheet, origin: StylesheetOrigin) {
@@ -325,7 +305,7 @@ impl Stylist {
                         };
                         map.$priority.insert(Rule {
                                 selector: selector.compound_selectors.clone(),
-                                property: MatchedProperty {
+                                declarations: DeclarationBlock {
                                     specificity: selector.specificity,
                                     declarations: $style_rule.declarations.$priority.clone(),
                                     source_order: rules_source_order,
@@ -337,7 +317,7 @@ impl Stylist {
         );
 
         let device = &Device { media_type: Screen };  // TODO, use Print when printing
-        iter_style_rules(stylesheet.rules.as_slice(), device, |style_rule| {
+        iter_stylesheet_style_rules(&stylesheet, device, |style_rule| {
             append!(style_rule, normal);
             append!(style_rule, important);
             rules_source_order += 1;
@@ -353,7 +333,7 @@ impl Stylist {
     /// in `css::matching::PrivateMatchMethods::candidate_element_allows_for_style_sharing`.
     pub fn push_applicable_declarations<E:TElement,
                                         N:TNode<E>,
-                                        V:VecLike<MatchedProperty>>(
+                                        V:VecLike<DeclarationBlock>>(
                                         &self,
                                         element: &N,
                                         style_attribute: Option<&PropertyDeclarationBlock>,
@@ -382,7 +362,7 @@ impl Stylist {
         // Step 2: Normal style attributes.
         style_attribute.map(|sa| {
             shareable = false;
-            applicable_declarations.vec_push(MatchedProperty::from_declarations(sa.normal.clone()))
+            applicable_declarations.vec_push(DeclarationBlock::from_declarations(sa.normal.clone()))
         });
 
         // Step 3: Author-supplied `!important` rules.
@@ -393,7 +373,7 @@ impl Stylist {
         // Step 4: `!important` style attributes.
         style_attribute.map(|sa| {
             shareable = false;
-            applicable_declarations.vec_push(MatchedProperty::from_declarations(sa.important.clone()))
+            applicable_declarations.vec_push(DeclarationBlock::from_declarations(sa.important.clone()))
         });
 
         // Step 5: User and UA `!important` rules.
@@ -446,22 +426,22 @@ struct Rule {
     // that it matches. Selector contains an owned vector (through
     // CompoundSelector) and we want to avoid the allocation.
     selector: Arc<CompoundSelector>,
-    property: MatchedProperty,
+    declarations: DeclarationBlock,
 }
 
 /// A property declaration together with its precedence among rules of equal specificity so that
 /// we can sort them.
 #[deriving(Clone)]
-pub struct MatchedProperty {
+pub struct DeclarationBlock {
     pub declarations: Arc<Vec<PropertyDeclaration>>,
     source_order: uint,
     specificity: u32,
 }
 
-impl MatchedProperty {
+impl DeclarationBlock {
     #[inline]
-    pub fn from_declarations(declarations: Arc<Vec<PropertyDeclaration>>) -> MatchedProperty {
+    pub fn from_declarations(declarations: Arc<Vec<PropertyDeclaration>>) -> DeclarationBlock {
-        MatchedProperty {
+        DeclarationBlock {
             declarations: declarations,
             source_order: 0,
             specificity: 0,
@@ -469,34 +449,12 @@ impl MatchedProperty {
     }
 }
 
-impl PartialEq for MatchedProperty {
+pub fn matches<E:TElement, N:TNode<E>>(selector_list: &SelectorList, element: &N) -> bool {
-    #[inline]
+    get_selector_list_selectors(selector_list).iter().any(|selector|
-    fn eq(&self, other: &MatchedProperty) -> bool {
+        selector.pseudo_element.is_none() &&
-        let this_rank = (self.specificity, self.source_order);
+        matches_compound_selector(&*selector.compound_selectors, element, &mut false))
-        let other_rank = (other.specificity, other.source_order);
-        this_rank == other_rank
-    }
 }
 
-impl Eq for MatchedProperty {}
-
-impl PartialOrd for MatchedProperty {
-    #[inline]
-    fn partial_cmp(&self, other: &MatchedProperty) -> Option<Ordering> {
-        let this_rank = (self.specificity, self.source_order);
-        let other_rank = (other.specificity, other.source_order);
-        this_rank.partial_cmp(&other_rank)
-    }
-}
-
-impl Ord for MatchedProperty {
-    #[inline]
-    fn cmp(&self, other: &MatchedProperty) -> Ordering {
-        let this_rank = (self.specificity, self.source_order);
-        let other_rank = (other.specificity, other.source_order);
-        this_rank.cmp(&other_rank)
-    }
-}
 
 /// Determines whether the given element matches the given single or compound selector.
 ///
@@ -504,7 +462,7 @@ impl Ord for MatchedProperty {
 /// `shareable` to false unless you are willing to update the style sharing logic. Otherwise things
 /// will almost certainly break as nodes will start mistakenly sharing styles. (See the code in
 /// `main/css/matching.rs`.)
-pub fn matches_compound_selector<E:TElement,
+fn matches_compound_selector<E:TElement,
                              N:TNode<E>>(
                              selector: &CompoundSelector,
                              element: &N,
@@ -645,11 +603,10 @@ fn matches_simple_selector<E:TElement,
                            shareable: &mut bool)
                            -> bool {
     match *selector {
-        // TODO: case-sensitivity depends on the document type
+        LocalNameSelector(LocalNameSelector { ref name, ref lower_name }) => {
-        // TODO: intern element names
+            let name = if element.is_html_element_in_html_document() { lower_name } else { name };
-        LocalNameSelector(ref name) => {
             let element = element.as_element();
-            element.get_local_name().as_slice().eq_ignore_ascii_case(name.as_slice())
+            element.get_local_name() == name
         }
 
         NamespaceSelector(ref namespace) => {
@@ -934,11 +891,30 @@ fn matches_last_child<E:TElement,N:TNode<E>>(element: &N) -> bool {
 }
 
 
+trait FindPush<K, V> {
+    fn find_push(&mut self, key: K, value: V);
+}
+
+impl<K: Eq + Hash, V> FindPush<K, V> for HashMap<K, Vec<V>> {
+    fn find_push(&mut self, key: K, value: V) {
+        match self.find_mut(&key) {
+            Some(vec) => {
+                vec.push(value);
+                return
+            }
+            None => {}
+        }
+        self.insert(key, vec![value]);
+    }
+}
+
+
 #[cfg(test)]
 mod tests {
     use servo_util::atom::Atom;
     use sync::Arc;
-    use super::{MatchedProperty, Rule, SelectorMap};
+    use super::{DeclarationBlock, Rule, SelectorMap};
+    use selectors::LocalNameSelector;
 
     /// Helper method to get some Rules from selector strings.
     /// Each sublist of the result contains the Rules for one StyleRule.
@@ -949,11 +925,11 @@ mod tests {
 
         let namespaces = NamespaceMap::new();
         css_selectors.iter().enumerate().map(|(i, selectors)| {
-            parse_selector_list(tokenize(*selectors).map(|(c, _)| c).collect(), &namespaces)
+            parse_selector_list(tokenize(*selectors).map(|(c, _)| c), &namespaces)
             .unwrap().move_iter().map(|s| {
                 Rule {
                     selector: s.compound_selectors.clone(),
-                    property: MatchedProperty {
+                    declarations: DeclarationBlock {
                         specificity: s.specificity,
                         declarations: Arc::new(vec!()),
                         source_order: i,
@@ -966,9 +942,10 @@ mod tests {
     #[test]
     fn test_rule_ordering_same_specificity(){
         let rules_list = get_mock_rules(["a.intro", "img.sidebar"]);
-        let rule1 = rules_list[0][0].clone();
+        let a = &rules_list[0][0].declarations;
-        let rule2 = rules_list[1][0].clone();
+        let b = &rules_list[1][0].declarations;
-        assert!(rule1.property < rule2.property, "The rule that comes later should win.");
+        assert!((a.specificity, a.source_order).cmp(&(b.specificity, b.source_order)) == Less,
+                "The rule that comes later should win.");
     }
 
     #[test]
@@ -986,12 +963,18 @@ mod tests {
     }
 
     #[test]
-    fn test_get_element_name(){
+    fn test_get_local_name(){
         let rules_list = get_mock_rules(["img.foo", "#top", "IMG", "ImG"]);
-        assert_eq!(SelectorMap::get_element_name(&rules_list[0][0]), Some(Atom::from_slice("img")));
+        let check = |i, names: Option<(&str, &str)>| {
-        assert_eq!(SelectorMap::get_element_name(&rules_list[1][0]), None);
+            assert!(SelectorMap::get_local_name(&rules_list[i][0])
-        assert_eq!(SelectorMap::get_element_name(&rules_list[2][0]), Some(Atom::from_slice("img")));
+                    == names.map(|(name, lower_name)| LocalNameSelector {
-        assert_eq!(SelectorMap::get_element_name(&rules_list[3][0]), Some(Atom::from_slice("img")));
+                            name: Atom::from_slice(name),
+                            lower_name: Atom::from_slice(lower_name) }))
+        };
+        check(0, Some(("img", "img")));
+        check(1, None);
+        check(2, Some(("IMG", "img")));
+        check(3, Some(("ImG", "img")));
     }
 
     #[test]
@@ -999,9 +982,9 @@ mod tests {
         let rules_list = get_mock_rules([".intro.foo", "#top"]);
         let mut selector_map = SelectorMap::new();
         selector_map.insert(rules_list[1][0].clone());
-        assert_eq!(1, selector_map.id_hash.find(&Atom::from_slice("top")).unwrap()[0].property.source_order);
+        assert_eq!(1, selector_map.id_hash.find(&Atom::from_slice("top")).unwrap()[0].declarations.source_order);
         selector_map.insert(rules_list[0][0].clone());
-        assert_eq!(0, selector_map.class_hash.find(&Atom::from_slice("intro")).unwrap()[0].property.source_order);
+        assert_eq!(0, selector_map.class_hash.find(&Atom::from_slice("intro")).unwrap()[0].declarations.source_order);
         assert!(selector_map.class_hash.find(&Atom::from_slice("foo")).is_none());
     }
 }

src/components/style/selectors.rs

@@ -3,12 +3,11 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use std::{cmp, iter};
-use std::ascii::StrAsciiExt;
+use std::ascii::{StrAsciiExt, OwnedStrAsciiExt};
-use std::vec;
 use sync::Arc;
 
 use cssparser::ast::*;
-use cssparser::parse_nth;
+use cssparser::{tokenize, parse_nth};
 
 use servo_util::atom::Atom;
 use servo_util::namespace::Namespace;
@@ -59,7 +58,7 @@ pub enum Combinator {
 pub enum SimpleSelector {
     IDSelector(Atom),
     ClassSelector(Atom),
-    LocalNameSelector(Atom),
+    LocalNameSelector(LocalNameSelector),
     NamespaceSelector(Namespace),
 
     // Attribute selectors
@@ -93,6 +92,12 @@ pub enum SimpleSelector {
     // ...
 }
 
+#[deriving(PartialEq, Clone)]
+pub struct LocalNameSelector {
+    pub name: Atom,
+    pub lower_name: Atom,
+}
+
 #[deriving(PartialEq, Clone)]
 pub struct AttrSelector {
     pub name: String,
@@ -107,21 +112,31 @@ pub enum NamespaceConstraint {
 }
 
 
-type Iter = iter::Peekable<ComponentValue, vec::MoveItems<ComponentValue>>;
+pub fn parse_selector_list_from_str(input: &str) -> Result<SelectorList, ()> {
+    let namespaces = NamespaceMap::new();
+    let iter = tokenize(input).map(|(token, _)| token);
+    parse_selector_list(iter, &namespaces).map(|s| SelectorList { selectors: s })
+}
 
+/// Re-exported to script, but opaque.
+pub struct SelectorList {
+    selectors: Vec<Selector>
+}
+
+/// Public to the style crate, but not re-exported to script
+pub fn get_selector_list_selectors<'a>(selector_list: &'a SelectorList) -> &'a [Selector] {
+    selector_list.selectors.as_slice()
+}
 
 /// Parse a comma-separated list of Selectors.
 /// aka Selector Group in http://www.w3.org/TR/css3-selectors/#grouping
 ///
 /// Return the Selectors or None if there is an invalid selector.
-pub fn parse_selector_list(input: Vec<ComponentValue>, namespaces: &NamespaceMap)
+pub fn parse_selector_list<I: Iterator<ComponentValue>>(
-                           -> Option<Vec<Selector>> {
+                           iter: I, namespaces: &NamespaceMap)
-    let iter = &mut input.move_iter().peekable();
+                           -> Result<Vec<Selector>, ()> {
-    let first = match parse_selector(iter, namespaces) {
+    let iter = &mut iter.peekable();
-        None => return None,
+    let mut results = vec![try!(parse_selector(iter, namespaces))];
-        Some(result) => result
-    };
-    let mut results = vec!(first);
 
     loop {
         skip_whitespace(iter);
@@ -130,29 +145,25 @@ pub fn parse_selector_list(input: Vec<ComponentValue>, namespaces: &NamespaceMap
             Some(&Comma) => {
                 iter.next();
             }
-            _ => return None,
+            _ => return Err(()),
         }
-        match parse_selector(iter, namespaces) {
+        results.push(try!(parse_selector(iter, namespaces)));
-            Some(selector) => results.push(selector),
-            None => return None,
     }
-    }
+    Ok(results)
-    Some(results)
 }
 
 
+type Iter<I> = iter::Peekable<ComponentValue, I>;
+
 /// Build up a Selector.
 /// selector : simple_selector_sequence [ combinator simple_selector_sequence ]* ;
 ///
-/// None means invalid selector.
+/// `Err` means invalid selector.
-fn parse_selector(iter: &mut Iter, namespaces: &NamespaceMap)
+fn parse_selector<I: Iterator<ComponentValue>>(
-                  -> Option<Selector> {
+                  iter: &mut Iter<I>, namespaces: &NamespaceMap)
-    let (first, pseudo_element) = match parse_simple_selectors(iter, namespaces) {
+                  -> Result<Selector, ()> {
-        None => return None,
+    let (first, mut pseudo_element) = try!(parse_simple_selectors(iter, namespaces));
-        Some(result) => result
-    };
     let mut compound = CompoundSelector{ simple_selectors: first, next: None };
-    let mut pseudo_element = pseudo_element;
 
     while pseudo_element.is_none() {
         let any_whitespace = skip_whitespace(iter);
@@ -164,21 +175,17 @@ fn parse_selector(iter: &mut Iter, namespaces: &NamespaceMap)
             Some(&Delim('~')) => { iter.next(); LaterSibling },
             Some(_) => {
                 if any_whitespace { Descendant }
-                else { return None }
+                else { return Err(()) }
             }
         };
-        match parse_simple_selectors(iter, namespaces) {
+        let (simple_selectors, pseudo) = try!(parse_simple_selectors(iter, namespaces));
-            None => return None,
-            Some((simple_selectors, pseudo)) => {
         compound = CompoundSelector {
             simple_selectors: simple_selectors,
             next: Some((box compound, combinator))
         };
         pseudo_element = pseudo;
     }
-        }
+    Ok(Selector {
-    }
-    Some(Selector {
         specificity: compute_specificity(&compound, &pseudo_element),
         compound_selectors: Arc::new(compound),
         pseudo_element: pseudo_element,
@@ -245,43 +252,39 @@ fn compute_specificity(mut selector: &CompoundSelector,
 /// : [ type_selector | universal ] [ HASH | class | attrib | pseudo | negation ]*
 /// | [ HASH | class | attrib | pseudo | negation ]+
 ///
-/// None means invalid selector
+/// `Err(())` means invalid selector
-fn parse_simple_selectors(iter: &mut Iter, namespaces: &NamespaceMap)
+fn parse_simple_selectors<I: Iterator<ComponentValue>>(
-                           -> Option<(Vec<SimpleSelector>, Option<PseudoElement>)> {
+                          iter: &mut Iter<I>, namespaces: &NamespaceMap)
+                          -> Result<(Vec<SimpleSelector>, Option<PseudoElement>), ()> {
     let mut empty = true;
-    let mut simple_selectors = match parse_type_selector(iter, namespaces) {
+    let mut simple_selectors = match try!(parse_type_selector(iter, namespaces)) {
-        InvalidTypeSelector => return None,
+        None => vec![],
-        NotATypeSelector => vec!(),
+        Some(s) => { empty = false; s }
-        TypeSelector(s) => { empty = false; s }
     };
 
     let mut pseudo_element = None;
     loop {
-        match parse_one_simple_selector(iter, namespaces, /* inside_negation = */ false) {
+        match try!(parse_one_simple_selector(iter, namespaces, /* inside_negation = */ false)) {
-            InvalidSimpleSelector => return None,
+            None => break,
-            NotASimpleSelector => break,
+            Some(SimpleSelectorResult(s)) => { simple_selectors.push(s); empty = false },
-            SimpleSelectorResult(s) => { simple_selectors.push(s); empty = false },
+            Some(PseudoElementResult(p)) => { pseudo_element = Some(p); empty = false; break },
-            PseudoElementResult(p) => { pseudo_element = Some(p); empty = false; break },
         }
     }
-    if empty { None }  // An empty selector is invalid
+    if empty { Err(()) }  // An empty selector is invalid
-    else { Some((simple_selectors, pseudo_element)) }
+    else { Ok((simple_selectors, pseudo_element)) }
 }
 
 
-enum TypeSelectorParseResult {
+/// * `Err(())`: Invalid selector, abort
-    InvalidTypeSelector,
+/// * `Ok(None)`: Not a type selector, could be something else. `iter` was not consumed.
-    NotATypeSelector,
+/// * `Ok(Some(vec))`: Length 0 (`*|*`), 1 (`*|E` or `ns|*`) or 2 (`|E` or `ns|E`)
-    TypeSelector(Vec<SimpleSelector>),  // Length 0 (*|*), 1 (*|E or ns|*) or 2 (|E or ns|E)
+fn parse_type_selector<I: Iterator<ComponentValue>>(
-}
+                       iter: &mut Iter<I>, namespaces: &NamespaceMap)
-
+                       -> Result<Option<Vec<SimpleSelector>>, ()> {
-fn parse_type_selector(iter: &mut Iter, namespaces: &NamespaceMap)
-                       -> TypeSelectorParseResult {
     skip_whitespace(iter);
-    match parse_qualified_name(iter, /* in_attr_selector = */ false, namespaces) {
+    match try!(parse_qualified_name(iter, /* in_attr_selector = */ false, namespaces)) {
-        InvalidQualifiedName => InvalidTypeSelector,
+        None => Ok(None),
-        NotAQualifiedName => NotATypeSelector,
+        Some((namespace, local_name)) => {
-        QualifiedName(namespace, local_name) => {
             let mut simple_selectors = vec!();
             match namespace {
                 SpecificNamespace(ns) => simple_selectors.push(NamespaceSelector(ns)),
@@ -289,121 +292,115 @@ fn parse_type_selector(iter: &mut Iter, namespaces: &NamespaceMap)
             }
             match local_name {
                 Some(name) => {
-                    let name_atom = Atom::from_slice(name.as_slice());
+                    simple_selectors.push(LocalNameSelector(LocalNameSelector {
-                    simple_selectors.push(LocalNameSelector(name_atom))
+                        name: Atom::from_slice(name.as_slice()),
+                        lower_name: Atom::from_slice(name.into_ascii_lower().as_slice())
+                    }))
                 }
                 None => (),
             }
-            TypeSelector(simple_selectors)
+            Ok(Some(simple_selectors))
         }
     }
 }
 
 
 enum SimpleSelectorParseResult {
-    InvalidSimpleSelector,
-    NotASimpleSelector,
     SimpleSelectorResult(SimpleSelector),
     PseudoElementResult(PseudoElement),
 }
 
-// Parse a simple selector other than a type selector
+/// Parse a simple selector other than a type selector.
-fn parse_one_simple_selector(iter: &mut Iter, namespaces: &NamespaceMap, inside_negation: bool)
+///
-                         -> SimpleSelectorParseResult {
+/// * `Err(())`: Invalid selector, abort
+/// * `Ok(None)`: Not a simple selector, could be something else. `iter` was not consumed.
+/// * `Ok(Some(_))`: Parsed a simple selector or pseudo-element
+fn parse_one_simple_selector<I: Iterator<ComponentValue>>(
+                             iter: &mut Iter<I>, namespaces: &NamespaceMap, inside_negation: bool)
+                             -> Result<Option<SimpleSelectorParseResult>, ()> {
     match iter.peek() {
         Some(&IDHash(_)) => match iter.next() {
-            Some(IDHash(id)) => SimpleSelectorResult(IDSelector(Atom::from_slice(id.as_slice()))),
+            Some(IDHash(id)) => Ok(Some(SimpleSelectorResult(
+                IDSelector(Atom::from_slice(id.as_slice()))))),
             _ => fail!("Implementation error, this should not happen."),
         },
         Some(&Delim('.')) => {
             iter.next();
             match iter.next() {
-                Some(Ident(class)) => SimpleSelectorResult(ClassSelector(Atom::from_slice(class.as_slice()))),
+                Some(Ident(class)) => Ok(Some(SimpleSelectorResult(
-                _ => InvalidSimpleSelector,
+                    ClassSelector(Atom::from_slice(class.as_slice()))))),
+                _ => Err(()),
             }
         }
         Some(&SquareBracketBlock(_)) => match iter.next() {
             Some(SquareBracketBlock(content))
-            => match parse_attribute_selector(content, namespaces) {
+            => Ok(Some(SimpleSelectorResult(try!(parse_attribute_selector(content, namespaces))))),
-                None => InvalidSimpleSelector,
-                Some(simple_selector) => SimpleSelectorResult(simple_selector),
-            },
             _ => fail!("Implementation error, this should not happen."),
         },
         Some(&Colon) => {
             iter.next();
             match iter.next() {
                 Some(Ident(name)) => match parse_simple_pseudo_class(name.as_slice()) {
-                    None => {
+                    Err(()) => {
                         match name.as_slice().to_ascii_lower().as_slice() {
                             // Supported CSS 2.1 pseudo-elements only.
                             // ** Do not add to this list! **
-                            "before" => PseudoElementResult(Before),
+                            "before" => Ok(Some(PseudoElementResult(Before))),
-                            "after" => PseudoElementResult(After),
+                            "after" => Ok(Some(PseudoElementResult(After))),
 //                            "first-line" => PseudoElementResult(FirstLine),
 //                            "first-letter" => PseudoElementResult(FirstLetter),
-                            _ => InvalidSimpleSelector
+                            _ => Err(())
                         }
                     },
-                    Some(result) => SimpleSelectorResult(result),
+                    Ok(result) => Ok(Some(SimpleSelectorResult(result))),
-                },
-                Some(Function(name, arguments)) => match parse_functional_pseudo_class(
-                        name, arguments, namespaces, inside_negation) {
-                    None => InvalidSimpleSelector,
-                    Some(simple_selector) => SimpleSelectorResult(simple_selector),
                 },
+                Some(Function(name, arguments))
+                => Ok(Some(SimpleSelectorResult(try!(parse_functional_pseudo_class(
+                        name, arguments, namespaces, inside_negation))))),
                 Some(Colon) => {
                     match iter.next() {
-                        Some(Ident(name)) => match parse_pseudo_element(name) {
+                        Some(Ident(name))
-                            Some(pseudo_element) => PseudoElementResult(pseudo_element),
+                        => Ok(Some(PseudoElementResult(try!(parse_pseudo_element(name))))),
-                            _ => InvalidSimpleSelector,
+                        _ => Err(()),
-                        },
-                        _ => InvalidSimpleSelector,
                     }
                 }
-                _ => InvalidSimpleSelector,
+                _ => Err(()),
             }
         }
-        _ => NotASimpleSelector,
+        _ => Ok(None),
     }
 }
 
 
-enum QualifiedNameParseResult {
+/// * `Err(())`: Invalid selector, abort
-    InvalidQualifiedName,
+/// * `Ok(None)`: Not a simple selector, could be something else. `iter` was not consumed.
-    NotAQualifiedName,
+/// * `Ok(Some((namespace, local_name)))`: `None` for the local name means a `*` universal selector
-    // Namespace URL, local name. None means '*'
+fn parse_qualified_name<I: Iterator<ComponentValue>>(
-    QualifiedName(NamespaceConstraint, Option<String>)
+                        iter: &mut Iter<I>, in_attr_selector: bool, namespaces: &NamespaceMap)
-}
+                        -> Result<Option<(NamespaceConstraint, Option<String>)>, ()> {
-
+    let default_namespace = |local_name| {
-fn parse_qualified_name(iter: &mut Iter, in_attr_selector: bool, namespaces: &NamespaceMap)
+        let namespace = match namespaces.default {
-                       -> QualifiedNameParseResult {
-    #[inline]
-    fn default_namespace(namespaces: &NamespaceMap, local_name: Option<String>)
-                         -> QualifiedNameParseResult {
-        QualifiedName(match namespaces.default {
             Some(ref ns) => SpecificNamespace(ns.clone()),
             None => AnyNamespace,
-        }, local_name)
+        };
-    }
+        Ok(Some((namespace, local_name)))
+    };
 
-    #[inline]
+    let explicit_namespace = |iter: &mut Iter<I>, namespace| {
-    fn explicit_namespace(iter: &mut Iter, in_attr_selector: bool, namespace: NamespaceConstraint)
-                         -> QualifiedNameParseResult {
         assert!(iter.next() == Some(Delim('|')),
                 "Implementation error, this should not happen.");
         match iter.peek() {
             Some(&Delim('*')) if !in_attr_selector => {
                 iter.next();
-                QualifiedName(namespace, None)
+                Ok(Some((namespace, None)))
             },
             Some(&Ident(_)) => {
                 let local_name = get_next_ident(iter);
-                QualifiedName(namespace, Some(local_name))
+                Ok(Some((namespace, Some(local_name))))
             },
-            _ => InvalidQualifiedName,
+            _ => Err(()),
-        }
         }
+    };
 
     match iter.peek() {
         Some(&Ident(_)) => {
@@ -411,40 +408,39 @@ fn parse_qualified_name(iter: &mut Iter, in_attr_selector: bool, namespaces: &Na
             match iter.peek() {
                 Some(&Delim('|')) => {
                     let namespace = match namespaces.prefix_map.find(&value) {
-                        None => return InvalidQualifiedName,  // Undeclared namespace prefix
+                        None => return Err(()),  // Undeclared namespace prefix
                         Some(ref ns) => (*ns).clone(),
                     };
-                    explicit_namespace(iter, in_attr_selector, SpecificNamespace(namespace))
+                    explicit_namespace(iter, SpecificNamespace(namespace))
                 },
-                _ if in_attr_selector => QualifiedName(
+                _ if in_attr_selector => Ok(Some(
-                    SpecificNamespace(namespace::Null), Some(value)),
+                    (SpecificNamespace(namespace::Null), Some(value)))),
-                _ => default_namespace(namespaces, Some(value)),
+                _ => default_namespace(Some(value)),
             }
         },
         Some(&Delim('*')) => {
             iter.next();  // Consume '*'
             match iter.peek() {
-                Some(&Delim('|')) => explicit_namespace(iter, in_attr_selector, AnyNamespace),
+                Some(&Delim('|')) => explicit_namespace(iter, AnyNamespace),
                 _ => {
-                    if !in_attr_selector { default_namespace(namespaces, None) }
+                    if !in_attr_selector { default_namespace(None) }
-                    else { InvalidQualifiedName }
+                    else { Err(()) }
                 },
             }
         },
-        Some(&Delim('|')) => explicit_namespace(
+        Some(&Delim('|')) => explicit_namespace(iter, SpecificNamespace(namespace::Null)),
-            iter, in_attr_selector, SpecificNamespace(namespace::Null)),
+        _ => Ok(None),
-        _ => NotAQualifiedName,
     }
 }
 
 
 fn parse_attribute_selector(content: Vec<ComponentValue>, namespaces: &NamespaceMap)
-                            -> Option<SimpleSelector> {
+                            -> Result<SimpleSelector, ()> {
     let iter = &mut content.move_iter().peekable();
-    let attr = match parse_qualified_name(iter, /* in_attr_selector = */ true, namespaces) {
+    let attr = match try!(parse_qualified_name(iter, /* in_attr_selector = */ true, namespaces)) {
-        InvalidQualifiedName | NotAQualifiedName => return None,
+        None => return Err(()),
-        QualifiedName(_, None) => fail!("Implementation error, this should not happen."),
+        Some((_, None)) => fail!("Implementation error, this should not happen."),
-        QualifiedName(namespace, Some(local_name)) => AttrSelector {
+        Some((namespace, Some(local_name))) => AttrSelector {
             namespace: namespace,
             lower_name: local_name.as_slice().to_ascii_lower(),
             name: local_name,
@@ -456,7 +452,7 @@ fn parse_attribute_selector(content: Vec<ComponentValue>, namespaces: &Namespace
         skip_whitespace(iter);
         match iter.next() {
             Some(Ident(value)) | Some(String(value)) => value,
-            _ => return None,
+            _ => return Err(())
         }
     }};)
     let result = match iter.next() {
@@ -471,93 +467,92 @@ fn parse_attribute_selector(content: Vec<ComponentValue>, namespaces: &Namespace
         Some(PrefixMatch) => AttrPrefixMatch(attr, (get_value!())),  // [foo^=bar]
         Some(SubstringMatch) => AttrSubstringMatch(attr, (get_value!())),  // [foo*=bar]
         Some(SuffixMatch) => AttrSuffixMatch(attr, (get_value!())),  // [foo$=bar]
-        _ => return None
+        _ => return Err(())
     };
     skip_whitespace(iter);
-    if iter.next().is_none() { Some(result) } else { None }
+    if iter.next().is_none() { Ok(result) } else { Err(()) }
 }
 
 
-fn parse_simple_pseudo_class(name: &str) -> Option<SimpleSelector> {
+fn parse_simple_pseudo_class(name: &str) -> Result<SimpleSelector, ()> {
     match name.to_ascii_lower().as_slice() {
-        "any-link" => Some(AnyLink),
+        "any-link" => Ok(AnyLink),
-        "link" => Some(Link),
+        "link" => Ok(Link),
-        "visited" => Some(Visited),
+        "visited" => Ok(Visited),
-        "hover" => Some(Hover),
+        "hover" => Ok(Hover),
-        "disabled" => Some(Disabled),
+        "disabled" => Ok(Disabled),
-        "enabled" => Some(Enabled),
+        "enabled" => Ok(Enabled),
-        "first-child" => Some(FirstChild),
+        "first-child" => Ok(FirstChild),
-        "last-child"  => Some(LastChild),
+        "last-child"  => Ok(LastChild),
-        "only-child"  => Some(OnlyChild),
+        "only-child"  => Ok(OnlyChild),
-        "root" => Some(Root),
+        "root" => Ok(Root),
-        "first-of-type" => Some(FirstOfType),
+        "first-of-type" => Ok(FirstOfType),
-        "last-of-type"  => Some(LastOfType),
+        "last-of-type"  => Ok(LastOfType),
-        "only-of-type"  => Some(OnlyOfType),
+        "only-of-type"  => Ok(OnlyOfType),
-//        "empty" => Some(Empty),
+//        "empty" => Ok(Empty),
-        _ => None
+        _ => Err(())
     }
 }
 
 
 fn parse_functional_pseudo_class(name: String, arguments: Vec<ComponentValue>,
                                  namespaces: &NamespaceMap, inside_negation: bool)
-                                 -> Option<SimpleSelector> {
+                                 -> Result<SimpleSelector, ()> {
     match name.as_slice().to_ascii_lower().as_slice() {
 //        "lang" => parse_lang(arguments),
         "nth-child"        => parse_nth(arguments.as_slice()).map(|(a, b)| NthChild(a, b)),
         "nth-last-child"   => parse_nth(arguments.as_slice()).map(|(a, b)| NthLastChild(a, b)),
         "nth-of-type"      => parse_nth(arguments.as_slice()).map(|(a, b)| NthOfType(a, b)),
         "nth-last-of-type" => parse_nth(arguments.as_slice()).map(|(a, b)| NthLastOfType(a, b)),
-        "not" => if inside_negation { None } else { parse_negation(arguments, namespaces) },
+        "not" => if inside_negation { Err(()) } else { parse_negation(arguments, namespaces) },
-        _ => None
+        _ => Err(())
     }
 }
 
 
-fn parse_pseudo_element(name: String) -> Option<PseudoElement> {
+fn parse_pseudo_element(name: String) -> Result<PseudoElement, ()> {
     match name.as_slice().to_ascii_lower().as_slice() {
         // All supported pseudo-elements
-        "before" => Some(Before),
+        "before" => Ok(Before),
-        "after" => Some(After),
+        "after" => Ok(After),
 //        "first-line" => Some(FirstLine),
 //        "first-letter" => Some(FirstLetter),
-        _ => None
+        _ => Err(())
     }
 }
 
 
-//fn parse_lang(arguments: vec!(ComponentValue)) -> Option<SimpleSelector> {
+//fn parse_lang(arguments: vec!(ComponentValue)) -> Result<SimpleSelector, ()> {
 //    let mut iter = arguments.move_skip_whitespace();
 //    match iter.next() {
 //        Some(Ident(value)) => {
 //            if "" == value || iter.next().is_some() { None }
-//            else { Some(Lang(value)) }
+//            else { Ok(Lang(value)) }
 //        },
-//        _ => None,
+//        _ => Err(()),
 //    }
 //}
 
 
-// Level 3: Parse ONE simple_selector
+/// Level 3: Parse **one** simple_selector
 fn parse_negation(arguments: Vec<ComponentValue>, namespaces: &NamespaceMap)
-                  -> Option<SimpleSelector> {
+                  -> Result<SimpleSelector, ()> {
     let iter = &mut arguments.move_iter().peekable();
-    Some(Negation(match parse_type_selector(iter, namespaces) {
+    match try!(parse_type_selector(iter, namespaces)) {
-        InvalidTypeSelector => return None,
+        Some(type_selector) => Ok(Negation(type_selector)),
-        TypeSelector(s) => s,
+        None => {
-        NotATypeSelector => {
+            match try!(parse_one_simple_selector(iter, namespaces, /* inside_negation = */ true)) {
-            match parse_one_simple_selector(iter, namespaces, /* inside_negation = */ true) {
+                Some(SimpleSelectorResult(simple_selector)) => Ok(Negation(vec![simple_selector])),
-                SimpleSelectorResult(s) => vec!(s),
+                _ => Err(())
-                _ => return None
             }
         },
-    }))
+    }
 }
 
 
 /// Assuming the next token is an ident, consume it and return its value
 #[inline]
-fn get_next_ident(iter: &mut Iter) -> String {
+fn get_next_ident<I: Iterator<ComponentValue>>(iter: &mut Iter<I>) -> String {
     match iter.next() {
         Some(Ident(value)) => value,
         _ => fail!("Implementation error, this should not happen."),
@@ -566,7 +561,7 @@ fn get_next_ident(iter: &mut Iter) -> String {
 
 
 #[inline]
-fn skip_whitespace(iter: &mut Iter) -> bool {
+fn skip_whitespace<I: Iterator<ComponentValue>>(iter: &mut Iter<I>) -> bool {
     let mut any_whitespace = false;
     loop {
         if iter.peek() != Some(&WhiteSpace) { return any_whitespace }
@@ -585,14 +580,12 @@ mod tests {
     use namespaces::NamespaceMap;
     use super::*;
 
-    fn parse(input: &str) -> Option<Vec<Selector>> {
+    fn parse(input: &str) -> Result<Vec<Selector>, ()> {
         parse_ns(input, &NamespaceMap::new())
     }
 
-    fn parse_ns(input: &str, namespaces: &NamespaceMap) -> Option<Vec<Selector>> {
+    fn parse_ns(input: &str, namespaces: &NamespaceMap) -> Result<Vec<Selector>, ()> {
-        parse_selector_list(
+        parse_selector_list(cssparser::tokenize(input).map(|(v, _)| v), namespaces)
-            cssparser::tokenize(input).map(|(v, _)| v).collect(),
-            namespaces)
     }
 
     fn specificity(a: u32, b: u32, c: u32) -> u32 {
@@ -601,16 +594,18 @@ mod tests {
 
     #[test]
     fn test_parsing() {
-        assert!(parse("") == None)
+        assert!(parse("") == Err(()))
-        assert!(parse("e") == Some(vec!(Selector{
+        assert!(parse("EeÉ") == Ok(vec!(Selector {
             compound_selectors: Arc::new(CompoundSelector {
-                simple_selectors: vec!(LocalNameSelector(Atom::from_slice("e"))),
+                simple_selectors: vec!(LocalNameSelector(LocalNameSelector {
+                    name: Atom::from_slice("EeÉ"),
+                    lower_name: Atom::from_slice("eeÉ") })),
                 next: None,
             }),
             pseudo_element: None,
             specificity: specificity(0, 0, 1),
         })))
-        assert!(parse(".foo") == Some(vec!(Selector{
+        assert!(parse(".foo") == Ok(vec!(Selector {
             compound_selectors: Arc::new(CompoundSelector {
                 simple_selectors: vec!(ClassSelector(Atom::from_slice("foo"))),
                 next: None,
@@ -618,7 +613,7 @@ mod tests {
             pseudo_element: None,
             specificity: specificity(0, 1, 0),
         })))
-        assert!(parse("#bar") == Some(vec!(Selector{
+        assert!(parse("#bar") == Ok(vec!(Selector {
             compound_selectors: Arc::new(CompoundSelector {
                 simple_selectors: vec!(IDSelector(Atom::from_slice("bar"))),
                 next: None,
@@ -626,9 +621,11 @@ mod tests {
             pseudo_element: None,
             specificity: specificity(1, 0, 0),
         })))
-        assert!(parse("e.foo#bar") == Some(vec!(Selector{
+        assert!(parse("e.foo#bar") == Ok(vec!(Selector {
             compound_selectors: Arc::new(CompoundSelector {
-                simple_selectors: vec!(LocalNameSelector(Atom::from_slice("e")),
+                simple_selectors: vec!(LocalNameSelector(LocalNameSelector {
+                                            name: Atom::from_slice("e"),
+                                            lower_name: Atom::from_slice("e") }),
                                        ClassSelector(Atom::from_slice("foo")),
                                        IDSelector(Atom::from_slice("bar"))),
                 next: None,
@@ -636,11 +633,13 @@ mod tests {
             pseudo_element: None,
             specificity: specificity(1, 1, 1),
         })))
-        assert!(parse("e.foo #bar") == Some(vec!(Selector{
+        assert!(parse("e.foo #bar") == Ok(vec!(Selector {
             compound_selectors: Arc::new(CompoundSelector {
                 simple_selectors: vec!(IDSelector(Atom::from_slice("bar"))),
                 next: Some((box CompoundSelector {
-                    simple_selectors: vec!(LocalNameSelector(Atom::from_slice("e")),
+                    simple_selectors: vec!(LocalNameSelector(LocalNameSelector {
+                                                name: Atom::from_slice("e"),
+                                                lower_name: Atom::from_slice("e") }),
                                            ClassSelector(Atom::from_slice("foo"))),
                     next: None,
                 }, Descendant)),
@@ -651,7 +650,7 @@ mod tests {
         // Default namespace does not apply to attribute selectors
         // https://github.com/mozilla/servo/pull/1652
         let mut namespaces = NamespaceMap::new();
-        assert!(parse_ns("[Foo]", &namespaces) == Some(vec!(Selector{
+        assert!(parse_ns("[Foo]", &namespaces) == Ok(vec!(Selector {
             compound_selectors: Arc::new(CompoundSelector {
                 simple_selectors: vec!(AttrExists(AttrSelector {
                     name: "Foo".to_string(),
@@ -666,7 +665,7 @@ mod tests {
         // Default namespace does not apply to attribute selectors
         // https://github.com/mozilla/servo/pull/1652
         namespaces.default = Some(namespace::MathML);
-        assert!(parse_ns("[Foo]", &namespaces) == Some(vec!(Selector{
+        assert!(parse_ns("[Foo]", &namespaces) == Ok(vec!(Selector {
             compound_selectors: Arc::new(CompoundSelector {
                 simple_selectors: vec!(AttrExists(AttrSelector {
                     name: "Foo".to_string(),
@@ -679,11 +678,13 @@ mod tests {
             specificity: specificity(0, 1, 0),
         })))
         // Default namespace does apply to type selectors
-        assert!(parse_ns("e", &namespaces) == Some(vec!(Selector{
+        assert!(parse_ns("e", &namespaces) == Ok(vec!(Selector {
             compound_selectors: Arc::new(CompoundSelector {
                 simple_selectors: vec!(
                     NamespaceSelector(namespace::MathML),
-                    LocalNameSelector(Atom::from_slice("e")),
+                    LocalNameSelector(LocalNameSelector {
+                        name: Atom::from_slice("e"),
+                        lower_name: Atom::from_slice("e") }),
                 ),
                 next: None,
             }),
@@ -691,7 +692,7 @@ mod tests {
             specificity: specificity(0, 0, 1),
         })))
         // https://github.com/mozilla/servo/issues/1723
-        assert!(parse("::before") == Some(vec!(Selector{
+        assert!(parse("::before") == Ok(vec!(Selector {
             compound_selectors: Arc::new(CompoundSelector {
                 simple_selectors: vec!(),
                 next: None,
@@ -699,11 +700,13 @@ mod tests {
             pseudo_element: Some(Before),
             specificity: specificity(0, 0, 1),
         })))
-        assert!(parse("div :after") == Some(vec!(Selector{
+        assert!(parse("div :after") == Ok(vec!(Selector {
             compound_selectors: Arc::new(CompoundSelector {
                 simple_selectors: vec!(),
                 next: Some((box CompoundSelector {
-                    simple_selectors: vec!(LocalNameSelector(Atom::from_slice("div"))),
+                    simple_selectors: vec!(LocalNameSelector(LocalNameSelector {
+                        name: Atom::from_slice("div"),
+                        lower_name: Atom::from_slice("div") })),
                     next: None,
                 }, Descendant)),
             }),

src/components/style/style.rs

@@ -30,22 +30,17 @@ extern crate servo_util = "util";
 
 
 // Public API
-pub use stylesheets::{Stylesheet, CSSRule, StyleRule, CSSFontFaceRule};
+pub use stylesheets::{Stylesheet, iter_font_face_rules};
 pub use selector_matching::{Stylist, StylesheetOrigin, UserAgentOrigin, AuthorOrigin, UserOrigin};
-pub use selector_matching::{MatchedProperty, matches_compound_selector};
+pub use selector_matching::{DeclarationBlock, matches};
 pub use properties::{cascade, cascade_anonymous};
 pub use properties::{PropertyDeclaration, ComputedValues, computed_values, style_structs};
 pub use properties::{PropertyDeclarationBlock, parse_style_attribute};  // Style attributes
 pub use properties::{CSSFloat, DeclaredValue, PropertyDeclarationParseResult};
 pub use properties::longhands;
-pub use errors::with_errors_silenced;
 pub use node::{TElement, TNode};
-pub use selectors::{PseudoElement, Before, After, AttrSelector, SpecificNamespace, AnyNamespace};
+pub use selectors::{PseudoElement, Before, After, SelectorList, parse_selector_list_from_str};
-pub use selectors::{NamespaceConstraint, Selector, CompoundSelector, SimpleSelector, Combinator};
+pub use selectors::{AttrSelector, NamespaceConstraint, SpecificNamespace, AnyNamespace};
-pub use selectors::{parse_selector_list};
-pub use namespaces::NamespaceMap;
-pub use media_queries::{MediaRule, MediaQueryList, MediaQuery, Device, MediaType, MediaQueryType};
-pub use font_face::{FontFaceFormat, FontFaceRule, FontFaceSource,FontFaceSourceLine, TtfFormat};
 
 mod stylesheets;
 mod errors;

src/components/style/stylesheets.rs

@@ -16,13 +16,13 @@ use errors::{ErrorLoggerIterator, log_css_error};
 use namespaces::{NamespaceMap, parse_namespace_rule};
 use media_queries::{MediaRule, parse_media_rule};
 use media_queries;
-use font_face::{FontFaceRule, parse_font_face_rule};
+use font_face::{FontFaceRule, parse_font_face_rule, iter_font_face_rules_inner};
 
 
 pub struct Stylesheet {
     /// List of rules in the order they were found (important for
     /// cascading order)
-    pub rules: Vec<CSSRule>,
+    rules: Vec<CSSRule>,
 }
 
 
@@ -129,12 +129,12 @@ pub fn parse_style_rule(rule: QualifiedRule, parent_rules: &mut Vec<CSSRule>,
     let QualifiedRule{location: location, prelude: prelude, block: block} = rule;
     // FIXME: avoid doing this for valid selectors
     let serialized = prelude.iter().to_css();
-    match selectors::parse_selector_list(prelude, namespaces) {
+    match selectors::parse_selector_list(prelude.move_iter(), namespaces) {
-        Some(selectors) => parent_rules.push(CSSStyleRule(StyleRule{
+        Ok(selectors) => parent_rules.push(CSSStyleRule(StyleRule{
             selectors: selectors,
             declarations: properties::parse_property_declaration_list(block.move_iter(), base_url)
         })),
-        None => log_css_error(location, format!(
+        Err(()) => log_css_error(location, format!(
             "Invalid/unsupported selector: {}", serialized).as_slice()),
     }
 }
@@ -164,3 +164,15 @@ pub fn iter_style_rules<'a>(rules: &[CSSRule], device: &media_queries::Device,
         }
     }
 }
+
+#[inline]
+pub fn iter_stylesheet_style_rules(stylesheet: &Stylesheet, device: &media_queries::Device,
+                                   callback: |&StyleRule|) {
+    iter_style_rules(stylesheet.rules.as_slice(), device, callback)
+}
+
+
+#[inline]
+pub fn iter_font_face_rules(stylesheet: &Stylesheet, callback: |family: &str, sources: &Url|) {
+    iter_font_face_rules_inner(stylesheet.rules.as_slice(), callback)
+}

src/components/util/sort.rs

@@ -4,7 +4,7 @@
 
 //! In-place sorting.
 
-fn quicksort_helper<T:Ord + Eq + PartialOrd + PartialEq>(arr: &mut [T], left: int, right: int) {
+fn quicksort_helper<T>(arr: &mut [T], left: int, right: int, compare: fn(&T, &T) -> Ordering) {
     if right <= left {
         return
     }
@@ -17,11 +17,11 @@ fn quicksort_helper<T:Ord + Eq + PartialOrd + PartialEq>(arr: &mut [T], left: in
         let v: *mut T = &mut arr[right as uint];
         loop {
             i += 1;
-            while arr[i as uint] < (*v) {
+            while compare(&arr[i as uint], &*v) == Less {
                 i += 1
             }
             j -= 1;
-            while (*v) < arr[j as uint] {
+            while compare(&*v, &arr[j as uint]) == Less {
                 if j == left {
                     break
                 }
@@ -31,11 +31,11 @@ fn quicksort_helper<T:Ord + Eq + PartialOrd + PartialEq>(arr: &mut [T], left: in
                 break
             }
             arr.swap(i as uint, j as uint);
-            if arr[i as uint] == (*v) {
+            if compare(&arr[i as uint], &*v) == Equal {
                 p += 1;
                 arr.swap(p as uint, i as uint)
             }
-            if (*v) == arr[j as uint] {
+            if compare(&*v, &arr[j as uint]) == Equal {
                 q -= 1;
                 arr.swap(j as uint, q as uint)
             }
@@ -60,21 +60,21 @@ fn quicksort_helper<T:Ord + Eq + PartialOrd + PartialEq>(arr: &mut [T], left: in
         assert!(k != 0);
     }
 
-    quicksort_helper(arr, left, j);
+    quicksort_helper(arr, left, j, compare);
-    quicksort_helper(arr, i, right);
+    quicksort_helper(arr, i, right, compare);
 }
 
 /// An in-place quicksort.
 ///
 /// The algorithm is from Sedgewick and Bentley, "Quicksort is Optimal":
 ///     http://www.cs.princeton.edu/~rs/talks/QuicksortIsOptimal.pdf
-pub fn quicksort<T:Ord + Eq + PartialOrd + PartialEq>(arr: &mut [T]) {
+pub fn quicksort_by<T>(arr: &mut [T], compare: fn(&T, &T) -> Ordering) {
     if arr.len() <= 1 {
         return
     }
 
     let len = arr.len();
-    quicksort_helper(arr, 0, (len - 1) as int);
+    quicksort_helper(arr, 0, (len - 1) as int, compare);
 }
 
 #[cfg(test)]
@@ -90,7 +90,8 @@ pub mod test {
         for _ in range(0u32, 50000u32) {
             let len: uint = rng.gen();
             let mut v: Vec<int> = rng.gen_iter::<int>().take((len % 32) + 1).collect();
-            sort::quicksort(v.as_mut_slice());
+            fn compare_ints(a: &int, b: &int) -> Ordering { a.cmp(b) }
+            sort::quicksort_by(v.as_mut_slice(), compare_ints);
             for i in range(0, v.len() - 1) {
                 assert!(v.get(i) <= v.get(i + 1))
             }

src/support/css/rust-cssparser

@@ -1 +1 @@
-Subproject commit 918bae60d4703e3db7f306b6c4a09de0670b9472
+Subproject commit 48e517b40c8aad55b5f2b7072d092102b48797bc