From 3029fbab922d7137f6701b4445c1a2cb1f7c7bfb Mon Sep 17 00:00:00 2001
From: Patrick Walton <pcwalton@mimiga.net>
Date: Tue, 9 Dec 2014 12:12:50 -0800
Subject: [PATCH] layout: Implement basic lists and the CSS1 list properties.

The exact rendering is ill-spec'd. Some things are ugly (especially the
width and height of list style images) but they are infrequently used
and I believe this implementation matches the spec. Numeric lists are
not supported yet, since they will require a separate layout pass.

The implementation is a subclass of `BlockFlow`, on advice from Robert
O'Callahan.
---
 components/layout/block.rs                |  14 +--
 components/layout/construct.rs            | 135 ++++++++++++++++-----
 components/layout/display_list_builder.rs |  76 ++++++++++--
 components/layout/flow.rs                 |   1 +
 components/layout/lib.rs                  |   1 +
 components/layout/list_item.rs            | 135 +++++++++++++++++++++
 components/layout/table_cell.rs           |   2 +-
 components/style/properties/mod.rs.mako   | 137 +++++++++++++++++++++-
 tests/ref/basic.list                      |   2 +
 tests/ref/list_style_position_a.html      |  29 +++++
 tests/ref/list_style_position_ref.html    |  28 +++++
 tests/ref/list_style_type_a.html          |  19 +++
 tests/ref/list_style_type_ref.html        |  14 +++
 tests/ref/smiling.png                     | Bin 0 -> 3390 bytes
 14 files changed, 540 insertions(+), 53 deletions(-)
 create mode 100644 components/layout/list_item.rs
 create mode 100644 tests/ref/list_style_position_a.html
 create mode 100644 tests/ref/list_style_position_ref.html
 create mode 100644 tests/ref/list_style_type_a.html
 create mode 100644 tests/ref/list_style_type_ref.html
 create mode 100644 tests/ref/smiling.png

diff --git a/components/layout/block.rs b/components/layout/block.rs
index 61fff95ec09..079a1961073 100644
--- a/components/layout/block.rs
+++ b/components/layout/block.rs
@@ -30,7 +30,7 @@
 use construct::FlowConstructor;
 use context::LayoutContext;
 use css::node_style::StyledNode;
-use display_list_builder::{BlockFlowDisplayListBuilding, BlockLevel, FragmentDisplayListBuilding};
+use display_list_builder::{BlockFlowDisplayListBuilding, FragmentDisplayListBuilding};
 use floats::{ClearBoth, ClearLeft, ClearRight, FloatKind, FloatLeft, Floats, PlacementInfo};
 use flow::{AbsolutePositionInfo, BaseFlow, BlockFlowClass, FloatIfNecessary, FlowClass, Flow};
 use flow::{ForceNonfloated, ImmutableFlowUtils, MutableFlowUtils, PreorderFlowTraversal};
@@ -51,6 +51,7 @@ use table::ColumnComputedInlineSize;
 use wrapper::ThreadSafeLayoutNode;
 
 use geom::Size2D;
+use gfx::display_list::DisplayList;
 use serialize::{Encoder, Encodable};
 use servo_msg::compositor_msg::LayerId;
 use servo_util::geometry::{Au, MAX_AU, MAX_RECT, ZERO_POINT};
