auto merge of #1143 : SimonSapin/servo/selector-parsing+first-child, r=SimonSapin

Fixes #1133, fixes #1134.

This is a rebase and clean-up of these two pull requests that have been reviewed individually. Additionally, 5c59699 disables parsing of `display: list-item` which is not supported yet and may have been the cause of the hangs when testing #1134.

src/components/main/layout/box_builder.rs

@@ -379,38 +379,21 @@ impl LayoutTreeBuilder {
         }
     }
 
-    
-
     pub fn box_generator_for_node<'a>(&mut self,
                                       node: AbstractNode<LayoutView>,
                                       grandparent_generator: Option<&mut BoxGenerator<'a>>,
                                       parent_generator: &mut BoxGenerator<'a>,
                                       mut sibling_generator: Option<&mut BoxGenerator<'a>>)
                                       -> BoxGenResult<'a> {
-        let display = if node.is_element() {
+        let display = match node.type_id() {
-            let display = node.style().Box.display;
+            ElementNodeTypeId(_) => match node.style().Box.display {
-            if node.is_root() {
+                display::none => return NoGenerator,
-                match display {
+                display => display,
-                    display::none => return NoGenerator,
+            },
-                    display::inline => display::block,
+            TextNodeTypeId => display::inline,
-                    display::list_item => display::block,
+            DoctypeNodeTypeId |
-                    v => v
+            DocumentFragmentNodeTypeId |
-                }
+            CommentNodeTypeId => return NoGenerator,
-            } else {
-                match display {
-                    display::none => return NoGenerator,
-                    display::list_item => display::block,
-                    v => v
-                }
-            }
-        } else {
-            match node.type_id() {
-                ElementNodeTypeId(_) => display::inline,
-                TextNodeTypeId => display::inline,
-                DoctypeNodeTypeId |
-                DocumentFragmentNodeTypeId |
-                CommentNodeTypeId => return NoGenerator,
-            }
         };
 
         // FIXME(pcwalton): Unsafe.
@@ -448,8 +431,8 @@ impl LayoutTreeBuilder {
             // afterward.
             (display::block, _, Some(InlineFlowClass)) if is_float.is_some() => {
                 let float_type = FloatFlowType(is_float.unwrap());
-                let float_generator = self.create_child_generator(node, 
+                let float_generator = self.create_child_generator(node,
-                                                                  sibling_generator.unwrap(), 
+                                                                  sibling_generator.unwrap(),
                                                                   float_type);
                 return Mixed(float_generator, ~SiblingGenerator);
             }

src/components/script/dom/node.rs

@@ -164,6 +164,10 @@ impl<View> TreeNodeRef<Node<View>> for AbstractNode<View> {
             _ => false
         }
     }
+
+    fn is_root(&self) -> bool {
+        self.parent_node().is_none()
+    }
 }
 
 impl<View> TreeNodeRefAsElement<Node<View>, Element> for AbstractNode<View> {
@@ -611,7 +615,7 @@ impl Node<ScriptView> {
     }
 
     pub fn GetPreviousSibling(&self) -> Option<AbstractNode<ScriptView>> {
-        self.prev_sibling 
+        self.prev_sibling
     }
 
     pub fn GetNextSibling(&self) -> Option<AbstractNode<ScriptView>> {
@@ -621,7 +625,7 @@ impl Node<ScriptView> {
     pub fn GetNodeValue(&self, abstract_self: AbstractNode<ScriptView>) -> DOMString {
         match self.type_id {
             // ProcessingInstruction
-            CommentNodeTypeId | TextNodeTypeId => { 
+            CommentNodeTypeId | TextNodeTypeId => {
                 do abstract_self.with_imm_characterdata() |characterdata| {
                     characterdata.Data()
                 }

src/components/style/common_types.rs

@@ -146,16 +146,13 @@ pub mod computed {
         // TODO, as needed: root font size, viewport size, etc.
     }
 
-    #[inline]
-    fn mul(a: Au, b: CSSFloat) -> Au { Au(((*a as CSSFloat) * b) as i32) }
-
     pub fn compute_Au(value: specified::Length, context: &Context) -> Au {
         match value {
             specified::Au_(value) => value,
-            specified::Em(value) => mul(context.font_size, value),
+            specified::Em(value) => context.font_size.scale_by(value),
             specified::Ex(value) => {
                 let x_height = 0.5;  // TODO: find that from the font
-                mul(context.font_size, value * x_height)
+                context.font_size.scale_by(value * x_height)
             },
         }
     }

src/components/style/properties.rs.mako

@@ -101,10 +101,9 @@ pub mod longhands {
         </%self:longhand>
     </%def>
 
-    <%def name="single_keyword(name, values, inherited=False)">
+    <%def name="single_keyword_computed(name, values, inherited=False)">
         <%self:single_component_value name="${name}" inherited="${inherited}">
-            // The computed value is the same as the specified value.
+            ${caller.body()}
-            pub use to_computed_value = super::computed_as_specified;
             pub mod computed_value {
                 #[deriving(Eq, Clone)]
                 pub enum T {
@@ -130,6 +129,13 @@ pub mod longhands {
         </%self:single_component_value>
     </%def>
 
+    <%def name="single_keyword(name, values, inherited=False)">
+        <%self:single_keyword_computed name="${name}" values="${values}" inherited="${inherited}">
+            // The computed value is the same as the specified value.
+            pub use to_computed_value = super::computed_as_specified;
+        </%self:single_keyword_computed>
+    </%def>
+
     <%def name="predefined_type(name, type, initial_value, parse_method='parse', inherited=False)">
         <%self:single_component_value name="${name}" inherited="${inherited}">
             pub use to_computed_value = super::super::common_types::computed::compute_${type};
@@ -217,11 +223,35 @@ pub mod longhands {
     ${new_style_struct("Box")}
 
     // TODO: don't parse values we don't support
-    ${single_keyword("display",
+    <%self:single_keyword_computed name="display"
-        "inline block list-item inline-block none "
+            values="inline block inline-block none">
-    )}
+//        list-item
-//        "table inline-table table-row-group table-header-group table-footer-group "
+//        table inline-table table-row-group table-header-group table-footer-group
-//        "table-row table-column-group table-column table-cell table-caption"
+//        table-row table-column-group table-column table-cell table-caption
+        pub fn to_computed_value(value: SpecifiedValue, context: &computed::Context)
+                              -> computed_value::T {
+//            if context.is_root_element && value == list_item {
+//                return block
+//            }
+            let positioned = match context.position {
+                position::absolute | position::fixed => true,
+                _ => false
+            };
+            if positioned || context.float != float::none || context.is_root_element {
+                match value {
+//                    inline_table => table,
+                    inline | inline_block
+//                    | table_row_group | table_column | table_column_group
+//                    | table_header_group | table_footer_group | table_row
+//                    | table_cell | table_caption
+                    => block,
+                    _ => value,
+                }
+            } else {
+                value
+            }
+        }
+    </%self:single_keyword_computed>
 
     ${single_keyword("position", "static absolute relative fixed")}
     ${single_keyword("float", "none left right")}

src/components/style/selector_matching.rs

@@ -150,13 +150,10 @@ fn matches_selector<N: TreeNode<T>, T: TreeNodeRefAsElement<N, E>, E: ElementLik
     matches_compound_selector::<N, T, E>(&selector.compound_selectors, element)
 }
 
-
 fn matches_compound_selector<N: TreeNode<T>, T: TreeNodeRefAsElement<N, E>, E: ElementLike>(
         selector: &CompoundSelector, element: &T) -> bool {
-    if do element.with_imm_element_like |element: &E| {
+    if !do selector.simple_selectors.iter().all |simple_selector| {
-        !do selector.simple_selectors.iter().all |simple_selector| {
             matches_simple_selector(simple_selector, element)
-        }
     } {
         return false
     }
@@ -193,25 +190,37 @@ fn matches_compound_selector<N: TreeNode<T>, T: TreeNodeRefAsElement<N, E>, E: E
 }
 
 #[inline]
-fn matches_simple_selector<E: ElementLike>(selector: &SimpleSelector, element: &E) -> bool {
+fn matches_simple_selector<N: TreeNode<T>, T: TreeNodeRefAsElement<N, E>, E: ElementLike>(
+        selector: &SimpleSelector, element: &T) -> bool {
     static WHITESPACE: &'static [char] = &'static [' ', '\t', '\n', '\r', '\x0C'];
 
     match *selector {
         // TODO: case-sensitivity depends on the document type
         // TODO: intern element names
-        LocalNameSelector(ref name)
+        LocalNameSelector(ref name) => {
-        => element.get_local_name().eq_ignore_ascii_case(name.as_slice()),
+            do element.with_imm_element_like |element: &E| {
+                element.get_local_name().eq_ignore_ascii_case(name.as_slice())
+            }
+        }
         NamespaceSelector(_) => false,  // TODO, when the DOM supports namespaces on elements.
         // TODO: case-sensitivity depends on the document type and quirks mode
         // TODO: cache and intern IDs on elements.
-        IDSelector(ref id) => element.get_attr("id") == Some(id.as_slice()),
+        IDSelector(ref id) => {
+            do element.with_imm_element_like |element: &E| {
+                element.get_attr("id") == Some(id.as_slice())
+            }
+        }
         // TODO: cache and intern classe names on elements.
-        ClassSelector(ref class) => match element.get_attr("class") {
+        ClassSelector(ref class) => {
-            None => false,
+            do element.with_imm_element_like |element: &E| {
-            // TODO: case-sensitivity depends on the document type and quirks mode
+                match element.get_attr("class") {
-            Some(ref class_attr)
+                    None => false,
-            => class_attr.split_iter(WHITESPACE).any(|c| c == class.as_slice()),
+                    // TODO: case-sensitivity depends on the document type and quirks mode
-        },
+                    Some(ref class_attr)
+                    => class_attr.split_iter(WHITESPACE).any(|c| c == class.as_slice()),
+                }
+            }
+        }
 
         AttrExists(ref attr) => match_attribute(attr, element, |_| true),
         AttrEqual(ref attr, ref value) => match_attribute(attr, element, |v| v == value.as_slice()),
@@ -232,20 +241,41 @@ fn matches_simple_selector<E: ElementLike>(selector: &SimpleSelector, element: &
             attr_value.ends_with(value.as_slice())
         },
 
+        FirstChild => matches_first_child(element),
+
         Negation(ref negated) => {
             !negated.iter().all(|s| matches_simple_selector(s, element))
         },
     }
 }
 
-
 #[inline]
-fn match_attribute<E: ElementLike>(attr: &AttrSelector, element: &E, f: &fn(&str)-> bool) -> bool {
+fn matches_first_child<N: TreeNode<T>, T: TreeNodeRefAsElement<N, E>, E: ElementLike>(
-    match attr.namespace {
+        element: &T) -> bool {
-        Some(_) => false,  // TODO, when the DOM supports namespaces on attributes
+    let mut node = element.clone();
-        None => match element.get_attr(attr.name) {
+    loop {
-            None => false,
+        match node.node().prev_sibling() {
-            Some(ref value) => f(value.as_slice())
+            Some(prev_sibling) => {
+                node = prev_sibling;
+                if node.is_element() {
+                    return false
+                }
+            }
+            None => return !element.is_root(),
+        }
+    }
+}
+
+#[inline]
+fn match_attribute<N: TreeNode<T>, T: TreeNodeRefAsElement<N, E>, E: ElementLike>(
+        attr: &AttrSelector, element: &T, f: &fn(&str)-> bool) -> bool {
+    do element.with_imm_element_like |element: &E| {
+        match attr.namespace {
+            Some(_) => false,  // TODO, when the DOM supports namespaces on attributes
+            None => match element.get_attr(attr.name) {
+                None => false,
+                Some(ref value) => f(value.as_slice())
+            }
         }
     }
 }

src/components/style/selectors.rs

@@ -8,7 +8,7 @@ use cssparser::*;
 use namespaces::NamespaceMap;
 
 
-#[deriving(Clone)]
+#[deriving(Eq, Clone)]
 pub struct Selector {
     compound_selectors: CompoundSelector,
     pseudo_element: Option<PseudoElement>,
@@ -27,7 +27,7 @@ pub enum PseudoElement {
 }
 
 
-#[deriving(Clone)]
+#[deriving(Eq, Clone)]
 pub struct CompoundSelector {
     simple_selectors: ~[SimpleSelector],
     next: Option<(~CompoundSelector, Combinator)>,  // c.next is left of c
@@ -41,7 +41,7 @@ pub enum Combinator {
     LaterSibling,  // ~
 }
 
-#[deriving(Clone)]
+#[deriving(Eq, Clone)]
 pub enum SimpleSelector {
     IDSelector(~str),
     ClassSelector(~str),
@@ -58,6 +58,7 @@ pub enum SimpleSelector {
     AttrSuffixMatch(AttrSelector, ~str),  // [foo$=bar]
 
     // Pseudo-classes
+    FirstChild,
 //    Empty,
 //    Root,
 //    Lang(~str),
@@ -66,7 +67,7 @@ pub enum SimpleSelector {
     // ...
 }
 
-#[deriving(Clone)]
+#[deriving(Eq, Clone)]
 pub struct AttrSelector {
     name: ~str,
     namespace: Option<~str>,
@@ -180,6 +181,7 @@ fn compute_specificity(mut selector: &CompoundSelector,
                 &ClassSelector(*)
                 | &AttrExists(*) | &AttrEqual(*) | &AttrIncludes(*) | &AttrDashMatch(*)
                 | &AttrPrefixMatch(*) | &AttrSubstringMatch(*) | &AttrSuffixMatch(*)
+                | &FirstChild
 //                | &Empty | &Root | &Lang(*) | &NthChild(*)
                 => specificity.class_like_selectors += 1,
                 &NamespaceSelector(*) => (),
@@ -192,7 +194,7 @@ fn compute_specificity(mut selector: &CompoundSelector,
     static MAX_10BIT: u32 = (1u32 << 10) - 1;
     specificity.id_selectors.min(&MAX_10BIT) << 20
     | specificity.class_like_selectors.min(&MAX_10BIT) << 10
-    | specificity.id_selectors.min(&MAX_10BIT)
+    | specificity.element_selectors.min(&MAX_10BIT)
 }
 
 
@@ -211,7 +213,7 @@ fn parse_simple_selectors(iter: &mut Iter, namespaces: &NamespaceMap)
         match parse_one_simple_selector(iter, namespaces, /* inside_negation = */ false) {
             None => return None, // invalid selector
             Some(None) => break,
-            Some(Some(Left(s))) => simple_selectors.push(s),
+            Some(Some(Left(s))) => { simple_selectors.push(s); empty = false },
             Some(Some(Right(p))) => { pseudo_element = Some(p); break },
         }
     }
@@ -272,19 +274,27 @@ fn parse_one_simple_selector(iter: &mut Iter, namespaces: &NamespaceMap, inside_
             },
             _ => fail!("Implementation error, this should not happen."),
         },
-        Some(&Delim(':')) => {
+        Some(&Colon) => {
             iter.next();
             match iter.next() {
                 Some(Ident(name)) => match parse_simple_pseudo_class(name) {
-                    None => None,
+                    None => match name.to_ascii_lower().as_slice() {
-                    Some(result) => Some(Some(result)),
+                        // Supported CSS 2.1 pseudo-elements only.
+                        // ** Do not add to this list! **
+                        "before" => Some(Some(Right(Before))),
+                        "after" => Some(Some(Right(After))),
+                        "first-line" => Some(Some(Right(FirstLine))),
+                        "first-letter" => Some(Some(Right(FirstLetter))),
+                        _ => None
+                    },
+                    Some(result) => Some(Some(Left(result))),
                 },
                 Some(Function(name, arguments)) => match parse_functional_pseudo_class(
                         name, arguments, namespaces, inside_negation) {
                     None => None,
                     Some(simple_selector) => Some(Some(Left(simple_selector))),
                 },
-                Some(Delim(':')) => {
+                Some(Colon) => {
                     match iter.next() {
                         Some(Ident(name)) => match parse_pseudo_element(name) {
                             Some(pseudo_element) => Some(Some(Right(pseudo_element))),
@@ -359,8 +369,7 @@ fn parse_qualified_name(iter: &mut Iter, allow_universal: bool, namespaces: &Nam
             }
         },
         Some(&Delim('|')) => explicit_namespace(iter, allow_universal, Some(~"")),
-        Some(&IDHash(*)) => default_namespace(namespaces, None),
+        _ => Some(None),
-        _ => return None,
     }
 }
 
@@ -405,16 +414,11 @@ fn parse_attribute_selector(content: ~[ComponentValue], namespaces: &NamespaceMa
 }
 
 
-fn parse_simple_pseudo_class(name: ~str) -> Option<Either<SimpleSelector, PseudoElement>> {
+fn parse_simple_pseudo_class(name: &str) -> Option<SimpleSelector> {
     match name.to_ascii_lower().as_slice() {
-//        "root" => Some(Left(Root)),
+        "first-child" => Some(FirstChild),
-//        "empty" => Some(Left(Empty)),
+//        "root" => Some(Root),
-
+//        "empty" => Some(Empty),
-        // Supported CSS 2.1 pseudo-elements only.
-        "before" => Some(Right(Before)),
-        "after" => Some(Right(After)),
-        "first-line" => Some(Right(FirstLine)),
-        "first-letter" => Some(Right(FirstLetter)),
         _ => None
     }
 }
@@ -492,3 +496,72 @@ fn skip_whitespace(iter: &mut Iter) -> bool {
         iter.next();
     }
 }
+
+
+#[cfg(test)]
+mod tests {
+    use cssparser;
+    use namespaces::NamespaceMap;
+    use super::*;
+
+    fn parse(input: &str) -> Option<~[Selector]> {
+        parse_selector_list(
+            cssparser::tokenize(input).map(|(v, _)| v).to_owned_vec(),
+            &NamespaceMap::new())
+    }
+
+    fn specificity(a: u32, b: u32, c: u32) -> u32 {
+        a << 20 | b << 10 | c
+    }
+
+    #[test]
+    fn test_parsing() {
+        assert_eq!(parse(""), None)
+        assert_eq!(parse("e"), Some(~[Selector{
+            compound_selectors: CompoundSelector {
+                simple_selectors: ~[LocalNameSelector(~"e")],
+                next: None,
+            },
+            pseudo_element: None,
+            specificity: specificity(0, 0, 1),
+        }]))
+        assert_eq!(parse(".foo"), Some(~[Selector{
+            compound_selectors: CompoundSelector {
+                simple_selectors: ~[ClassSelector(~"foo")],
+                next: None,
+            },
+            pseudo_element: None,
+            specificity: specificity(0, 1, 0),
+        }]))
+        assert_eq!(parse("#bar"), Some(~[Selector{
+            compound_selectors: CompoundSelector {
+                simple_selectors: ~[IDSelector(~"bar")],
+                next: None,
+            },
+            pseudo_element: None,
+            specificity: specificity(1, 0, 0),
+        }]))
+        assert_eq!(parse("e.foo#bar"), Some(~[Selector{
+            compound_selectors: CompoundSelector {
+                simple_selectors: ~[LocalNameSelector(~"e"),
+                                    ClassSelector(~"foo"),
+                                    IDSelector(~"bar")],
+                next: None,
+            },
+            pseudo_element: None,
+            specificity: specificity(1, 1, 1),
+        }]))
+        assert_eq!(parse("e.foo #bar"), Some(~[Selector{
+            compound_selectors: CompoundSelector {
+                simple_selectors: ~[IDSelector(~"bar")],
+                next: Some((~CompoundSelector {
+                    simple_selectors: ~[LocalNameSelector(~"e"),
+                                        ClassSelector(~"foo")],
+                    next: None,
+                }, Descendant)),
+            },
+            pseudo_element: None,
+            specificity: specificity(1, 1, 1),
+        }]))
+    }
+}

src/components/util/tree.rs

@@ -239,6 +239,8 @@ pub trait TreeNodeRef<Node>: Clone {
     }
 
     fn is_element(&self) -> bool;
+
+    fn is_root(&self) -> bool;
 }
 
 pub trait TreeNodeRefAsElement<Node, E: ElementLike>: TreeNodeRef<Node> {