Fill in remaining DisplayType values, replace uses, and make box-building return and option'd box since display:none nodes generate no boxes.

src/servo/css/parser.rs

@@ -3,8 +3,8 @@
 // TODO: fail according to the css spec instead of failing when things
 // are not as expected
 
-use css::values::{DisInline, DisBlock, DisNone, Display, TextColor, BackgroundColor, FontSize,
+use css::values::{TextColor, BackgroundColor, FontSize, Height, Width,
-                     Height, Width, StyleDeclaration};
+                     Display, StyleDeclaration};
 // Disambiguate parsed Selector, Rule values from tokens
 use css = css::values;
 use tok = lexer;

src/servo/css/parser_util.rs

@@ -1,9 +1,10 @@
 #[doc = "Helper functions to parse values of specific attributes."]
 
-import css::values::*;
+use css::values::{DisplayType, Inline, Block, DisplayNone};
-import str::{pop_char, from_chars};
+use css::values::{Unit, Pt, Mm, Px, Percent, Auto};
-import float::from_str;
+use str::{pop_char, from_chars};
-import option::map;
+use float::from_str;
+use option::map;
 
 export parse_font_size;
 export parse_size;
@@ -53,9 +54,9 @@ fn parse_size(str : ~str) -> Option<Unit> {
 
 fn parse_display_type(str : ~str) -> Option<DisplayType> {
     match str {
-      ~"inline" => Some(DisInline),
+      ~"inline" => Some(Inline),
-      ~"block" => Some(DisBlock),
+      ~"block" => Some(Block),
-      ~"none" => Some(DisNone),
+      ~"none" => Some(DisplayNone),
       _ => { #debug["Recieved unknown display value '%s'", str]; None }
     }
 }

src/servo/css/styles.rs

@@ -2,7 +2,7 @@
 
 import std::arc::{ARC, get, clone};
 
-import css::values::{DisplayType, DisBlock, DisInline, DisNone, Unit, Auto};
+import css::values::{DisplayType, DisplayNone, Inline, Block, Unit, Auto};
 import css::values::Stylesheet;
 import dom::base::{HTMLDivElement, HTMLHeadElement, HTMLImageElement, UnknownElement, HTMLScriptElement};
 import dom::base::{Comment, Doctype, Element, Node, NodeKind, Text};
@@ -37,17 +37,17 @@ impl NodeKind : DefaultStyleMethods {
 
     fn default_display_type() -> DisplayType {
         match self {
-          Text(*) => { DisInline }
+          Text(*) => { Inline }
           Element(element) => {
             match *element.kind {
-              HTMLDivElement => DisBlock,
+              HTMLDivElement => Block,
-              HTMLHeadElement => DisNone,
+              HTMLHeadElement => DisplayNone,
-              HTMLImageElement(*) => DisInline,
+              HTMLImageElement(*) => Inline,
-              HTMLScriptElement => DisNone,
+              HTMLScriptElement => DisplayNone,
-              UnknownElement => DisInline,
+              UnknownElement => Inline,
             }
           },
-          Comment(*) | Doctype(*) => DisNone
+          Comment(*) | Doctype(*) => DisplayNone
         }
     }
     

src/servo/css/values.rs

@@ -7,9 +7,21 @@ import util::color::Color;
 "]
 
 enum DisplayType {
-    DisBlock,
+    Inline,
-    DisInline,
+    Block,
-    DisNone
+    ListItem,
+    InlineBlock,
+    Table,
+    InlineTable,
+    TableRowGroup,
+    TableHeaderGroup,
+    TableFooterGroup,
+    TableRow,
+    TableColumnGroup,
+    TableColumn,
+    TableCell,
+    TableCaption,
+    DisplayNone
 }
 
 enum Unit {

src/servo/layout/box_builder.rs

@@ -1,6 +1,6 @@
 #[doc="Creates CSS boxes from a DOM."]
 
-import css::values::{DisplayType, DisBlock, DisInline, DisNone};
+import css::values::{DisplayType, Block, Inline, DisplayNone};
 import dom::base::{ElementData, HTMLDivElement, HTMLImageElement, Element, Text, Node};
 import gfx::geometry::zero_size_au;
 import layout::base::{Appearance, BTree, BlockBox, Box, BoxKind, InlineBox, IntrinsicBox, NTree};
@@ -43,17 +43,20 @@ impl ctxt {
 
             // Create boxes for the child. Get its primary box.
             let kid_box = kid.construct_boxes();
+            if (kid_box.is_none()) {
+                again;
+            }
 
             // Determine the child's display.
             let disp = kid.get_specified_style().display_type;
-            if disp != Some(DisInline) {
+            if disp != Some(Inline) {
                 self.finish_anonymous_box_if_necessary();
             }
 
             // Add the child's box to the current enclosing box or the current anonymous box.
             match kid.get_specified_style().display_type {
-              Some(DisBlock) => BTree.add_child(self.parent_box, kid_box),
+              Some(Block) => BTree.add_child(self.parent_box, kid_box.get()),
-              Some(DisInline) => {
+              Some(Inline) => {
                 let anon_box = match self.anon_box {
                   None => {
                     //
@@ -70,9 +73,9 @@ impl ctxt {
                   }
                   Some(b) => b
                 };
-                BTree.add_child(anon_box, kid_box);
+                BTree.add_child(anon_box, kid_box.get());
               }
-              Some(DisNone) => {
+              Some(DisplayNone) => {
                 // Nothing to do.
               }
               _ => { //hack for now
@@ -93,21 +96,21 @@ impl ctxt {
 
             // Determine the child's display.
             let disp = kid.get_specified_style().display_type;
-            if disp != Some(DisInline) {
+            if disp != Some(Inline) {
                 // TODO
             }
 
             // Add the child's box to the current enclosing box.
             match kid.get_specified_style().display_type {
-              Some(DisBlock) => {
+              Some(Block) => {
                 // TODO
                 #warn("TODO: non-inline display found inside inline box");
-                BTree.add_child(self.parent_box, kid_box);
+                BTree.add_child(self.parent_box, kid_box.get());
               }
-              Some(DisInline) => {
+              Some(Inline) => {
-                BTree.add_child(self.parent_box, kid_box);
+                BTree.add_child(self.parent_box, kid_box.get());
               }
-              Some(DisNone) => {
+              Some(DisplayNone) => {
                 // Nothing to do.
               }
               _  => { //hack for now
@@ -122,9 +125,9 @@ impl ctxt {
         self.parent_node.dump();
 
         match self.parent_node.get_specified_style().display_type {
-          Some(DisBlock) => self.construct_boxes_for_block_children(),
+          Some(Block) => self.construct_boxes_for_block_children(),
-          Some(DisInline) => self.construct_boxes_for_inline_children(),
+          Some(Inline) => self.construct_boxes_for_inline_children(),
-          Some(DisNone) => { /* Nothing to do. */ }
+          Some(DisplayNone) => { /* Nothing to do. */ }
           _ => { //hack for now
           }
         }
@@ -147,7 +150,7 @@ impl ctxt {
 }
 
 trait PrivBoxBuilder {
-    fn determine_box_kind() -> BoxKind;
+    fn determine_box_kind() -> Option<BoxKind>;
 }
 
 impl Node : PrivBoxBuilder {
@@ -155,18 +158,16 @@ impl Node : PrivBoxBuilder {
       Determines the kind of box that this node needs. Also, for images, computes the intrinsic
       size.
      "]
-    fn determine_box_kind() -> BoxKind {
+    fn determine_box_kind() -> Option<BoxKind> {
         match self.read(|n| copy n.kind) {
-            ~Text(string) => TextBoxKind(@TextBox(copy string)),
+            ~Text(string) => Some(TextBoxKind(@TextBox(copy string))),
             ~Element(element) => {
                 match (copy *element.kind, self.get_specified_style().display_type)  {
-                    (HTMLImageElement({size}), _) => IntrinsicBox(@size),
+                    (HTMLImageElement({size}), _) => Some(IntrinsicBox(@size)),
-                    (_, Some(DisBlock)) => BlockBox,
+                    (_, Some(Block)) => Some(BlockBox),
-                    (_, Some(DisInline)) => InlineBox,
+                    (_, Some(Inline)) => Some(InlineBox),
-                    (_, Some(DisNone)) => {
+                    (_, Some(DisplayNone)) => None,
-                        // TODO: don't have a box here at all?
+                    (_, Some(_)) => Some(InlineBox),
-                        IntrinsicBox(@zero_size_au())
-                    }
                     (_, None) => {
                         fail ~"The specified display style should be a default instead of none"
                     }
@@ -178,24 +179,28 @@ impl Node : PrivBoxBuilder {
 }
 
 trait BoxBuilder {
-    fn construct_boxes() -> @Box;
+    fn construct_boxes() -> Option<@Box>;
 }
 
 impl Node : BoxBuilder {
     #[doc="Creates boxes for this node. This is the entry point."]
-    fn construct_boxes() -> @Box {
+    fn construct_boxes() -> Option<@Box> {
-        let box_kind = self.determine_box_kind();
+        match self.determine_box_kind() {
-        let my_box = @Box(self, box_kind);
+            None => None,
-        match box_kind {
+            Some(kind) => {
-          BlockBox | InlineBox => {
+                let my_box = @Box(self, kind);
-            let cx = create_context(self, my_box);
+                match kind {
-            cx.construct_boxes_for_children();
+                    BlockBox | InlineBox => {
-          }
+                        let cx = create_context(self, my_box);
-          _ => {
+                        cx.construct_boxes_for_children();
-            // Nothing to do.
+                    }
-          }
+                    _ => {
+                        // Nothing to do.
+                    }
+                }
+                Some(my_box)
+            }
         }
-        return my_box;
     }
 }
 

src/servo/layout/layout_task.rs

@@ -10,6 +10,7 @@ import css::values::Stylesheet;
 import gfx::geometry::px_to_au;
 import gfx::render_task;
 import render_task::RenderTask;
+import layout::base::Box;
 import resource::image_cache_task::ImageCacheTask;
 import std::net::url::Url;
 import css::resolve::apply::apply_style;
@@ -48,16 +49,21 @@ fn LayoutTask(render_task: RenderTask, image_cache_task: ImageCacheTask) -> Layo
                     layout_data_refs += node.initialize_style_for_subtree();
                     node.recompute_style_for_subtree(styles);
 
-                    let this_box = node.construct_boxes();
+                    let root_box: @Box;
-                    this_box.dump();
+                    match node.construct_boxes() {
+                        None => fail ~"Root node should always exist; did it get 'display: none' somehow?",
+                        Some(root) => root_box = root
+                    }
+                    
+                    root_box.dump();
 
                     let reflow: fn~() = || event_chan.send(ReflowEvent);
 
-                    apply_style(this_box, &doc_url, image_cache_task, reflow);
+                    apply_style(root_box, &doc_url, image_cache_task, reflow);
 
-                    this_box.reflow_subtree(px_to_au(800));
+                    root_box.reflow_subtree(px_to_au(800));
 
-                    let dlist = build_display_list(this_box);
+                    let dlist = build_display_list(root_box);
                     render_task.send(render_task::RenderMsg(dlist));
                 }
               }