@@ -1853,16 +1854,7 @@ impl Flow for BlockFlow {
     }
 
     fn build_display_list(&mut self, layout_context: &LayoutContext) {
-        if self.base.flags.is_float() {
-            // TODO(#2009, pcwalton): This is a pseudo-stacking context. We need to merge `z-index:
-            // auto` kids into the parent stacking context, when that is supported.
-            self.build_display_list_for_floating_block(layout_context)
-        } else if self.base.flags.contains(IS_ABSOLUTELY_POSITIONED) {
-            self.build_display_list_for_absolutely_positioned_block(layout_context)
-        } else {
-            self.build_display_list_for_block(layout_context, BlockLevel)
-        }
-
+        self.build_display_list_for_block(box DisplayList::new(), layout_context);
         if opts::get().validate_display_list_geometry {
             self.base.validate_display_list_geometry();
         }
diff --git a/components/layout/construct.rs b/components/layout/construct.rs
index 21a24af7efb..ec8533058cd 100644
--- a/components/layout/construct.rs
+++ b/components/layout/construct.rs
@@ -30,6 +30,7 @@ use fragment::{TableColumnFragment, TableColumnFragmentInfo, TableFragment, Tabl
 use fragment::{TableWrapperFragment, UnscannedTextFragment, UnscannedTextFragmentInfo};
 use incremental::{RECONSTRUCT_FLOW, RestyleDamage};
 use inline::InlineFlow;
+use list_item::{mod, ListItemFlow};
 use parallel;
 use table_wrapper::TableWrapperFlow;
 use table::TableFlow;
@@ -59,7 +60,7 @@ use std::collections::DList;
 use std::mem;
 use std::sync::atomic::Relaxed;
 use style::ComputedValues;
-use style::computed_values::{display, position, float};
+use style::computed_values::{display, position, float, list_style_position};
 use sync::Arc;
 use url::Url;
 
@@ -471,38 +472,25 @@ impl<'a> FlowConstructor<'a> {
         }
     }
 
-    /// Build block flow for current node using information from children nodes.
-    ///
-    /// Consume results from children and combine them, handling {ib} splits.
-    /// Block flows and inline flows thus created will become the children of
-    /// this block flow.
-    /// Also, deal with the absolute and fixed descendants bubbled up by
-    /// children nodes.
-    fn build_flow_for_block(&mut self, mut flow: FlowRef, node: &ThreadSafeLayoutNode)
-                            -> ConstructionResult {
+    /// Constructs a block flow, beginning with the given `initial_fragment` if present and then
+    /// appending the construction results of children to the child list of the block flow. {ib}
+    /// splits and absolutely-positioned descendants are handled correctly.
+    fn build_flow_for_block_starting_with_fragment(&mut self,
+                                                   mut flow: FlowRef,
+                                                   node: &ThreadSafeLayoutNode,
+                                                   initial_fragment: Option<Fragment>)
+                                                   -> ConstructionResult {
         // Gather up fragments for the inline flows we might need to create.
         let mut inline_fragment_accumulator = InlineFragmentsAccumulator::new();
         let mut consecutive_siblings = vec!();
-        let mut first_fragment = true;
 
-        // Special case: If this is generated content, then we need to initialize the accumulator
-        // with the fragment corresponding to that content.
-        if node.get_pseudo_element_type() != Normal ||
-           node.type_id() == Some(ElementNodeTypeId(HTMLInputElementTypeId)) ||
-           node.type_id() == Some(ElementNodeTypeId(HTMLTextAreaElementTypeId)) {
-            // A TextArea's text contents are displayed through the input text
-            // box, so don't construct them.
-            // TODO Maybe this belongs somewhere else?
-            if node.type_id() == Some(ElementNodeTypeId(HTMLTextAreaElementTypeId)) {
-                for kid in node.children() {
-                    kid.set_flow_construction_result(NoConstructionResult)
-                }
+        let mut first_fragment = match initial_fragment {
+            None => true,
+            Some(initial_fragment) => {
+                inline_fragment_accumulator.fragments.push_back(initial_fragment);
+                false
             }
-            let fragment_info = UnscannedTextFragment(UnscannedTextFragmentInfo::new(node));
-            let fragment = Fragment::new_from_specific_info(node, fragment_info);
-            inline_fragment_accumulator.fragments.push_back(fragment);
-            first_fragment = false;
-        }
+        };
 
         // List of absolute descendants, in tree order.
         let mut abs_descendants = Descendants::new();
@@ -552,6 +540,39 @@ impl<'a> FlowConstructor<'a> {
         FlowConstructionResult(flow, abs_descendants)
     }
 
+    /// Constructs a flow for the given block node and its children. This method creates an
+    /// initial fragment as appropriate and then dispatches to
+    /// `build_flow_for_block_starting_with_fragment`. Currently the following kinds of flows get
+    /// initial content:
+    ///
+    /// * Generated content gets the initial content specified by the `content` attribute of the
+    ///   CSS.
+    /// * `<input>` and `<textarea>` elements get their content.
+    ///
+    /// FIXME(pcwalton): It is not clear to me that there isn't a cleaner way to handle
+    /// `<textarea>`.
+    fn build_flow_for_block(&mut self, flow: FlowRef, node: &ThreadSafeLayoutNode)
+                            -> ConstructionResult {
+        let initial_fragment = if node.get_pseudo_element_type() != Normal ||
+           node.type_id() == Some(ElementNodeTypeId(HTMLInputElementTypeId)) ||
+           node.type_id() == Some(ElementNodeTypeId(HTMLTextAreaElementTypeId)) {
+            // A TextArea's text contents are displayed through the input text
+            // box, so don't construct them.
+            if node.type_id() == Some(ElementNodeTypeId(HTMLTextAreaElementTypeId)) {
+                for kid in node.children() {
+                    kid.set_flow_construction_result(NoConstructionResult)
+                }
+            }
+            Some(Fragment::new_from_specific_info(
+                    node,
+                    UnscannedTextFragment(UnscannedTextFragmentInfo::new(node))))
+        } else {
+            None
+        };
+
+        self.build_flow_for_block_starting_with_fragment(flow, node, initial_fragment)
+    }
+
     /// Builds a flow for a node with `display: block`. This yields a `BlockFlow` with possibly
     /// other `BlockFlow`s or `InlineFlow`s underneath it, depending on whether {ib} splits needed
     /// to happen.
@@ -905,6 +926,59 @@ impl<'a> FlowConstructor<'a> {
         self.build_flow_for_block(FlowRef::new(flow), node)
     }
 
+    /// Builds a flow for a node with `display: list-item`. This yields a `ListItemFlow` with
+    /// possibly other `BlockFlow`s or `InlineFlow`s underneath it.
+    fn build_flow_for_list_item(&mut self, node: &ThreadSafeLayoutNode) -> ConstructionResult {
+        let marker_fragment = match node.style().get_list().list_style_image {
+            Some(ref url) => {
+                Some(Fragment::new_from_specific_info(
+                    node,
+                    self.build_fragment_info_for_image(node, Some((*url).clone()))))
+            }
+            None => {
+                match list_item::static_text_for_list_style_type(node.style()
+                                                                     .get_list()
+                                                                     .list_style_type) {
+                    None => None,
+                    Some(text) => {
+                        let text = text.to_string();
+                        let mut unscanned_marker_fragments = DList::new();
+                        unscanned_marker_fragments.push_back(Fragment::new_from_specific_info(
+                            node,
+                            UnscannedTextFragment(UnscannedTextFragmentInfo::from_text(text))));
+                        let marker_fragments = TextRunScanner::new().scan_for_runs(
+                            self.layout_context.font_context(),
+                            unscanned_marker_fragments);
+                        debug_assert!(marker_fragments.len() == 1);
+                        marker_fragments.fragments.into_iter().next()
+                    }
+                }
+            }
+        };
+
+        // If the list marker is outside, it becomes the special "outside fragment" that list item
+        // flows have. If it's inside, it's just a plain old fragment. Note that this means that
+        // we adopt Gecko's behavior rather than WebKit's when the marker causes an {ib} split,
+        // which has caused some malaise (Bugzilla #36854) but CSS 2.1 § 12.5.1 lets me do it, so
+        // there.
+        let flow;
+        let initial_fragment;
+        match node.style().get_list().list_style_position {
+            list_style_position::outside => {
+                flow = box ListItemFlow::from_node_and_marker(self, node, marker_fragment);
+                initial_fragment = None;
+            }
+            list_style_position::inside => {
+                flow = box ListItemFlow::from_node_and_marker(self, node, None);
+                initial_fragment = marker_fragment;
+            }
+        }
+
+        self.build_flow_for_block_starting_with_fragment(FlowRef::new(flow as Box<Flow>),
+                                                         node,
+                                                         initial_fragment)
+    }
+
     /// Creates a fragment for a node with `display: table-column`.
     fn build_fragments_for_table_column(&mut self, node: &ThreadSafeLayoutNode)
                                         -> ConstructionResult {
@@ -1058,6 +1132,11 @@ impl<'a> PostorderNodeMutTraversal for FlowConstructor<'a> {
                 node.set_flow_construction_result(self.build_flow_for_nonfloated_block(node))
             }
 
+            // List items contribute their own special flows.
+            (display::list_item, _, _) => {
+                node.set_flow_construction_result(self.build_flow_for_list_item(node))
+            }
+
             // Inline items that are absolutely-positioned contribute inline fragment construction
             // results with a hypothetical fragment.
             (display::inline, _, position::absolute) => {
diff --git a/components/layout/display_list_builder.rs b/components/layout/display_list_builder.rs
index b11a14033c0..c8db1d4f992 100644
--- a/components/layout/display_list_builder.rs
+++ b/components/layout/display_list_builder.rs
@@ -12,12 +12,13 @@
 
 use block::BlockFlow;
 use context::LayoutContext;
-use flow::{mod, Flow, NEEDS_LAYER};
+use flow::{mod, Flow, IS_ABSOLUTELY_POSITIONED, NEEDS_LAYER};
 use fragment::{Fragment, GenericFragment, IframeFragment, IframeFragmentInfo, ImageFragment};
 use fragment::{ImageFragmentInfo, InlineAbsoluteHypotheticalFragment, InlineBlockFragment};
 use fragment::{ScannedTextFragment, ScannedTextFragmentInfo, TableFragment};
 use fragment::{TableCellFragment, TableColumnFragment, TableRowFragment, TableWrapperFragment};
 use fragment::{UnscannedTextFragment};
+use list_item::ListItemFlow;
 use model;
 use util::{OpaqueNodeMethods, ToGfxColor};
 
@@ -914,12 +915,19 @@ pub trait BlockFlowDisplayListBuilding {
                                          display_list: &mut DisplayList,
                                          layout_context: &LayoutContext,
                                          background_border_level: BackgroundAndBorderLevel);
-    fn build_display_list_for_block(&mut self,
-                                    layout_context: &LayoutContext,
-                                    background_border_level: BackgroundAndBorderLevel);
+    fn build_display_list_for_static_block(&mut self,
+                                           display_list: Box<DisplayList>,
+                                           layout_context: &LayoutContext,
+                                           background_border_level: BackgroundAndBorderLevel);
     fn build_display_list_for_absolutely_positioned_block(&mut self,
+                                                          display_list: Box<DisplayList>,
                                                           layout_context: &LayoutContext);
-    fn build_display_list_for_floating_block(&mut self, layout_context: &LayoutContext);
+    fn build_display_list_for_floating_block(&mut self,
+                                             display_list: Box<DisplayList>,
+                                             layout_context: &LayoutContext);
+    fn build_display_list_for_block(&mut self,
+                                    display_list: Box<DisplayList>,
+                                    layout_context: &LayoutContext);
     fn create_stacking_context(&self,
                                display_list: Box<DisplayList>,
                                layer: Option<Arc<PaintLayer>>)
@@ -945,10 +953,10 @@ impl BlockFlowDisplayListBuilding for BlockFlow {
         }
     }
 
-    fn build_display_list_for_block(&mut self,
-                                    layout_context: &LayoutContext,
-                                    background_border_level: BackgroundAndBorderLevel) {
-        let mut display_list = box DisplayList::new();
+    fn build_display_list_for_static_block(&mut self,
+                                           mut display_list: Box<DisplayList>,
+                                           layout_context: &LayoutContext,
+                                           background_border_level: BackgroundAndBorderLevel) {
         self.build_display_list_for_block_base(&mut *display_list,
                                                layout_context,
                                                background_border_level);
@@ -961,8 +969,8 @@ impl BlockFlowDisplayListBuilding for BlockFlow {
     }
 
     fn build_display_list_for_absolutely_positioned_block(&mut self,
+                                                          mut display_list: Box<DisplayList>,
                                                           layout_context: &LayoutContext) {
-        let mut display_list = box DisplayList::new();
         self.build_display_list_for_block_base(&mut *display_list,
                                                layout_context,
                                                RootOfStackingContextLevel);
@@ -991,8 +999,9 @@ impl BlockFlowDisplayListBuilding for BlockFlow {
         self.base.display_list_building_result = StackingContextResult(stacking_context)
     }
 
-    fn build_display_list_for_floating_block(&mut self, layout_context: &LayoutContext) {
-        let mut display_list = box DisplayList::new();
+    fn build_display_list_for_floating_block(&mut self,
+                                             mut display_list: Box<DisplayList>,
+                                             layout_context: &LayoutContext) {
         self.build_display_list_for_block_base(&mut *display_list,
                                                layout_context,
                                                RootOfStackingContextLevel);
@@ -1005,6 +1014,20 @@ impl BlockFlowDisplayListBuilding for BlockFlow {
         }
     }
 
+    fn build_display_list_for_block(&mut self,
+                                    display_list: Box<DisplayList>,
+                                    layout_context: &LayoutContext) {
+        if self.base.flags.is_float() {
+            // TODO(#2009, pcwalton): This is a pseudo-stacking context. We need to merge `z-index:
+            // auto` kids into the parent stacking context, when that is supported.
+            self.build_display_list_for_floating_block(display_list, layout_context)
+        } else if self.base.flags.contains(IS_ABSOLUTELY_POSITIONED) {
+            self.build_display_list_for_absolutely_positioned_block(display_list, layout_context)
+        } else {
+            self.build_display_list_for_static_block(display_list, layout_context, BlockLevel)
+        }
+    }
+
     fn create_stacking_context(&self,
                                display_list: Box<DisplayList>,
                                layer: Option<Arc<PaintLayer>>)
@@ -1017,6 +1040,35 @@ impl BlockFlowDisplayListBuilding for BlockFlow {
     }
 }
 
+pub trait ListItemFlowDisplayListBuilding {
+    fn build_display_list_for_list_item(&mut self,
+                                        display_list: Box<DisplayList>,
+                                        layout_context: &LayoutContext);
+}
+
+impl ListItemFlowDisplayListBuilding for ListItemFlow {
+    fn build_display_list_for_list_item(&mut self,
+                                        mut display_list: Box<DisplayList>,
+                                        layout_context: &LayoutContext) {
+        // Draw the marker, if applicable.
+        match self.marker {
+            None => {}
+            Some(ref mut marker) => {
+                let stacking_relative_fragment_origin =
+                    self.block_flow.base.stacking_relative_position_of_child_fragment(marker);
+                marker.build_display_list(&mut *display_list,
+                                          layout_context,
+                                          stacking_relative_fragment_origin,
+                                          ContentLevel,
+                                          &self.block_flow.base.clip_rect);
+            }
+        }
+
+        // Draw the rest of the block.
+        self.block_flow.build_display_list_for_block(display_list, layout_context)
+    }
+}
+
 // A helper data structure for gradients.
 struct StopRun {
     start_offset: f32,
diff --git a/components/layout/flow.rs b/components/layout/flow.rs
index bfca9c6168c..20645e70123 100644
--- a/components/layout/flow.rs
+++ b/components/layout/flow.rs
@@ -430,6 +430,7 @@ pub trait MutableOwnedFlowUtils {
 pub enum FlowClass {
     BlockFlowClass,
     InlineFlowClass,
+    ListItemFlowClass,
     TableWrapperFlowClass,
     TableFlowClass,
     TableColGroupFlowClass,
diff --git a/components/layout/lib.rs b/components/layout/lib.rs
index 2d045508088..1fa21311373 100644
--- a/components/layout/lib.rs
+++ b/components/layout/lib.rs
@@ -52,6 +52,7 @@ pub mod flow_ref;
 pub mod fragment;
 pub mod layout_task;
 pub mod inline;
+pub mod list_item;
 pub mod model;
 pub mod parallel;
 pub mod sequential;
diff --git a/components/layout/list_item.rs b/components/layout/list_item.rs
new file mode 100644
index 00000000000..fbf40c7747c
--- /dev/null
+++ b/components/layout/list_item.rs
@@ -0,0 +1,135 @@
+/* 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/. */
+
+//! Layout for elements with a CSS `display` property of `list-item`. These elements consist of a
+//! block and an extra inline fragment for the marker.
+
+#![deny(unsafe_blocks)]
+
+use block::BlockFlow;
+use construct::FlowConstructor;
+use context::LayoutContext;
+use display_list_builder::ListItemFlowDisplayListBuilding;
+use flow::{Flow, FlowClass, ListItemFlowClass};
+use fragment::{Fragment, FragmentBoundsIterator};
+use wrapper::ThreadSafeLayoutNode;
+
+use gfx::display_list::DisplayList;
+use servo_util::geometry::Au;
+use servo_util::opts;
+use style::ComputedValues;
+use style::computed_values::list_style_type;
+use sync::Arc;
+
+/// A block with the CSS `display` property equal to `list-item`.
+#[deriving(Show)]
+pub struct ListItemFlow {
+    /// Data common to all block flows.
+    pub block_flow: BlockFlow,
+    /// The marker, if outside. (Markers that are inside are instead just fragments on the interior
+    /// `InlineFlow`.)
+    pub marker: Option<Fragment>,
+}
+
+impl ListItemFlow {
+    pub fn from_node_and_marker(constructor: &mut FlowConstructor,
+                                node: &ThreadSafeLayoutNode,
+                                marker_fragment: Option<Fragment>)
+                                -> ListItemFlow {
+        ListItemFlow {
+            block_flow: BlockFlow::from_node(constructor, node),
+            marker: marker_fragment,
+        }
+    }
+}
+
+impl Flow for ListItemFlow {
+    fn class(&self) -> FlowClass {
+        ListItemFlowClass
+    }
+
+    fn as_block<'a>(&'a mut self) -> &'a mut BlockFlow {
+        &mut self.block_flow
+    }
+
+    fn bubble_inline_sizes(&mut self) {
+        // The marker contributes no intrinsic inline-size, so…
+        self.block_flow.bubble_inline_sizes()
+    }
+
+    fn assign_inline_sizes(&mut self, layout_context: &LayoutContext) {
+        self.block_flow.assign_inline_sizes(layout_context);
+
+        match self.marker {
+            None => {}
+            Some(ref mut marker) => {
+                // Do this now. There's no need to do this in bubble-widths, since markers do not
+                // contribute to the inline size of this flow.
+                let intrinsic_inline_sizes = marker.compute_intrinsic_inline_sizes();
+
+                marker.border_box.size.inline =
+                    intrinsic_inline_sizes.content_intrinsic_sizes.preferred_inline_size;
+                marker.border_box.start.i = self.block_flow.fragment.border_box.start.i -
+                    marker.border_box.size.inline;
+            }
+        }
+    }
+
+    fn assign_block_size<'a>(&mut self, layout_context: &'a LayoutContext<'a>) {
+        self.block_flow.assign_block_size(layout_context);
+
+        match self.marker {
+            None => {}
+            Some(ref mut marker) => {
+                marker.border_box.start.b = Au(0);
+                marker.border_box.size.block = marker.calculate_line_height(layout_context);
+            }
+        }
+    }
+
+    fn compute_absolute_position(&mut self) {
+        self.block_flow.compute_absolute_position()
+    }
+
+    fn update_late_computed_inline_position_if_necessary(&mut self, inline_position: Au) {
+        self.block_flow.update_late_computed_inline_position_if_necessary(inline_position)
+    }
+
+    fn update_late_computed_block_position_if_necessary(&mut self, block_position: Au) {
+        self.block_flow.update_late_computed_block_position_if_necessary(block_position)
+    }
+
+    fn build_display_list(&mut self, layout_context: &LayoutContext) {
+        self.build_display_list_for_list_item(box DisplayList::new(), layout_context);
+        if opts::get().validate_display_list_geometry {
+            self.block_flow.base.validate_display_list_geometry();
+        }
+    }
+
+    fn repair_style(&mut self, new_style: &Arc<ComputedValues>) {
+        self.block_flow.repair_style(new_style)
+    }
+
+    fn iterate_through_fragment_bounds(&self, iterator: &mut FragmentBoundsIterator) {
+        self.block_flow.iterate_through_fragment_bounds(iterator);
+    }
+}
+
+/// Returns the static text to be used for the given value of the `list-style-type` property.
+///
+/// TODO(pcwalton): Return either a string or a counter descriptor, once we support counters.
+pub fn static_text_for_list_style_type(list_style_type: list_style_type::T)
+                                       -> Option<&'static str> {
+    // Just to keep things simple, use a nonbreaking space (Unicode 0xa0) to provide the marker
+    // separation.
+    match list_style_type {
+        list_style_type::none => None,
+        list_style_type::disc => Some("•\u00a0"),
+        list_style_type::circle => Some("◦\u00a0"),
+        list_style_type::square => Some("▪\u00a0"),
+        list_style_type::disclosure_open => Some("▾\u00a0"),
+        list_style_type::disclosure_closed => Some("‣\u00a0"),
+    }
+}
+
diff --git a/components/layout/table_cell.rs b/components/layout/table_cell.rs
index 8dd0817b6da..280e86408cb 100644
--- a/components/layout/table_cell.rs
+++ b/components/layout/table_cell.rs
@@ -23,7 +23,7 @@ use sync::Arc;
 /// A table formatting context.
 #[deriving(Encodable)]
 pub struct TableCellFlow {
-    /// Data common to all flows.
+    /// Data common to all block flows.
     pub block_flow: BlockFlow,
 }
 
diff --git a/components/style/properties/mod.rs.mako b/components/style/properties/mod.rs.mako
index 9453b32f05c..2107ac86c7b 100644
--- a/components/style/properties/mod.rs.mako
+++ b/components/style/properties/mod.rs.mako
@@ -398,7 +398,7 @@ pub mod longhands {
 
     ${new_style_struct("Box", is_inherited=False)}
 
-    // TODO: don't parse values we don't support
+    // TODO(SimonSapin): don't parse `inline-table`, since we don't support it
     <%self:single_keyword_computed name="display"
             values="inline block inline-block
             table inline-table table-row-group table-header-group table-footer-group
@@ -698,6 +698,45 @@ pub mod longhands {
                 Ok(Content(content))
             }
     </%self:longhand>
+
+    ${new_style_struct("List", is_inherited=True)}
+
+    ${single_keyword("list-style-position", "outside inside")}
+
+    // TODO(pcwalton): Implement the full set of counter styles per CSS-COUNTER-STYLES [1] 6.1:
+    //
+    //     decimal, decimal-leading-zero, arabic-indic, armenian, upper-armenian, lower-armenian,
+    //     bengali, cambodian, khmer, cjk-decimal, devanagiri, georgian, gujarati, gurmukhi,
+    //     hebrew, kannada, lao, malayalam, mongolian, myanmar, oriya, persian, lower-roman,
+    //     upper-roman, telugu, thai, tibetan
+    //
+    // [1]: http://dev.w3.org/csswg/css-counter-styles/
+    ${single_keyword("list-style-type",
+                     "disc none circle square disclosure-open disclosure-closed")}
+
+    <%self:single_component_value name="list-style-image">
+        pub use super::computed_as_specified as to_computed_value;
+        #[deriving(Clone)]
+        pub type SpecifiedValue = Option<Url>;
+        pub mod computed_value {
+            use url::Url;
+            #[deriving(Clone, PartialEq)]
+            pub type T = Option<Url>;
+        }
+        pub fn from_component_value(input: &ComponentValue, base_url: &Url)
+                                    -> Result<SpecifiedValue,()> {
+            match *input {
+                URL(ref url) => Ok(Some(super::parse_url(url.as_slice(), base_url))),
+                Ident(ref value) if value.as_slice().eq_ignore_ascii_case("none") => Ok(None),
+                _ => Err(()),
+            }
+        }
+        #[inline]
+        pub fn get_initial_value() -> computed_value::T {
+            None
+        }
+    </%self:single_component_value>
+
     // CSS 2.1, Section 13 - Paged media
 
     // CSS 2.1, Section 14 - Colors and Backgrounds
@@ -1903,6 +1942,102 @@ pub mod shorthands {
             }
         })
     </%self:shorthand>
+
+    <%self:shorthand name="list-style"
+                     sub_properties="list-style-image list-style-position list-style-type">
+        // `none` is ambiguous until we've finished parsing the shorthands, so we count the number
+        // of times we see it.
+        let mut nones = 0u8;
+        let (mut image, mut position, mut list_style_type, mut any) = (None, None, None, false);
+        for component_value in input.skip_whitespace() {
+            match component_value {
+                &Ident(ref value) if value.eq_ignore_ascii_case("none") => {
+                    nones = nones + 1;
+                    if nones > 2 {
+                        return Err(())
+                    }
+                    any = true;
+                    continue
+                }
+                _ => {}
+            }
+
+            if list_style_type.is_none() {
+                match list_style_type::from_component_value(component_value, base_url) {
+                    Ok(v) => {
+                        list_style_type = Some(v);
+                        any = true;
+                        continue
+                    },
+                    Err(()) => ()
+                }
+            }
+
+            if image.is_none() {
+                match list_style_image::from_component_value(component_value, base_url) {
+                    Ok(v) => {
+                        image = Some(v);
+                        any = true;
+                        continue
+                    },
+                    Err(()) => (),
+                }
+            }
+
+            if position.is_none() {
+                match list_style_position::from_component_value(component_value, base_url) {
+                    Ok(v) => {
+                        position = Some(v);
+                        any = true;
+                        continue
+                    },
+                    Err(()) => ()
+                }
+            }
+        }
+
+        // If there are two `none`s, then we can't have a type or image; if there is one `none`,
+        // then we can't have both a type *and* an image; if there is no `none` then we're fine as
+        // long as we parsed something.
+        match (any, nones, list_style_type, image) {
+            (true, 2, None, None) => {
+                Ok(Longhands {
+                    list_style_position: position,
+                    list_style_image: Some(None),
+                    list_style_type: Some(list_style_type::none),
+                })
+            }
+            (true, 1, None, Some(image)) => {
+                Ok(Longhands {
+                    list_style_position: position,
+                    list_style_image: Some(image),
+                    list_style_type: Some(list_style_type::none),
+                })
+            }
+            (true, 1, Some(list_style_type), None) => {
+                Ok(Longhands {
+                    list_style_position: position,
+                    list_style_image: Some(None),
+                    list_style_type: Some(list_style_type),
+                })
+            }
+            (true, 1, None, None) => {
+                Ok(Longhands {
+                    list_style_position: position,
+                    list_style_image: Some(None),
+                    list_style_type: Some(list_style_type::none),
+                })
+            }
+            (true, 0, list_style_type, image) => {
+                Ok(Longhands {
+                    list_style_position: position,
+                    list_style_image: image,
+                    list_style_type: list_style_type,
+                })
+            }
+            _ => Err(()),
+        }
+    </%self:shorthand>
 }
 
 
diff --git a/tests/ref/basic.list b/tests/ref/basic.list
index 49a0ff9e959..d4b0571a7da 100644
--- a/tests/ref/basic.list
+++ b/tests/ref/basic.list
@@ -207,3 +207,5 @@ fragment=top != ../html/acid2.html acid2_ref.html
 == box_shadow_spread_a.html box_shadow_spread_ref.html
 == box_shadow_inset_a.html box_shadow_inset_ref.html
 == box_shadow_inset_parsing_a.html box_shadow_inset_parsing_ref.html
+!= list_style_type_a.html list_style_type_ref.html
+== list_style_position_a.html list_style_position_ref.html
diff --git a/tests/ref/list_style_position_a.html b/tests/ref/list_style_position_a.html
new file mode 100644
index 00000000000..eb32fb5de0e
--- /dev/null
+++ b/tests/ref/list_style_position_a.html
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<!--
+    Tests that `list-style-position: inside` and `list-style-image` work. This is deliberately
+    conservative because the exact placement of the marker is unspecified.
+-->
+<html>
+<head>
+<style>
+ul {
+    margin: 0;
+    padding: 0;
+}
+li {
+    list-style-image: url(smiling.png);
+    list-style-position: inside;
+    margin: 0;
+    padding: 0;
+}
+</style>
+</head>
+<body>
+<ul>
+<li></li>
+<li></li>
+<li></li>
+</ul>
+</body>
+</html>
+
diff --git a/tests/ref/list_style_position_ref.html b/tests/ref/list_style_position_ref.html
new file mode 100644
index 00000000000..d637d4b495c
--- /dev/null
+++ b/tests/ref/list_style_position_ref.html
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<!--
+    Tests that `list-style-position: inside` and `list-style-image` work. This is deliberately
+    conservative because the exact placement of the marker is unspecified.
+-->
+<html>
+<head>
+<style>
+ul {
+    margin: 0;
+    padding: 0;
+}
+li {
+    display: block;
+    margin: 0;
+    padding: 0;
+}
+</style>
+</head>
+<body>
+<ul>
+<li><img src=smiling.png></li>
+<li><img src=smiling.png></li>
+<li><img src=smiling.png></li>
+</ul>
+</body>
+</html>
+
diff --git a/tests/ref/list_style_type_a.html b/tests/ref/list_style_type_a.html
new file mode 100644
index 00000000000..f0bad6fde52
--- /dev/null
+++ b/tests/ref/list_style_type_a.html
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<!--
+    Tests that `list-style-type` does something. This is deliberately conservative because the
+    exact placement of the marker is unspecified.
+-->
+<html>
+<head>
+<style>
+li {
+    list-style: square;
+}
+</style>
+<body>
+<ul>
+<li>Cheetahmen</li>
+</ul>
+</body>
+</html>
+
diff --git a/tests/ref/list_style_type_ref.html b/tests/ref/list_style_type_ref.html
new file mode 100644
index 00000000000..d9ce190ca18
--- /dev/null
+++ b/tests/ref/list_style_type_ref.html
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<!--
+    Tests that `list-style-type` does something. This is deliberately conservative because the
+    exact placement of the marker is unspecified.
+-->
+<html>
+<head>
+<body>
+<ul>
+<li>Cheetahmen</li>
+</ul>
+</body>
+</html>
+
diff --git a/tests/ref/smiling.png b/tests/ref/smiling.png
new file mode 100644
index 0000000000000000000000000000000000000000..725eef526d704d45f1bff18a035d666fc08f1445
GIT binary patch
literal 3390
zcmV-E4Z-q>P)<h;3K|Lk000e1NJLTq000jF000jN1^@s6JJyv-00009a7bBm000XU
z000XU0RWnu7ytkYPiaF#P*7-ZbZ>KLZ*U+<Lqi~Na&Km7Y-Iodc-oy)XH-+^7Crag
z^g>IBfRsybQWXdwQbLP>6p<z>Aqfylh#{fb6;Z(vMMVS~$e@S=j*ftg6;Uh<iVD~V
z<RPMtgQJLw%KPDaqifc@_vX$1wbwr9tn;0-&j-K=43<bUQ8j=JsX`tR;Dg7+#^K~H
zK!FM*Z~zbpvt%K2{UZSY_<lS*D<Z%Lz5oGu(+dayz)hRLFdT>f59&ghTmgWD0l;*T
zI7<kC6aYYajzXpYKt=(8otP$50H6c_V9R4-;{Z@C0AMG7=F<Rxo%or10RUT+Ar%3j
zkpLhQWr#!oXgdI`&sK^>09Y^p6lP1rIRMx#05C~cW=H_Aw*bJ-5DT&Z2n+x)QHX^p
z00esgV8|mQcmRZ%02D^@S3L16t`O%c004NIvOKvYIYoh62rY33S640`D9%Y2D-<?i
z0%4j!F2Z@488U%158(66005wo6%pWr^Zj_v4zAA5HjcIqUoGmt2LB>rV&neh&#Q1i
z007~1e$oCcFS8neI|hJl{-P!B1ZZ9hpmq0)X0i`JwE&>$+E?>%_<lS*MWK+n+1cgf
z<k(8YLR(?VSAG6x!e78w{cQPuJpA|d;J)G{fihizM+Erb!p!tcr5w+a34~(Y=8s4G
zw+sLL9n&JjNn*KJDiq^U5^;`1nvC-@r6P$!k}1U{(*I=Q-z@tBKHoI}uxdU5dyy@u
zU1J0GOD7Ombim^G008p4Z^6_k2m^p<gW=D2|L;HjN1!DDfM!XOaR2~bL?kX$%CkSm
z2mk;?pn)o|K^yeJ7%adB9Ki+L!3+FgHiSYX#KJ-lLJDMn9CBbOtb#%)hRv`YDqt_v
zKpix|QD}yfa1JiQRk#j4a1Z)n2%f<xynzV>LC6RbVIkUx0b+_+BaR3cnT7Zv!AJxW
zizFb)h!jyGOOZ85F;a?DAXP{m@;!0_Ifq<Ex{*7`05XF7hP+2Hl!3BQJ=6@fL%FCo
z8iYoo3(#bAF`ADSpqtQgv>H8(HlgRxt7s3}k3K`kFu>>-2Q$QMFfPW!La{h336o>X
zu_CMttHv6zR;&ZNiS=X8v3CR#fknUxHUxJ<AYmRsNLWl*PS{AOARHt#5!wki2?K;t
z!Y3k=s7tgax)J%r7-BLphge7~Bi0g+6E6^Zh(p9TBoc{3GAFr^0!gu?RMHaCM$&Fl
zBk3%un>0uoBa_M6WNWeqIg~6QE69c9o#eyhGvpiOA@W-aonk<7r1(?fC{oI5N*U!4
z<uv66WtcKSRim0x-Ke2d5jBrmLam{;Qm;{ms1r1GnmNsb7D-E`t)i9F8fX`2_i3-_
zbh;7Ul^#x)&{xvS=|||7=mYe33=M`AgU5(xC>fg=2N-7=cNnjjOr{yriy6mMFgG#l
znCF=fnQv8CDz++o6_Lscl}eQ+l^ZHARH>?_s@|##Rr6KLRFA1%Q+=*RRWnoLsR`7U
zt5vF<Q0r40Q)j6=sE4X&sBct1q<&fbi3VB2Ov6t@q*0);U*o*SAPZv|vv@2aYYnT0
zb%8a+Cb7-ge0D0knEf5Qi#@8Tp*ce{N;6lpQuCB%KL_KOarm5cP6_8Ir<e17iry6O
zDdH&`rZh~sF=bq9s+O0QSgS~@QL9Jmy*94xr=6y~MY~!1fet~(N+(<=M`w@D1)b+p
z*;C!83a1uLJv#NSE~;y#8=<>IcfW3@?wFpwUVxrVZ>QdQz32KIeJ}k~{cZZE^+ya?
z2D1z#2HOnI7(B%_ac?{wFUQ;QQA1tBKtrWrm0_3Rgps+?Jfqb{jYbcQX~taRB;#$y
zZN{S}1|}gUOHJxc?wV3fxuz+mJ4`!F$IZ;mqRrNsHJd##*D~ju=bP7?-?v~|cv>vB
zsJ6IeNwVZxrdjT`yl#bBIa#GxRa#xMMy;K#CDyyGyQdMSxlWT#tDe?p!?5wT$+oGt
z8L;Kp2HUQ-ZMJ=3XJQv;x5ci*?vuTfeY$;({XGW_huIFR9a<fJbF^|4I#xQ~n$Dc=
zKYhjYmgz5NSkDm8*fZm{6U!;YX`NG>(?@3)XSs8O^N5RyOM=TTmp(3=8^+zpz2r)C
z^>JO{deZfso3oq3?Wo(Y?l$ge?uXo;%ru`Vo>?<<(8I_>;8Eq#KMS9gFl*neeosSB
zfoHYnBQIkwkyowPu(zdms`p{<7e4kra-ZWq<2*OsGTvEV%s0Td$hXT+!*8Bnh2KMe
zBmZRodjHV?r+_5^X9J0WL4jKW`}lf%A-|44I@@LTvf1rHjG(ze6+w@Jt%Bvjts!X0
z?2xS?_ve_-k<Mujg;0Lz*3buG=3$G&ehepthlN*$KaOySSQ^nWmo<0M+(UEUMEXRQ
zMBbZcF;6+KElM>iKB_KiJlZ$9G`c^=E@oNG)mWWaNo-3TIW8)$Hg0Ub-~8?KhvJ>$
z3*&nim@mj(aCxE5!t{lw7O5^0EIO7zOo&c6l<+|iDySBWCGrz@C5{St!X3hAA}`T4
z(TLbXTq+(;@<=L8dXnssyft|w#WSTW<++3>sgS%(4NTpeI-VAqb|7ssJvzNHgOZVu
zaYCvgO_R1~>SyL=cFU|~g|hy|Zi}}s9+d~lYqOB71z9Z$wnC=pR9Yz4DhIM>Wmjgu
z&56o6maCpC&F##y%G;1PobR9i?GnNg;gYtchD%p19a!eQtZF&3JaKv33gZ<8D~47E
ztUS1iwkmDaPpj=$m#%)jCVEY4fnLGNg2A-`YwHVD3gv};>)hAvT~AmqS>Lr``i7kw
zJ{5_It`yrBmlc25DBO7E8;5VoznR>Ww5hAaxn$2~(q`%A-YuS64wkBy=9dm`4cXeX
z4c}I@?e+FW+b@^RDBHV(wnMq2zdX3SWv9u`%{xC-q*U}&`cyXV(%rRT*Z6MH?i+i&
z_B8C(+grT%{XWUQ+f@NoP1R=AW&26{v-dx)iK^-Nmiuj8txj!m?Z*Ss1N{dh4z}01
z)YTo*JycSU)+_5r4#yw9{+;i4Ee$peRgIj+;v;ZGdF1K$3E%e~4LaI(jC-u%2h$&R
z9cLXcYC@Xwnns&bn)_Q~Te?roKGD|d-g^8;+aC{{G(1^(O7m37Y1-+6)01cN&y1aw
zoqc{T`P^XJqPBbIW6s}d4{z_f5Om?vMgNQEJG?v2T=KYd^0M3I6IZxbny)%vZR&LD
zJpPl@Psh8QyPB@KTx+@RdcC!KX7}kEo;S|j^u2lU7XQ}Oo;f|;z4Ll+_r>@1-xl3|
zawq-H%e&ckC+@AhPrP6BK<z=<L*0kfKU@CX*zeqbYQT4(^U>T#_XdT7&;F71j}Joy
zkC~6lh7E@6o;W@^IpRNZ{ptLtL(gQ-CY~4mqW;US7Zxvm_|@yz&e53Bp_lTPlfP|z
zrTyx_>lv@x#=^!PzR7qqF<$gm`|ZJZ+;<)Cqu&ot<a{81DF0~rvGr5Xr~8u`lav1h
z1DNytV>2z=0000WV@Og>004R=004l4008;_004mL004C`008P>0026e000+nl3&F}
z0007MNkl<Zc-mBx-%FEW6vsdBJGBX65KUc}UF6J>f*{N;%xP&}2vN{YLC{s*beoX6
zf$xm;PpGS)D^aK}ya+GMS_PKBAk(&yOOS02yK<Z6-gCN`&H2E?1BdVDoG<4&$52&D
zPcRth2l|0FpcPmK7JzBX3Qd=4jsI{k7;pj48yg#Mc|47{-EL}XYe*&%BoYZC(Fn0<
z>^1PvvO?)191I2;fUw`+Rnyhgg`%K}s-jSMRrxSCM>rf#0j-u5N;*Vr2k`ECU!Uhn
zN5`QM6fHukg=F>T>%rjU=H}*w?(VA-4!^(iPD@M6qu!q0W57}6(j-D%T^&0+JKm|u
z$#oOBd#Sk@Q9(r*4TX3*I*O<it_Wk##uy(PM??@&yk0NBJrnSqb-Ph1h>M*x3{(}7
zqnN{VpeQca8GyDjz~!uT7C?n3gM*lcap<IAbztBzhG`yFwkfYD2RK~@q<43BU6qxU
z6n;&yq3A?a5d^hC#70EO<#GUjOkjR%YYP!!e{YX?JWk#|AaCcfZ5!LRImjOn-`F6R
z%VFC#sZ<JJ!2~AO*47XevU^!(W@ZR~43o)b5EU|+44*%JVRrUCfA{xMgrDm_0bV)$
z{?65OI(?(!L`7|V{W-i%O(c^kmKT?ZM506^F{(~g(bIbkr_;&e!V;_Bf4mtPu>!>j
zGfgwxe!0EI*Y3kG01N<B3r#x6=UG`=;oH}RZNo5Hhlc}+W3z3V=8FbT!_6k|MO@X@
zRFs#K&1Tr%P7{m9*!cDPt;1nH7#bShKDHPoTXh?_5BLBVkOt;~mzEWpD%JiQ0N+<~
UjUj=oeE<Le07*qoM6N<$f&?f>Z~y=R

literal 0
HcmV?d00001