layout: Address review feedback.

src/components/gfx/display_list.rs

@@ -23,8 +23,6 @@ use servo_net::image::base::Image;
 use servo_util::geometry::Au;
 use servo_util::range::Range;
 use servo_util::smallvec::{SmallVec, SmallVec0, SmallVecIterator};
-use std::cast::transmute_region;
-use std::cast;
 use std::libc::uintptr_t;
 use std::mem;
 use std::vec::Items;
@@ -43,9 +41,8 @@ pub struct OpaqueNode(uintptr_t);
 impl OpaqueNode {
     /// Returns the address of this node, for debugging purposes.
     pub fn id(&self) -> uintptr_t {
-        unsafe {
+        let OpaqueNode(pointer) = *self;
-            cast::transmute_copy(self)
+        pointer
-        }
     }
 }
 
@@ -428,16 +425,13 @@ impl DisplayItem {
     }
 
     pub fn base<'a>(&'a self) -> &'a BaseDisplayItem {
-        // FIXME(tkuehn): Workaround for Rust region bug.
-        unsafe {
         match *self {
-                SolidColorDisplayItemClass(ref solid_color) => transmute_region(&solid_color.base),
+            SolidColorDisplayItemClass(ref solid_color) => &solid_color.base,
-                TextDisplayItemClass(ref text) => transmute_region(&text.base),
+            TextDisplayItemClass(ref text) => &text.base,
-                ImageDisplayItemClass(ref image_item) => transmute_region(&image_item.base),
+            ImageDisplayItemClass(ref image_item) => &image_item.base,
-                BorderDisplayItemClass(ref border) => transmute_region(&border.base),
+            BorderDisplayItemClass(ref border) => &border.base,
-                LineDisplayItemClass(ref line) => transmute_region(&line.base),
+            LineDisplayItemClass(ref line) => &line.base,
-                ClipDisplayItemClass(ref clip) => transmute_region(&clip.base),
+            ClipDisplayItemClass(ref clip) => &clip.base,
-            }
         }
     }
 

src/components/gfx/render_task.rs

@@ -40,9 +40,9 @@ pub struct RenderLayer {
     /// The display list describing the contents of this layer.
     display_list: Arc<DisplayList>,
     /// The position of the layer in pixels.
-    rect: Rect<uint>,
+    position: Rect<uint>,
     /// The color of the background in this layer. Used for unrendered content.
-    color: Color,
+    background_color: Color,
     /// The scrolling policy of this layer.
     scroll_policy: ScrollPolicy,
 }
@@ -73,7 +73,7 @@ pub fn BufferRequest(screen_rect: Rect<uint>, page_rect: Rect<f32>) -> BufferReq
     }
 }
 
-// FIXME(pcwalton): This should be a newtype struct.
+// FIXME(#2005, pcwalton): This should be a newtype struct.
 pub struct RenderChan {
     chan: Chan<Msg>,
 }
@@ -157,8 +157,8 @@ fn initialize_layers<C:RenderListener>(
     let metadata = render_layers.iter().map(|render_layer| {
         LayerMetadata {
             id: render_layer.id,
-            rect: render_layer.rect,
+            position: render_layer.position,
-            color: render_layer.color,
+            background_color: render_layer.background_color,
             scroll_policy: render_layer.scroll_policy,
         }
     }).collect();
@@ -297,14 +297,7 @@ impl<C: RenderListener + Send> RenderTask<C> {
             let mut new_buffers = ~[];
 
             // Find the appropriate render layer.
-            let mut render_layer = None;
+            let render_layer = match self.render_layers.iter().find(|layer| layer.id == layer_id) {
-            for layer in self.render_layers.iter() {
-                if layer.id == layer_id {
-                    render_layer = Some(layer);
-                    break
-                }
-            }
-            let render_layer = match render_layer {
                 Some(render_layer) => render_layer,
                 None => return,
             };
@@ -349,8 +342,8 @@ impl<C: RenderListener + Send> RenderTask<C> {
                     let matrix = matrix.scale(scale as AzFloat, scale as AzFloat);
                     let matrix = matrix.translate(-(tile.page_rect.origin.x) as AzFloat,
                                                   -(tile.page_rect.origin.y) as AzFloat);
-                    let matrix = matrix.translate(-(render_layer.rect.origin.x as AzFloat),
+                    let matrix = matrix.translate(-(render_layer.position.origin.x as AzFloat),
-                                                  -(render_layer.rect.origin.y as AzFloat));
+                                                  -(render_layer.position.origin.y as AzFloat));
 
                     ctx.draw_target.set_transform(&matrix);
 

src/components/main/compositing/compositor.rs

@@ -326,7 +326,7 @@ impl IOCompositor {
         }
     }
 
-    // FIXME(pcwalton): Take the pipeline ID and layer ID into account.
+    // FIXME(#2004, pcwalton): Take the pipeline ID and layer ID into account.
     fn set_unrendered_color(&mut self, _: PipelineId, _: LayerId, color: Color) {
         match self.compositor_layer {
             Some(ref mut layer) => layer.unrendered_color = color,

src/components/main/compositing/compositor_layer.rs

@@ -41,7 +41,7 @@ static MAX_TILE_MEMORY_PER_LAYER: uint = 10000000;
 /// or animation behavior. This can include absolute positioned elements, iframes, etc.
 /// Each layer can also have child layers.
 ///
-/// FIXME(pcwalton): This should be merged with the concept of a layer in `rust-layers` and
+/// FIXME(#2003, pcwalton): This should be merged with the concept of a layer in `rust-layers` and
 /// ultimately removed, except as a set of helper methods on `rust-layers` layers.
 pub struct CompositorLayer {
     /// This layer's pipeline. BufferRequests and mouse events will be sent through this.
@@ -248,12 +248,12 @@ impl CompositorLayer {
         }
 
         // See if we've already made this child layer.
-        for kid_holder in self.children.iter() {
+        if self.children.iter().any(|kid_holder| {
-            if kid_holder.child.pipeline.id == pipeline_id &&
+                    kid_holder.child.pipeline.id == pipeline_id &&
-                    kid_holder.child.id == child_layer_id {
+                    kid_holder.child.id == child_layer_id
+                }) {
             return true
         }
-        }
 
         let mut kid = ~CompositorLayer::new(self.pipeline.clone(),
                                             child_layer_id,
@@ -452,8 +452,8 @@ impl CompositorLayer {
                 if !request.is_empty() {
                     // Ask for tiles.
                     //
-                    // FIXME(pcwalton): We may want to batch these up in the case in which one
+                    // FIXME(#2003, pcwalton): We may want to batch these up in the case in which
-                    // page has multiple layers, to avoid the user seeing inconsistent states.
+                    // one page has multiple layers, to avoid the user seeing inconsistent states.
                     let msg = ReRenderMsg(request, scale, self.id, self.epoch);
                     self.pipeline.render_chan.try_send(msg);
                 }
@@ -600,12 +600,9 @@ impl CompositorLayer {
                 -> bool {
         // Search children for the right layer to move.
         if self.pipeline.id != pipeline_id || self.id != layer_id {
-            for kid_holder in self.children.mut_iter() {
+            return self.children.mut_iter().any(|kid_holder| {
-                if kid_holder.child.move(pipeline_id, layer_id, origin, window_size) {
+                kid_holder.child.move(pipeline_id, layer_id, origin, window_size)
-                    return true
+            })
-                }
-            }
-            return false
         }
 
         if self.wants_scroll_events != WantsScrollEvents {
@@ -876,7 +873,6 @@ impl CompositorLayer {
             };
 
             let mut unused_tiles = ~[];
-            // `move_rev_iter` is more efficient than `.move_iter().reverse()`.
             for buffer in new_buffers.buffers.move_rev_iter() {
                 unused_tiles.push_all_move(quadtree.add_tile_pixel(buffer.screen_pos.origin.x,
                                                                    buffer.screen_pos.origin.y,
@@ -1013,10 +1009,7 @@ impl CompositorLayer {
     }
 
     pub fn id_of_first_child(&self) -> LayerId {
-        for kid_holder in self.children.iter() {
+        self.children.iter().next().expect("no first child!").child.id
-            return kid_holder.child.id
-        }
-        fail!("no first child!");
     }
 }
 

src/components/main/compositing/compositor_task.rs

@@ -94,13 +94,15 @@ impl RenderListener for CompositorChan {
                                       pipeline_id: PipelineId,
                                       metadata: ~[LayerMetadata],
                                       epoch: Epoch) {
-        // FIXME(pcwalton): This assumes that the first layer determines the page size, and that
+        // FIXME(#2004, pcwalton): This assumes that the first layer determines the page size, and
-        // all other layers are immediate children of it. This is sufficient to handle
+        // that all other layers are immediate children of it. This is sufficient to handle
         // `position: fixed` but will not be sufficient to handle `overflow: scroll` or transforms.
         let mut first = true;
         for metadata in metadata.iter() {
-            let origin = Point2D(metadata.rect.origin.x as f32, metadata.rect.origin.y as f32);
+            let origin = Point2D(metadata.position.origin.x as f32,
-            let size = Size2D(metadata.rect.size.width as f32, metadata.rect.size.height as f32);
+                                 metadata.position.origin.y as f32);
+            let size = Size2D(metadata.position.size.width as f32,
+                              metadata.position.size.height as f32);
             let rect = Rect(origin, size);
             if first {
                 self.chan.send(CreateRootCompositorLayerIfNecessary(pipeline_id,
@@ -115,7 +117,9 @@ impl RenderListener for CompositorChan {
                                                                      metadata.scroll_policy));
             }
 
-            self.chan.send(SetUnRenderedColor(pipeline_id, metadata.id, metadata.color));
+            self.chan.send(SetUnRenderedColor(pipeline_id,
+                                              metadata.id,
+                                              metadata.background_color));
             self.chan.send(SetLayerPageSize(pipeline_id, metadata.id, size, epoch));
         }
     }

src/components/main/css/node_util.rs

@@ -27,7 +27,7 @@ impl<'ln> NodeUtil for ThreadSafeLayoutNode<'ln> {
             let layout_data_ref = self.borrow_layout_data();
             match self.get_element_type() {
                 Before | BeforeBlock => {
-                     return cast::transmute_region(layout_data_ref.get()
+                     cast::transmute_region(layout_data_ref.get()
                                                            .as_ref()
                                                            .unwrap()
                                                            .data
@@ -36,7 +36,7 @@ impl<'ln> NodeUtil for ThreadSafeLayoutNode<'ln> {
                                                            .unwrap())
                 }
                 After | AfterBlock => {
-                    return cast::transmute_region(layout_data_ref.get()
+                    cast::transmute_region(layout_data_ref.get()
                                                           .as_ref()
                                                           .unwrap()
                                                           .data
@@ -45,7 +45,7 @@ impl<'ln> NodeUtil for ThreadSafeLayoutNode<'ln> {
                                                           .unwrap())
                 }
                 Normal => {
-                    return cast::transmute_region(layout_data_ref.get()
+                    cast::transmute_region(layout_data_ref.get()
                                                           .as_ref()
                                                           .unwrap()
                                                           .data

src/components/main/layout/block.rs

@@ -12,7 +12,7 @@
 //!
 //! CB: Containing Block of the current flow.
 
-use layout::box_::{Box, ImageBox, MainBoxKind, ScannedTextBox};
+use layout::box_::{Box, ImageBox, ScannedTextBox};
 use layout::construct::FlowConstructor;
 use layout::context::LayoutContext;
 use layout::display_list_builder::{DisplayListBuilder, DisplayListBuildingInfo};
@@ -459,10 +459,10 @@ pub enum MarginsMayCollapseFlag {
 //
 // If any fixed descendants of kids are present, this kid needs a layer.
 //
-// FIXME(pcwalton): This is too layer-happy. Like WebKit, we shouldn't do this unless
+// FIXME(#2006, pcwalton): This is too layer-happy. Like WebKit, we shouldn't do this unless
 // the positioned descendants are actually on top of the fixed kids.
 //
-// TODO(pcwalton): Do this for CSS transforms and opacity too, at least if they're
+// TODO(#1244, #2007, pcwalton): Do this for CSS transforms and opacity too, at least if they're
 // animating.
 fn propagate_layer_flag_from_child(layers_needed_for_descendants: &mut bool, kid: &mut Flow) {
     if kid.is_absolute_containing_block() {
@@ -498,21 +498,17 @@ pub struct BlockFlow {
 }
 
 impl BlockFlow {
-    pub fn from_node(constructor: &mut FlowConstructor,
+    pub fn from_node(constructor: &mut FlowConstructor, node: &ThreadSafeLayoutNode) -> BlockFlow {
-                     node: &ThreadSafeLayoutNode)
-                     -> BlockFlow {
         BlockFlow {
             base: BaseFlow::new((*node).clone()),
-            box_: Some(Box::new(constructor, node, MainBoxKind)),
+            box_: Some(Box::new(constructor, node)),
             is_root: false,
             static_y_offset: Au::new(0),
             float: None
         }
     }
 
-    pub fn from_node_and_box(node: &ThreadSafeLayoutNode,
+    pub fn from_node_and_box(node: &ThreadSafeLayoutNode, box_: Box) -> BlockFlow {
-                             box_: Box)
-                             -> BlockFlow {
         BlockFlow {
             base: BaseFlow::new((*node).clone()),
             box_: Some(box_),
@@ -528,7 +524,7 @@ impl BlockFlow {
                            -> BlockFlow {
         BlockFlow {
             base: BaseFlow::new((*node).clone()),
-            box_: Some(Box::new(constructor, node, MainBoxKind)),
+            box_: Some(Box::new(constructor, node)),
             is_root: false,
             static_y_offset: Au::new(0),
             float: Some(~FloatedBlockInfo::new(float_kind))
@@ -828,7 +824,7 @@ impl BlockFlow {
 
         // Absolute positioning establishes a block formatting context. Don't propagate floats
         // in or out. (But do propagate them between kids.)
-        if inorder && (self.is_absolutely_positioned()) {
+        if inorder && self.is_absolutely_positioned() {
             self.base.floats = Floats::new();
         }
         if margins_may_collapse != MarginsMayCollapse {
@@ -879,7 +875,7 @@ impl BlockFlow {
                     if kid_base.clear != clear::none {
                         // We have clearance, so assume there are no floats in and perform layout.
                         //
-                        // FIXME(pcwalton): This could be wrong if we have `clear: left` or
+                        // FIXME(#2008, pcwalton): This could be wrong if we have `clear: left` or
                         // `clear: right` and there are still floats to impact, of course. But this
                         // gets complicated with margin collapse. Possibly the right thing to do is
                         // to lay out the block again in this rare case. (Note that WebKit can lay
@@ -899,8 +895,8 @@ impl BlockFlow {
 
                 floats_out = Some(flow::mut_base(kid).floats.clone());
 
-                // FIXME(pcwalton): A horrible hack that has to do with the fact that `origin.y`
+                // A horrible hack that has to do with the fact that `origin.y` is used for
-                // is used for something else later (containing block for float).
+                // something else later (containing block for float).
                 if kid.is_float() {
                     flow::mut_base(kid).position.origin.y = cur_y;
                 }
@@ -972,10 +968,10 @@ impl BlockFlow {
             translate_including_floats(&mut cur_y, delta, inorder, &mut floats);
         }
 
-        // FIXME(pcwalton): The max is taken here so that you can scroll the page, but this is not
+        // FIXME(#2003, pcwalton): The max is taken here so that you can scroll the page, but this
-        // correct behavior according to CSS 2.1 § 10.5. Instead I think we should treat the root
+        // is not correct behavior according to CSS 2.1 § 10.5. Instead I think we should treat the
-        // element as having `overflow: scroll` and use the layers-based scrolling infrastructure
+        // root element as having `overflow: scroll` and use the layers-based scrolling
-        // to make it scrollable.
+        // infrastructure to make it scrollable.
         let mut height = cur_y - top_offset;
         if self.is_root() {
             height = Au::max(layout_context.screen_size.height, height)
@@ -1002,7 +998,6 @@ impl BlockFlow {
         }
 
         for fragment in self.box_.iter() {
-            // FIXME(pcwalton): Is this right?
             let containing_block_height = height;
 
             let mut candidate_height_iterator =
@@ -1214,14 +1209,6 @@ impl BlockFlow {
         }
 
         // Process absolute descendant links.
-        //
-        // TODO: Maybe we should handle position 'absolute' and 'fixed'
-        // descendants before normal descendants just in case there is a
-        // problem when display-list building is parallel and both the
-        // original parent and this flow access the same absolute flow.
-        // Note that this can only be done once we have paint order
-        // working cos currently the later boxes paint over the absolute
-        // and fixed boxes :|
         let mut absolute_info = info;
         absolute_info.layers_needed_for_positioned_flows =
             self.base.flags_info.flags.layers_needed_for_descendants();
@@ -1245,8 +1232,8 @@ impl BlockFlow {
                                     builder: &mut DisplayListBuilder,
                                     info: &DisplayListBuildingInfo) {
         if self.is_float() {
-            // TODO(pcwalton): This is a pseudo-stacking context. We need to merge `z-index: auto`
+            // TODO(#2009, pcwalton): This is a pseudo-stacking context. We need to merge `z-index:
-            // kids into the parent stacking context, when that is supported.
+            // auto` kids into the parent stacking context, when that is supported.
             self.build_display_list_float(stacking_context, builder, info)
         } else if self.is_absolutely_positioned() {
             self.build_display_list_abs(stacking_context, builder, info)
@@ -1395,14 +1382,12 @@ impl BlockFlow {
         if !info.layers_needed_for_positioned_flows && !self.base.flags_info.flags.needs_layer() {
             // We didn't need a layer.
             //
-            // TODO(pcwalton): `z-index`.
+            // TODO(#781, pcwalton): `z-index`.
             parent_stacking_context.positioned_descendants.push((0, stacking_context.flatten()));
             return
         }
 
         // If we got here, then we need a new layer.
-        // 
-        // FIXME(pcwalton): The color is wrong!
         let size = Size2D(self.base.position.size.width.to_nearest_px() as uint,
                           self.base.position.size.height.to_nearest_px() as uint);
         let origin = Point2D(info.absolute_containing_block_position.x.to_nearest_px() as uint,
@@ -1415,8 +1400,8 @@ impl BlockFlow {
         let new_layer = RenderLayer {
             id: self.layer_id(0),
             display_list: Arc::new(stacking_context.flatten()),
-            rect: Rect(origin, size),
+            position: Rect(origin, size),
-            color: color::rgba(255.0, 255.0, 255.0, 0.0),
+            background_color: color::rgba(255.0, 255.0, 255.0, 0.0),
             scroll_policy: scroll_policy,
         };
         builder.layers.push(new_layer)
@@ -1433,6 +1418,8 @@ impl BlockFlow {
         self.base.position.origin.y
     }
 
+    /// Initializes the containing width if this block flow is a float. This is done at the start
+    /// of `assign_widths`.
     fn set_containing_width_if_float(&mut self, containing_block_width: Au) {
         if self.is_float() {
             self.float.get_mut_ref().containing_width = containing_block_width;
@@ -1442,6 +1429,7 @@ impl BlockFlow {
         }
     }
 
+    /// Assigns the computed left content edge and width to all the children of this block flow.
     pub fn propagate_assigned_width_to_children(&mut self,
                                                 left_content_edge: Au,
                                                 content_width: Au,
@@ -1474,7 +1462,6 @@ impl BlockFlow {
         // This value is used only for table cells.
         let mut kid_left_margin_edge = left_content_edge;
 
-        // FIXME(ksh8281): avoid copy
         let flags_info = self.base.flags_info.clone();
         for (i, kid) in self.base.child_iter().enumerate() {
             if kid.is_block_flow() {
@@ -1747,9 +1734,9 @@ impl Flow for BlockFlow {
     }
 
     fn layer_id(&self, fragment_index: uint) -> LayerId {
-        // FIXME(pcwalton): This is a hack and is totally bogus in the presence of pseudo-elements.
+        // FIXME(#2010, pcwalton): This is a hack and is totally bogus in the presence of pseudo-
-        // But until we have incremental reflow we can't do better--we recreate the flow for every
+        // elements. But until we have incremental reflow we can't do better--we recreate the flow
-        // DOM node so otherwise we nuke layers on every reflow.
+        // for every DOM node so otherwise we nuke layers on every reflow.
         match self.box_ {
             Some(ref box_) => {
                 LayerId(box_.node.id(), fragment_index)

src/components/main/layout/box_.rs

@@ -292,8 +292,8 @@ pub enum SplitBoxResult {
 
 /// Data for inline boxes.
 ///
-/// FIXME(pcwalton): Copying `InlineParentInfo` vectors all the time is really inefficient. Use
+/// FIXME(#2013, pcwalton): Copying `InlineParentInfo` vectors all the time is really inefficient.
-/// atomic reference counting instead.
+/// Use atomic reference counting instead.
 #[deriving(Clone)]
 pub struct InlineInfo {
     parent_info: SmallVec0<InlineParentInfo>,
@@ -418,14 +418,6 @@ def_noncontent!(bottom, noncontent_bottom, noncontent_inline_bottom)
 def_noncontent_horiz!(left,  merge_noncontent_inline_left,  clear_noncontent_inline_left)
 def_noncontent_horiz!(right, merge_noncontent_inline_right, clear_noncontent_inline_right)
 
-/// Some DOM nodes can contribute more than one type of box. We call these boxes "sub-boxes". For
-/// these nodes, this enum is used to determine which sub-box to construct for that node.
-pub enum SubBoxKind {
-    /// The main box for this node. All DOM nodes that are rendered at all have at least a main
-    /// box.
-    MainBoxKind,
-}
-
 impl Box {
     /// Constructs a new `Box` instance for the given node.
     ///
@@ -434,13 +426,7 @@ impl Box {
     ///   * `constructor`: The flow constructor.
     ///
     ///   * `node`: The node to create a box for.
-    ///
+    pub fn new(constructor: &mut FlowConstructor, node: &ThreadSafeLayoutNode) -> Box {
-    ///   * `sub_box_kind`: The kind of box to create for the node, in case this node can
-    ///     contribute more than one type of box. See the definition of `SubBoxKind`.
-    pub fn new(constructor: &mut FlowConstructor,
-               node: &ThreadSafeLayoutNode,
-               sub_box_kind: SubBoxKind)
-               -> Box {
         Box {
             node: OpaqueNodeMethods::from_thread_safe_layout_node(node),
             style: node.style().clone(),
@@ -448,7 +434,7 @@ impl Box {
             border: RefCell::new(Zero::zero()),
             padding: RefCell::new(Zero::zero()),
             margin: RefCell::new(Zero::zero()),
-            specific: constructor.build_specific_box_info_for_node(node, sub_box_kind),
+            specific: constructor.build_specific_box_info_for_node(node),
             position_offsets: RefCell::new(Zero::zero()),
             inline_info: RefCell::new(None),
             new_line_pos: ~[],

src/components/main/layout/construct.rs

@@ -23,7 +23,7 @@
 use css::node_style::StyledNode;
 use layout::block::BlockFlow;
 use layout::box_::{Box, GenericBox, IframeBox, IframeBoxInfo, ImageBox, ImageBoxInfo};
-use layout::box_::{InlineInfo, InlineParentInfo, MainBoxKind, SpecificBoxInfo, SubBoxKind};
+use layout::box_::{InlineInfo, InlineParentInfo, SpecificBoxInfo};
 use layout::box_::{TableBox, TableCellBox, TableColumnBox, TableColumnBoxInfo, TableRowBox};
 use layout::box_::{TableWrapperBox, UnscannedTextBox, UnscannedTextBoxInfo};
 use layout::context::LayoutContext;
@@ -50,7 +50,7 @@ use gfx::font_context::FontContext;
 use script::dom::bindings::codegen::InheritTypes::TextCast;
 use script::dom::bindings::js::JS;
 use script::dom::element::{HTMLIFrameElementTypeId, HTMLImageElementTypeId};
-use script::dom::element::{HTMLObjectElementTypeId, HTMLPseudoElementTypeId};
+use script::dom::element::{HTMLObjectElementTypeId};
 use script::dom::element::{HTMLTableColElementTypeId, HTMLTableDataCellElementTypeId};
 use script::dom::element::{HTMLTableElementTypeId, HTMLTableHeaderCellElementTypeId};
 use script::dom::element::{HTMLTableRowElementTypeId, HTMLTableSectionElementTypeId};
@@ -281,7 +281,8 @@ impl<'a> FlowConstructor<'a> {
     }
 
     /// Builds the `ImageBoxInfo` for the given image. This is out of line to guide inlining.
-    fn build_box_info_for_image(&mut self, node: &ThreadSafeLayoutNode, url: Option<Url>) -> SpecificBoxInfo {
+    fn build_box_info_for_image(&mut self, node: &ThreadSafeLayoutNode, url: Option<Url>)
+                                -> SpecificBoxInfo {
         match url {
             None => GenericBox,
             Some(url) => {
@@ -293,29 +294,28 @@ impl<'a> FlowConstructor<'a> {
     }
 
     /// Builds specific `Box` info for the given node.
-    pub fn build_specific_box_info_for_node(&mut self,
+    pub fn build_specific_box_info_for_node(&mut self, node: &ThreadSafeLayoutNode)
-                                            node: &ThreadSafeLayoutNode,
-                                            sub_box_kind: SubBoxKind)
                                             -> SpecificBoxInfo {
         match node.type_id() {
-            ElementNodeTypeId(HTMLImageElementTypeId) => {
+            Some(ElementNodeTypeId(HTMLImageElementTypeId)) => {
                 self.build_box_info_for_image(node, node.image_url())
             }
-            ElementNodeTypeId(HTMLIFrameElementTypeId) => IframeBox(IframeBoxInfo::new(node)),
+            Some(ElementNodeTypeId(HTMLIFrameElementTypeId)) => {
-            ElementNodeTypeId(HTMLObjectElementTypeId) => {
+                IframeBox(IframeBoxInfo::new(node))
+            }
+            Some(ElementNodeTypeId(HTMLObjectElementTypeId)) => {
                 let data = node.get_object_data(&self.layout_context.url);
                 self.build_box_info_for_image(node, data)
             }
-            ElementNodeTypeId(HTMLTableElementTypeId) => TableWrapperBox,
+            Some(ElementNodeTypeId(HTMLTableElementTypeId)) => TableWrapperBox,
-            ElementNodeTypeId(HTMLTableColElementTypeId) => {
+            Some(ElementNodeTypeId(HTMLTableColElementTypeId)) => {
                 TableColumnBox(TableColumnBoxInfo::new(node))
             }
-            ElementNodeTypeId(HTMLTableDataCellElementTypeId) |
+            Some(ElementNodeTypeId(HTMLTableDataCellElementTypeId)) |
-            ElementNodeTypeId(HTMLTableHeaderCellElementTypeId) => TableCellBox,
+            Some(ElementNodeTypeId(HTMLTableHeaderCellElementTypeId)) => TableCellBox,
-            ElementNodeTypeId(HTMLTableRowElementTypeId) |
+            Some(ElementNodeTypeId(HTMLTableRowElementTypeId)) |
-            ElementNodeTypeId(HTMLTableSectionElementTypeId) => TableRowBox,
+            Some(ElementNodeTypeId(HTMLTableSectionElementTypeId)) => TableRowBox,
-            ElementNodeTypeId(HTMLPseudoElementTypeId) |
+            None | Some(TextNodeTypeId) => UnscannedTextBox(UnscannedTextBoxInfo::new(node)),
-            TextNodeTypeId => UnscannedTextBox(UnscannedTextBoxInfo::new(node)),
             _ => GenericBox,
         }
     }
@@ -676,11 +676,11 @@ impl<'a> FlowConstructor<'a> {
         }
     }
 
-    // FIXME(pcwalton): Why does this function create a box only to throw it away???
+    // FIXME(#1999, pcwalton): Why does this function create a box only to throw it away???
     fn set_inline_info_for_inline_child(&mut self,
                                         boxes: &[&Box],
                                         parent_node: &ThreadSafeLayoutNode) {
-        let parent_box = Box::new(self, parent_node, MainBoxKind);
+        let parent_box = Box::new(self, parent_node);
         let font_style = parent_box.font_style();
         let font_group = self.font_context().get_resolved_font_for_style(&font_style);
         let (font_ascent,font_descent) = font_group.borrow().with_mut( |fg| {
@@ -692,7 +692,8 @@ impl<'a> FlowConstructor<'a> {
         let boxes_len = boxes.len();
         parent_box.compute_borders(parent_box.style());
 
-        // FIXME(pcwalton): I suspect that `Au(0)` is not correct for the containing block width.
+        // FIXME(#2000, pcwalton): I suspect that `Au(0)` is not correct for the containing block
+        // width.
         parent_box.compute_padding(parent_box.style(), Au(0));
 
         for (i, box_) in boxes.iter().enumerate() {
@@ -739,7 +740,7 @@ impl<'a> FlowConstructor<'a> {
 
         // If this node is ignorable whitespace, bail out now.
         //
-        // FIXME(pcwalton): Don't do this if there's padding or borders.
+        // FIXME(#2001, pcwalton): Don't do this if there's padding or borders.
         if node.is_ignorable_whitespace() {
             let opaque_node = OpaqueNodeMethods::from_thread_safe_layout_node(node);
             return ConstructionItemConstructionResult(WhitespaceConstructionItem(
@@ -748,7 +749,7 @@ impl<'a> FlowConstructor<'a> {
         }
 
         let mut opt_box_accumulator = None;
-        opt_box_accumulator.push(Box::new(self, node, MainBoxKind));
+        opt_box_accumulator.push(Box::new(self, node));
 
         let construction_item = InlineBoxesConstructionItem(InlineBoxesConstructionResult {
             splits: None,
@@ -951,20 +952,23 @@ impl<'a> PostorderNodeMutTraversal for FlowConstructor<'a> {
     fn process(&mut self, node: &ThreadSafeLayoutNode) -> bool {
         // Get the `display` property for this node, and determine whether this node is floated.
         let (display, float, positioning) = match node.type_id() {
-            ElementNodeTypeId(HTMLPseudoElementTypeId) => {
+            None => {
+                // Pseudo-element.
                 let style = node.style().get();
                 (display::inline, style.Box.get().float, style.Box.get().position)
             }
-            ElementNodeTypeId(_) => {
+            Some(ElementNodeTypeId(_)) => {
                 let style = node.style().get();
                 (style.Box.get().display, style.Box.get().float, style.Box.get().position)
             }
-            TextNodeTypeId => (display::inline, float::none, position::static_),
+            Some(TextNodeTypeId) => (display::inline, float::none, position::static_),
-            CommentNodeTypeId |
+            Some(CommentNodeTypeId) |
-            DoctypeNodeTypeId |
+            Some(DoctypeNodeTypeId) |
-            DocumentFragmentNodeTypeId |
+            Some(DocumentFragmentNodeTypeId) |
-            DocumentNodeTypeId |
+            Some(DocumentNodeTypeId) |
-            ProcessingInstructionNodeTypeId => (display::none, float::none, position::static_),
+            Some(ProcessingInstructionNodeTypeId) => {
+                (display::none, float::none, position::static_)
+            }
         };
 
         debug!("building flow for node: {:?} {:?}", display, float);
@@ -1077,22 +1081,22 @@ trait NodeUtils {
 impl<'ln> NodeUtils for ThreadSafeLayoutNode<'ln> {
     fn is_replaced_content(&self) -> bool {
         match self.type_id() {
-            TextNodeTypeId |
+            Some(TextNodeTypeId) |
-            ProcessingInstructionNodeTypeId |
+            Some(ProcessingInstructionNodeTypeId) |
-            CommentNodeTypeId |
+            Some(CommentNodeTypeId) |
-            DoctypeNodeTypeId |
+            Some(DoctypeNodeTypeId) |
-            DocumentFragmentNodeTypeId |
+            Some(DocumentFragmentNodeTypeId) |
-            DocumentNodeTypeId |
+            Some(DocumentNodeTypeId) |
-            ElementNodeTypeId(HTMLPseudoElementTypeId) |
+            None |
-            ElementNodeTypeId(HTMLImageElementTypeId) => true,
+            Some(ElementNodeTypeId(HTMLImageElementTypeId)) => true,
-            ElementNodeTypeId(HTMLObjectElementTypeId) => self.has_object_data(),
+            Some(ElementNodeTypeId(HTMLObjectElementTypeId)) => self.has_object_data(),
-            ElementNodeTypeId(_) => false,
+            Some(ElementNodeTypeId(_)) => false,
         }
     }
 
     fn is_ignorable_whitespace(&self) -> bool {
         match self.type_id() {
-            TextNodeTypeId => {
+            Some(TextNodeTypeId) => {
                 unsafe {
                     let text: JS<Text> = TextCast::to(self.get_jsmanaged()).unwrap();
                     if !is_whitespace(text.get().characterdata.data) {
@@ -1141,12 +1145,17 @@ impl<'ln> NodeUtils for ThreadSafeLayoutNode<'ln> {
             Some(ref mut layout_data) => {
                 match self.get_element_type() {
                     Before | BeforeBlock => {
-                        return mem::replace(&mut layout_data.data.before_flow_construction_result, NoConstructionResult)
+                        mem::replace(&mut layout_data.data.before_flow_construction_result,
-                    },
+                                     NoConstructionResult)
+                    }
                     After | AfterBlock => {
-                        return mem::replace(&mut layout_data.data.after_flow_construction_result, NoConstructionResult)
+                        mem::replace(&mut layout_data.data.after_flow_construction_result,
-                    },
+                                     NoConstructionResult)
-                    Normal => { return mem::replace(&mut layout_data.data.flow_construction_result, NoConstructionResult) },
+                    }
+                    Normal => {
+                        mem::replace(&mut layout_data.data.flow_construction_result,
+                                     NoConstructionResult)
+                    }
                 }
             }
             None => fail!("no layout data"),

src/components/main/layout/flow.rs

@@ -1101,7 +1101,7 @@ impl<'a> MutableFlowUtils for &'a mut Flow {
                 overflow = overflow.union(&kid_overflow)
             }
 
-            // FIXME(pcwalton): This is wrong for `position: fixed`.
+            // FIXME(#2004, pcwalton): This is wrong for `position: fixed`.
             for descendant_link in mut_base(self).abs_descendants.iter() {
                 match descendant_link.resolve() {
                     Some(flow) => {
@@ -1165,7 +1165,7 @@ impl<'a> MutableFlowUtils for &'a mut Flow {
                 self.as_table_cell().build_display_list_table_cell(stacking_context, builder, info)
             }
             TableColGroupFlowClass => {
-                // Nothing to do here, I guess? --pcwalton
+                // Nothing to do here, as column groups don't render.
             }
         }
     }
@@ -1233,4 +1233,3 @@ impl MutableOwnedFlowUtils for ~Flow {
         self_borrowed.destroy();
     }
 }
-

src/components/main/layout/layout_task.rs

@@ -649,8 +649,8 @@ impl LayoutTask {
                 // it with extreme prejudice.
                 let mut color = color::rgba(255.0, 255.0, 255.0, 255.0);
                 for child in node.traverse_preorder() {
-                    if child.type_id() == ElementNodeTypeId(HTMLHtmlElementTypeId) ||
+                    if child.type_id() == Some(ElementNodeTypeId(HTMLHtmlElementTypeId)) ||
-                            child.type_id() == ElementNodeTypeId(HTMLBodyElementTypeId) {
+                            child.type_id() == Some(ElementNodeTypeId(HTMLBodyElementTypeId)) {
                         let element_bg_color = {
                             let thread_safe_child = ThreadSafeLayoutNode::new(&child);
                             thread_safe_child.style()
@@ -681,8 +681,8 @@ impl LayoutTask {
                 let render_layer = RenderLayer {
                     id: layout_root.layer_id(0),
                     display_list: display_list.clone(),
-                    rect: Rect(Point2D(0u, 0u), root_size),
+                    position: Rect(Point2D(0u, 0u), root_size),
-                    color: color,
+                    background_color: color,
                     scroll_policy: Scrollable,
                 };
 

src/components/main/layout/model.rs

@@ -83,7 +83,7 @@ pub struct MarginCollapseInfo {
 }
 
 impl MarginCollapseInfo {
-    /// TODO(pcwalton): Remove this method once `box_` is not an `Option`.
+    /// TODO(#2012, pcwalton): Remove this method once `box_` is not an `Option`.
     pub fn new() -> MarginCollapseInfo {
         MarginCollapseInfo {
             state: AccumulatingCollapsibleTopMargin,

src/components/main/layout/table.rs

@@ -127,7 +127,7 @@ impl TableFlow {
 
     /// Assign height for table flow.
     ///
-    /// TODO(pcwalton): This probably doesn't handle margin collapse right.
+    /// TODO(#2014, pcwalton): This probably doesn't handle margin collapse right.
     ///
     /// inline(always) because this is only ever called by in-order or non-in-order top-level
     /// methods

src/components/main/layout/table_cell.rs

@@ -41,7 +41,7 @@ impl TableCellFlow {
 
     /// Assign height for table-cell flow.
     ///
-    /// TODO(pcwalton): This doesn't handle floats right.
+    /// TODO(#2015, pcwalton): This doesn't handle floats right.
     ///
     /// inline(always) because this is only ever called by in-order or non-in-order top-level
     /// methods

src/components/main/layout/table_wrapper.rs

@@ -116,11 +116,6 @@ impl TableWrapperFlow {
                                             info: &DisplayListBuildingInfo) {
         debug!("build_display_list_table_wrapper: same process as block flow");
         self.block_flow.build_display_list_block(stacking_context, builder, info);
-
-        for kid in self.block_flow.base.child_iter() {
-            kid.as_table().block_flow.base.position.size.height =
-                self.block_flow.base.position.size.height;
-        }
     }
 }
 
@@ -174,8 +169,6 @@ impl Flow for TableWrapperFlow {
         let mut left_content_edge = Au::new(0);
         let mut content_width = containing_block_width;
 
-        // self.block_flow.set_containing_width_if_float(containing_block_width);
-
         let width_computer = TableWrapper;
         width_computer.compute_used_width_table_wrapper(self, ctx, containing_block_width);
 
@@ -286,7 +279,6 @@ impl TableWrapper {
             },
             AutoLayout => {
                 // Automatic table layout is calculated according to CSS 2.1 § 17.5.2.2.
-                // But, this spec is not specified. Since the new spec is specified, it may be modified. See #1687.
                 let mut cap_min = Au(0);
                 let mut cols_min = Au(0);
                 let mut cols_max = Au(0);
@@ -303,8 +295,8 @@ impl TableWrapper {
                         col_pref_widths = kid.col_pref_widths();
                     }
                 }
-                // 'extra_width': difference between the calculated table width and minimum width required by all columns.
+                // 'extra_width': difference between the calculated table width and minimum width
-                // It will be distributed over the columns
+                // required by all columns. It will be distributed over the columns.
                 let (width, extra_width) = match input.computed_width {
                     Auto => {
                         if input.available_width > geometry::max(cols_max, cap_min) {
@@ -337,8 +329,9 @@ impl TableWrapper {
                 // The extra width is distributed over the columns
                 if extra_width > Au(0) {
                     let cell_len = table_wrapper.col_widths.len() as f64;
-                    table_wrapper.col_widths = col_min_widths.map(|width|
+                    table_wrapper.col_widths = col_min_widths.map(|width| {
-                                                                  width + extra_width.scale_by(1.0/cell_len));
+                        width + extra_width.scale_by(1.0 / cell_len)
+                    });
                 }
                 width
             }

src/components/main/layout/util.rs

@@ -235,7 +235,7 @@ impl OpaqueNodeMethods for OpaqueNode {
     fn from_thread_safe_layout_node(node: &ThreadSafeLayoutNode) -> OpaqueNode {
         unsafe {
             let abstract_node = node.get_jsmanaged();
-            let ptr: uintptr_t = cast::transmute(abstract_node.reflector().get_jsobject());
+            let ptr: uintptr_t = abstract_node.reflector().get_jsobject() as uint;
             OpaqueNode(ptr)
         }
     }

src/components/main/layout/wrapper.rs

@@ -37,7 +37,7 @@ use extra::url::Url;
 use script::dom::bindings::codegen::InheritTypes::{HTMLIFrameElementDerived};
 use script::dom::bindings::codegen::InheritTypes::{HTMLImageElementDerived, TextDerived};
 use script::dom::bindings::js::JS;
-use script::dom::element::{Element, HTMLAreaElementTypeId, HTMLAnchorElementTypeId, HTMLPseudoElementTypeId};
+use script::dom::element::{Element, HTMLAreaElementTypeId, HTMLAnchorElementTypeId};
 use script::dom::element::{HTMLLinkElementTypeId};
 use script::dom::htmliframeelement::HTMLIFrameElement;
 use script::dom::htmlimageelement::HTMLImageElement;
@@ -58,8 +58,9 @@ pub trait TLayoutNode {
     /// Creates a new layout node with the same lifetime as this layout node.
     unsafe fn new_with_this_lifetime(&self, node: &JS<Node>) -> Self;
 
-    /// Returns the type ID of this node. Fails if this node is borrowed mutably.
+    /// Returns the type ID of this node. Fails if this node is borrowed mutably. Returns `None`
-    fn type_id(&self) -> NodeTypeId;
+    /// if this is a pseudo-element; otherwise, returns `Some`.
+    fn type_id(&self) -> Option<NodeTypeId>;
 
     /// Returns the interior of this node as a `JS`. This is highly unsafe for layout to
     /// call and as such is marked `unsafe`.
@@ -73,14 +74,14 @@ pub trait TLayoutNode {
 
     fn node_is_element(&self) -> bool {
         match self.type_id() {
-            ElementNodeTypeId(..) => true,
+            Some(ElementNodeTypeId(..)) => true,
             _ => false
         }
     }
 
     fn node_is_document(&self) -> bool {
         match self.type_id() {
-            DocumentNodeTypeId(..) => true,
+            Some(DocumentNodeTypeId(..)) => true,
             _ => false
         }
     }
@@ -160,17 +161,21 @@ impl<'ln> TLayoutNode for LayoutNode<'ln> {
             chain: self.chain,
         }
     }
-    fn type_id(&self) -> NodeTypeId {
+
-        self.node.type_id()
+    fn type_id(&self) -> Option<NodeTypeId> {
+        Some(self.node.type_id())
     }
+
     unsafe fn get_jsmanaged<'a>(&'a self) -> &'a JS<Node> {
         &self.node
     }
+
     fn first_child(&self) -> Option<LayoutNode<'ln>> {
         unsafe {
             self.get().first_child_ref().map(|node| self.new_with_this_lifetime(node))
         }
     }
+
     fn text(&self) -> ~str {
         unsafe {
             if !self.get().is_text() {
@@ -382,7 +387,7 @@ fn get_content(content_list: &content::T) -> ~str{
         content::Content(ref value) => {
             let iter = &mut value.clone().move_iter().peekable();
             match iter.next() {
-                Some(content::StringContent(str)) => str,
+                Some(content::StringContent(content)) => content,
                 _ => ~"",
             }
         }
@@ -420,21 +425,25 @@ impl<'ln> TLayoutNode for ThreadSafeLayoutNode<'ln> {
         }
     }
 
-    fn type_id(&self) -> NodeTypeId {
+    /// Returns `None` if this is a pseudo-element.
+    fn type_id(&self) -> Option<NodeTypeId> {
         if self.pseudo == Before || self.pseudo == After {
-            return ElementNodeTypeId(HTMLPseudoElementTypeId)
+            return None
-        } else {
-            return self.node.type_id()
         }
+
+        self.node.type_id()
     }
+
     unsafe fn get_jsmanaged<'a>(&'a self) -> &'a JS<Node> {
         self.node.get_jsmanaged()
     }
+
     unsafe fn get<'a>(&'a self) -> &'a Node { // this change.
         cast::transmute::<*mut Node,&'a Node>(self.get_jsmanaged().unsafe_get())
     }
+
     fn first_child(&self) -> Option<ThreadSafeLayoutNode<'ln>> {
-        if self.pseudo == Before {
+        if self.pseudo == Before || self.pseudo == After {
             return None
         }
 
@@ -452,19 +461,21 @@ impl<'ln> TLayoutNode for ThreadSafeLayoutNode<'ln> {
             self.get().first_child_ref().map(|node| self.new_with_this_lifetime(node))
         }
     }
+
     fn text(&self) -> ~str {
         if self.pseudo == Before || self.pseudo == After {
             let layout_data_ref = self.borrow_layout_data();
-            let node_ldw = layout_data_ref.get().get_ref();
+            let node_layout_data_wrapper = layout_data_ref.get().get_ref();
 
             if self.pseudo == Before {
-                let before_style = node_ldw.data.before_style.get_ref();
+                let before_style = node_layout_data_wrapper.data.before_style.get_ref();
                 return get_content(&before_style.get().Box.get().content)
             } else {
-                let after_style = node_ldw.data.after_style.get_ref();
+                let after_style = node_layout_data_wrapper.data.after_style.get_ref();
                 return get_content(&after_style.get().Box.get().content)
             }
-        } else {
+        }
+
         unsafe {
             if !self.get().is_text() {
                 fail!("not text!")
@@ -474,7 +485,6 @@ impl<'ln> TLayoutNode for ThreadSafeLayoutNode<'ln> {
         }
     }
 }
-}
 
 impl<'ln> Clone for ThreadSafeLayoutNode<'ln> {
     fn clone(&self) -> ThreadSafeLayoutNode<'ln> {
@@ -547,40 +557,36 @@ impl<'ln> ThreadSafeLayoutNode<'ln> {
 
     pub fn is_block(&self, kind: ElementType) -> bool {
         let mut layout_data_ref = self.mutate_layout_data();
-        let node_ldw = layout_data_ref.get().get_mut_ref();
+        let node_layout_data_wrapper = layout_data_ref.get().get_mut_ref();
 
         let display = match kind {
             Before | BeforeBlock => {
-                let before_style = node_ldw.data.before_style.get_ref();
+                let before_style = node_layout_data_wrapper.data.before_style.get_ref();
                 before_style.get().Box.get().display
             }
             After | AfterBlock => {
-                let after_style = node_ldw.data.after_style.get_ref();
+                let after_style = node_layout_data_wrapper.data.after_style.get_ref();
                 after_style.get().Box.get().display
             }
             Normal => {
-                let after_style = node_ldw.data.style.get_ref();
+                let after_style = node_layout_data_wrapper.data.style.get_ref();
                 after_style.get().Box.get().display
             }
         };
 
-        if display == display::block {
+        display == display::block
-            return true
-        }
-        
-        false
     }
 
     pub fn has_before_pseudo(&self) -> bool {
-        let ldw = self.borrow_layout_data();
+        let layout_data_wrapper = self.borrow_layout_data();
-        let ldw_ref = ldw.get().get_ref();
+        let layout_data_wrapper_ref = layout_data_wrapper.get().get_ref();
-        ldw_ref.data.before_style.is_some()
+        layout_data_wrapper_ref.data.before_style.is_some()
     }
 
     pub fn has_after_pseudo(&self) -> bool {
-        let ldw = self.borrow_layout_data();
+        let layout_data_wrapper = self.borrow_layout_data();
-        let ldw_ref = ldw.get().get_ref();
+        let layout_data_wrapper_ref = layout_data_wrapper.get().get_ref();
-        ldw_ref.data.after_style.is_some()
+        layout_data_wrapper_ref.data.after_style.is_some()
     }
 
     /// Borrows the layout data immutably. Fails on a conflicting borrow.

src/components/msg/compositor_msg.rs

@@ -89,7 +89,7 @@ impl Show for LayerId {
 }
 
 impl LayerId {
-    /// FIXME(pcwalton): This is unfortunate. Maybe remove this in the future.
+    /// FIXME(#2011, pcwalton): This is unfortunate. Maybe remove this in the future.
     pub fn null() -> LayerId {
         LayerId(0, 0)
     }
@@ -110,9 +110,9 @@ pub struct LayerMetadata {
     /// An opaque ID. This is usually the address of the flow and index of the box within it.
     id: LayerId,
     /// The position and size of the layer in pixels.
-    rect: Rect<uint>,
+    position: Rect<uint>,
     /// The background color of the layer.
-    color: Color,
+    background_color: Color,
     /// The scrolling policy of this layer.
     scroll_policy: ScrollPolicy,
 }

src/components/script/dom/element.rs

@@ -112,7 +112,6 @@ pub enum ElementTypeId {
     HTMLParamElementTypeId,
     HTMLPreElementTypeId,
     HTMLProgressElementTypeId,
-    HTMLPseudoElementTypeId,
     HTMLQuoteElementTypeId,
     HTMLScriptElementTypeId,
     HTMLSelectElementTypeId,

src/components/script/script_task.rs

@@ -934,9 +934,10 @@ impl ScriptTask {
             page.query_layout(ContentBoxQuery(node.to_trusted_node_address(), chan), port);
         let point = Point2D(to_frac_px(rect.origin.x).to_f32().unwrap(),
                             to_frac_px(rect.origin.y).to_f32().unwrap());
-        // FIXME(pcwalton): This is pretty bogus when multiple layers are involved. Really
+        // FIXME(#2003, pcwalton): This is pretty bogus when multiple layers are involved.
-        // what needs to happen is that this needs to go through layout to ask which layer
+        // Really what needs to happen is that this needs to go through layout to ask which
-        // the element belongs to, and have it send the scroll message to the compositor.
+        // layer the element belongs to, and have it send the scroll message to the
+        // compositor.
         self.compositor.scroll_fragment_point(pipeline_id, LayerId::null(), point);
     }
 

src/components/style/common_types.rs

@@ -8,6 +8,7 @@ pub use servo_util::geometry::Au;
 
 pub type CSSFloat = f64;
 
+pub static DEFAULT_LINE_HEIGHT: CSSFloat = 1.14;
 
 pub mod specified {
     use std::ascii::StrAsciiExt;

src/components/style/properties.rs.mako

@@ -89,9 +89,7 @@ pub mod longhands {
         THIS_STYLE_STRUCT.longhands.append(property)
         LONGHANDS.append(property)
         LONGHANDS_BY_NAME[name] = property
-        if derived_from not in DERIVED_LONGHANDS:
+        DERIVED_LONGHANDS.setdefault(derived_from, []).append(property)
-            DERIVED_LONGHANDS[derived_from] = []
-        DERIVED_LONGHANDS[derived_from].append(property)
     %>
         pub mod ${property.ident} {
             % if not no_super:
@@ -114,9 +112,13 @@ pub mod longhands {
     <%def name="longhand(name, no_super=False, derived_from=None)">
         <%self:raw_longhand name="${name}" derived_from="${derived_from}">
             ${caller.body()}
-            pub fn parse_specified(input: &[ComponentValue], base_url: &Url)
+            pub fn parse_specified(_input: &[ComponentValue], _base_url: &Url)
                                -> Option<DeclaredValue<SpecifiedValue>> {
-                parse(input, base_url).map(super::SpecifiedValue)
+                % if derived_from is None:
+                    parse(_input, _base_url).map(super::SpecifiedValue)
+                % else:
+                    None
+                % endif
             }
         </%self:raw_longhand>
     </%def>
@@ -380,6 +382,7 @@ pub mod longhands {
 
     <%self:longhand name="-servo-minimum-line-height" derived_from="line-height">
         use super::Au;
+        use super::super::common_types::DEFAULT_LINE_HEIGHT;
         use super::super::longhands::display;
         use super::super::longhands::line_height;
 
@@ -396,17 +399,12 @@ pub mod longhands {
             Au(0)
         }
 
-        #[inline]
-        pub fn parse(_: &[ComponentValue], _: &Url) -> Option<SpecifiedValue> {
-            None
-        }
-
         #[inline]
         pub fn derive(value: line_height::computed_value::T, context: &computed::Context)
                       -> Au {
             if context.display != display::computed_value::inline {
                 match value {
-                    line_height::Normal => context.font_size.scale_by(1.14),
+                    line_height::Normal => context.font_size.scale_by(DEFAULT_LINE_HEIGHT),
                     line_height::Number(percentage) => context.font_size.scale_by(percentage),
                     line_height::Length(length) => length,
                 }
@@ -573,8 +571,7 @@ pub mod longhands {
             use super::super::common_types::specified;
 
             pub mod computed_value {
-                pub use super::super::super::common_types::computed::{LP_Length, LP_Percentage};
+                use super::super::super::common_types::computed::LengthOrPercentage;
-                pub use super::super::super::common_types::computed::{LengthOrPercentage};
 
                 #[deriving(Eq, Clone)]
                 pub struct T {
@@ -601,11 +598,12 @@ pub mod longhands {
             #[inline]
             pub fn get_initial_value() -> computed_value::T {
                 computed_value::T {
-                    horizontal: computed_value::LP_Percentage(0.0),
+                    horizontal: computed::LP_Percentage(0.0),
-                    vertical: computed_value::LP_Percentage(0.0),
+                    vertical: computed::LP_Percentage(0.0),
                 }
             }
 
+            // FIXME(#1997, pcwalton): Support complete CSS2 syntax.
             pub fn parse_horizontal_and_vertical(horiz: &ComponentValue, vert: &ComponentValue)
                                                  -> Option<SpecifiedValue> {
                 let horiz = match specified::LengthOrPercentage::parse_non_negative(horiz) {
@@ -625,13 +623,11 @@ pub mod longhands {
             }
 
             pub fn parse(input: &[ComponentValue], _: &Url) -> Option<SpecifiedValue> {
-                let (mut horizontal, mut vertical) = (None, None);
+                let mut input_iter = input.skip_whitespace();
-                for value in input.skip_whitespace() {
+                let horizontal = input_iter.next();
-                    match (horizontal, vertical) {
+                let vertical = input_iter.next();
-                        (None, None) => horizontal = Some(value),
+                if input_iter.next().is_some() {
-                        (Some(_), None) => vertical = Some(value),
+                    return None
-                        _ => return None,
-                    }
                 }
 
                 match (horizontal, vertical) {
@@ -1005,21 +1001,33 @@ 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) => { color = Some(v); any = true; continue },
+                            Some(v) => {
+                                color = Some(v);
+                                any = true;
+                                continue
+                            },
                             None => ()
                         }
                     }
 
                     if image.is_none() {
                         match background_image::from_component_value(component_value, base_url) {
-                            Some(v) => { image = Some(v); any = true; continue },
+                            Some(v) => {
+                                image = Some(v);
+                                any = true;
+                                continue
+                            },
                             None => (),
                         }
                     }
 
                     if repeat.is_none() {
                         match background_repeat::from_component_value(component_value, base_url) {
-                            Some(v) => { repeat = Some(v); any = true; continue },
+                            Some(v) => {
+                                repeat = Some(v);
+                                any = true;
+                                continue
+                            },
                             None => ()
                         }
                     }
@@ -1027,7 +1035,11 @@ pub mod shorthands {
                     if attachment.is_none() {
                         match background_attachment::from_component_value(component_value,
                                                                           base_url) {
-                            Some(v) => { attachment = Some(v); any = true; continue },
+                            Some(v) => {
+                                attachment = Some(v);
+                                any = true;
+                                continue
+                            },
                             None => ()
                         }
                     }
@@ -1038,7 +1050,11 @@ pub mod shorthands {
                                 match background_position::parse_horizontal_and_vertical(
                                         saved_component_value,
                                         component_value) {
-                                    Some(v) => { position = Some(v); any = true; continue },
+                                    Some(v) => {
+                                        position = Some(v);
+                                        any = true;
+                                        continue
+                                    },
                                     None => (),
                                 }
                             }

src/components/util/smallvec.rs

@@ -61,8 +61,12 @@ pub trait SmallVec<T> : SmallVecPrivate<T> {
     }
 
     fn mut_iter<'a>(&'a mut self) -> SmallVecMutIterator<'a,T> {
+        unsafe {
             SmallVecMutIterator {
-            iter: self.iter(),
+                ptr: cast::transmute(self.begin()),
+                end: cast::transmute(self.end()),
+                lifetime: None,
+            }
         }
     }
 
@@ -207,17 +211,25 @@ impl<'a,T> Iterator<&'a T> for SmallVecIterator<'a,T> {
 }
 
 pub struct SmallVecMutIterator<'a,T> {
-    priv iter: SmallVecIterator<'a,T>,
+    priv ptr: *mut T,
+    priv end: *mut T,
+    priv lifetime: Option<&'a mut T>
 }
 
 impl<'a,T> Iterator<&'a mut T> for SmallVecMutIterator<'a,T> {
     #[inline]
     fn next(&mut self) -> Option<&'a mut T> {
         unsafe {
-            match self.iter.next() {
+            if self.ptr == self.end {
-                None => None,
+                return None
-                Some(reference) => Some(cast::transmute::<&'a T,&'a mut T>(reference)),
             }
+            let old = self.ptr;
+            self.ptr = if mem::size_of::<T>() == 0 {
+                cast::transmute(self.ptr as uint + 1)
+            } else {
+                self.ptr.offset(1)
+            };
+            Some(cast::transmute(old))
         }
     }
 }

src/support/azure/rust-azure

@@ -1 +1 @@
-Subproject commit aa3e649a68b6622a26d45c78023a717d76ff369b
+Subproject commit 10b1cf8e35fa1757bcab11dcc284f77e4ceeaffc