auto merge of #1578 : pradeep90/servo/stacking-context, r=larsbergstrom,larsbergstrom

For ScannedTextBox and ImageBox, the ClipDisplayItem's child_list is
currently always empty.

ClipDisplayItem is used to implement overflow hidden and should only be
created for block containers, as per
http://www.w3.org/TR/CSS2/visufx.html#propdef-overflow

src/components/main/layout/block.rs

@@ -136,8 +136,8 @@ impl BlockFlow {
                      right_margin: MaybeAuto,
                      available_width: Au)
                      -> (Au, Au, Au) {
-        // If width is not 'auto', and width + margins > available_width, all 'auto' margins are
+        // If width is not 'auto', and width + margins > available_width, all
-        // treated as 0.
+        // 'auto' margins are treated as 0.
         let (left_margin, right_margin) = match width {
             Auto => (left_margin, right_margin),
             Specified(width) => {
@@ -152,34 +152,46 @@ impl BlockFlow {
             }
         };
 
-        //Invariant: left_margin_Au + width_Au + right_margin_Au == available_width
+        // Invariant: left_margin_Au + width_Au + right_margin_Au == available_width
         let (left_margin_Au, width_Au, right_margin_Au) = match (left_margin, width, right_margin) {
-            //If all have a computed value other than 'auto', the system is over-constrained and we need to discard a margin.
+            // If all have a computed value other than 'auto', the system is
-            //if direction is ltr, ignore the specified right margin and solve for it. If it is rtl, ignore the specified
+            // over-constrained and we need to discard a margin.
-            //left margin. FIXME(eatkinson): this assumes the direction is ltr
+            // If direction is ltr, ignore the specified right margin and
-            (Specified(margin_l), Specified(width), Specified(_margin_r)) => (margin_l, width, available_width - (margin_l + width )),
+            // solve for it.
+            // If it is rtl, ignore the specified left margin.
+            // FIXME(eatkinson): this assumes the direction is ltr
+            (Specified(margin_l), Specified(width), Specified(_margin_r)) =>
+                (margin_l, width, available_width - (margin_l + width )),
 
-            //If exactly one value is 'auto', solve for it
+            // If exactly one value is 'auto', solve for it
-            (Auto, Specified(width), Specified(margin_r)) => (available_width - (width + margin_r), width, margin_r),
+            (Auto, Specified(width), Specified(margin_r)) =>
-            (Specified(margin_l), Auto, Specified(margin_r)) => (margin_l, available_width - (margin_l + margin_r), margin_r),
+                (available_width - (width + margin_r), width, margin_r),
-            (Specified(margin_l), Specified(width), Auto) => (margin_l, width, available_width - (margin_l + width)),
+            (Specified(margin_l), Auto, Specified(margin_r)) =>
+                (margin_l, available_width - (margin_l + margin_r), margin_r),
+            (Specified(margin_l), Specified(width), Auto) =>
+                (margin_l, width, available_width - (margin_l + width)),
 
-            //If width is set to 'auto', any other 'auto' value becomes '0', and width is solved for
+            // If width is set to 'auto', any other 'auto' value becomes '0',
-            (Auto, Auto, Specified(margin_r)) => (Au::new(0), available_width - margin_r, margin_r),
+            // and width is solved for
-            (Specified(margin_l), Auto, Auto) => (margin_l, available_width - margin_l, Au::new(0)),
+            (Auto, Auto, Specified(margin_r)) =>
-            (Auto, Auto, Auto) => (Au::new(0), available_width, Au::new(0)),
+                (Au::new(0), available_width - margin_r, margin_r),
+            (Specified(margin_l), Auto, Auto) =>
+                (margin_l, available_width - margin_l, Au::new(0)),
+            (Auto, Auto, Auto) =>
+                (Au::new(0), available_width, Au::new(0)),
 
-            //If left and right margins are auto, they become equal
+            // If left and right margins are auto, they become equal
             (Auto, Specified(width), Auto) => {
                 let margin = (available_width - width).scale_by(0.5);
                 (margin, width, margin)
             }
 
         };
-        //return values in same order as params
+        // Return values in same order as params.
         (width_Au, left_margin_Au, right_margin_Au)
     }
 
+    // Return (content width, left margin, right, margin)
     fn compute_block_margins(&self, box_: &Box, remaining_width: Au, available_width: Au)
                              -> (Au, Au, Au) {
         let style = box_.style();
@@ -194,6 +206,7 @@ impl BlockFlow {
                                                                     maybe_margin_right,
                                                                     available_width);
 
+        // CSS Section 10.4: Minimum and Maximum widths
         // If the tentative used width is greater than 'max-width', width should be recalculated,
         // but this time using the computed value of 'max-width' as the computed value for 'width'.
         let (width, margin_left, margin_right) = {
@@ -223,6 +236,7 @@ impl BlockFlow {
         return (width, margin_left, margin_right);
     }
 
+    // CSS Section 10.3.5
     fn compute_float_margins(&self, box_: &Box, remaining_width: Au) -> (Au, Au, Au) {
         let style = box_.style();
         let margin_left = MaybeAuto::from_style(style.Margin.margin_left,
@@ -243,6 +257,7 @@ impl BlockFlow {
     fn assign_height_block_base(&mut self, ctx: &mut LayoutContext, inorder: bool) {
         let mut cur_y = Au::new(0);
         let mut clearance = Au::new(0);
+        // Offset to content edge of box_
         let mut top_offset = Au::new(0);
         let mut bottom_offset = Au::new(0);
         let mut left_offset = Au::new(0);
@@ -280,7 +295,15 @@ impl BlockFlow {
             }
         }
 
+        // The amount of margin that we can potentially collapse with
         let mut collapsible = Au::new(0);
+        // How much to move up by to get to the beginning of
+        // current kid flow.
+        // Example: if previous sibling's margin-bottom is 20px and your
+        // margin-top is 12px, the collapsed margin will be 20px. Since cur_y
+        // will be at the bottom margin edge of the previous sibling, we have
+        // to move up by 12px to get to our top margin edge. So, `collapsing`
+        // will be set to 12px
         let mut collapsing = Au::new(0);
         let mut margin_top = Au::new(0);
         let mut margin_bottom = Au::new(0);
@@ -300,7 +323,9 @@ impl BlockFlow {
             margin_bottom = box_.margin.get().bottom;
         }
 
+        // At this point, cur_y is at the content edge of the flow's box_
         for kid in self.base.child_iter() {
+            // At this point, cur_y is at bottom margin edge of previous kid
             kid.collapse_margins(top_margin_collapsible,
                                  &mut first_in_flow,
                                  &mut margin_top,
@@ -310,8 +335,11 @@ impl BlockFlow {
 
             let child_node = flow::mut_base(*kid);
             cur_y = cur_y - collapsing;
+            // At this point, after moving up by `collapsing`, cur_y is at the
+            // top margin edge of kid
             child_node.position.origin.y = cur_y;
             cur_y = cur_y + child_node.position.size.height;
+            // At this point, cur_y is at the bottom margin edge of kid
         }
 
         // The bottom margin collapses with its last in-flow block-level child's bottom margin
@@ -338,6 +366,9 @@ impl BlockFlow {
             // infrastructure to make it scrollable.
             Au::max(screen_height, cur_y)
         } else {
+            // (cur_y - collapsing) will get you the bottom content edge
+            // top_offset will be at top content edge
+            // hence, height = content height
             cur_y - top_offset - collapsing
         };
 
@@ -347,18 +378,23 @@ impl BlockFlow {
             // At this point, `height` is the height of the containing block, so passing `height`
             // as the second argument here effectively makes percentages relative to the containing
             // block per CSS 2.1 § 10.5.
+            // TODO: We need to pass in the correct containing block height
+            // for absolutely positioned elems
             height = match MaybeAuto::from_style(style.Box.height, height) {
                 Auto => height,
                 Specified(value) => value
             };
         }
 
+        // Here, height is content height of box_
+
         let mut noncontent_height = Au::new(0);
         for box_ in self.box_.iter() {
             let mut position = box_.position.get();
             let mut margin = box_.margin.get();
 
             // The associated box is the border box of this flow.
+            // Margin after collapse
             margin.top = margin_top;
             margin.bottom = margin_bottom;
 
@@ -373,7 +409,7 @@ impl BlockFlow {
             position.origin.y = y;
             height = h;
 
-            if self.is_fixed { 
+            if self.is_fixed {
                 for kid in self.base.child_iter() {
                     let child_node = flow::mut_base(*kid);
                     child_node.position.origin.y = position.origin.y + top_offset;
@@ -383,6 +419,7 @@ impl BlockFlow {
             position.size.height = if self.is_fixed {
                 height
             } else {
+                // Border box height
                 height + noncontent_height
             };
 
@@ -395,6 +432,7 @@ impl BlockFlow {
         self.base.position.size.height = if self.is_fixed {
             height
         } else {
+            // Height of margin box + clearance
             height + noncontent_height
         };
 
@@ -685,7 +723,7 @@ impl Flow for BlockFlow {
                                                margin_left));
 
             let screen_size = ctx.screen_size;
-            let (x, w) = box_.get_x_coord_and_new_width_if_fixed(screen_size.width, 
+            let (x, w) = box_.get_x_coord_and_new_width_if_fixed(screen_size.width,
                                                                  screen_size.height,
                                                                  width,
                                                                  box_.offset(),
@@ -771,6 +809,7 @@ impl Flow for BlockFlow {
         }
     }
 
+    // CSS Section 8.3.1 - Collapsing Margins
     fn collapse_margins(&mut self,
                         top_margin_collapsible: bool,
                         first_in_flow: &mut bool,
@@ -825,4 +864,3 @@ impl Flow for BlockFlow {
         })
     }
 }
-

src/components/main/layout/box_.rs

@@ -321,14 +321,20 @@ impl Box {
         }
     }
 
+    // CSS Section 10.6.4
+    // We have to solve the constraint equation:
+    // top + bottom + height + (vertical border + padding) = height of
+    // containing block (`screen_height`)
+    //
+    // `y`: static position of the element
     //TODO(ibnc) take into account padding.
-    pub fn get_y_coord_and_new_height_if_fixed(&self, 
+    pub fn get_y_coord_and_new_height_if_fixed(&self,
                                                screen_height: Au,
                                                mut height: Au,
                                                mut y: Au,
                                                is_fixed: bool)
                                                -> (Au, Au) {
-        if is_fixed { 
+        if is_fixed {
             let position_offsets = self.position_offsets.get();
             match (position_offsets.top, position_offsets.bottom) {
                 (Au(0), Au(0)) => {}
@@ -352,6 +358,7 @@ impl Box {
         return (y, height);
     }
 
+    // CSS Section 10.3.7
     //TODO(ibnc) removing padding when width needs to be stretched.
     pub fn get_x_coord_and_new_width_if_fixed(&self,
                                               screen_width: Au,
@@ -960,18 +967,6 @@ impl Box {
         match self.specific {
             UnscannedTextBox(_) => fail!("Shouldn't see unscanned boxes here."),
             ScannedTextBox(ref text_box) => {
-                lists.with_mut(|lists| {
-                    let item = ~ClipDisplayItem {
-                        base: BaseDisplayItem {
-                            bounds: absolute_box_bounds,
-                            extra: ExtraDisplayListData::new(self),
-                        },
-                        child_list: ~[],
-                        need_clip: false
-                    };
-                    lists.lists[index].append_item(ClipDisplayItemClass(item));
-                });
-
                 let text_color = self.style().Color.color.to_gfx_color();
 
                 // Set the various text display item flags.
@@ -991,7 +986,7 @@ impl Box {
                 text_flags.set_override_underline(flow_flags.flags.override_underline());
                 text_flags.set_override_overline(flow_flags.flags.override_overline());
                 text_flags.set_override_line_through(flow_flags.flags.override_line_through());
-                
+
                 let mut bounds = absolute_box_bounds.clone();
                 bounds.origin.x = bounds.origin.x + self.noncontent_left()
                                   + self.noncontent_inline_left();
@@ -1097,18 +1092,6 @@ impl Box {
                 });
             },
             ImageBox(ref image_box) => {
-                lists.with_mut(|lists| {
-                    let item = ~ClipDisplayItem {
-                        base: BaseDisplayItem {
-                            bounds: absolute_box_bounds,
-                            extra: ExtraDisplayListData::new(self),
-                        },
-                        child_list: ~[],
-                        need_clip: false
-                    };
-                    lists.lists[index].append_item(ClipDisplayItemClass(item));
-                });
-
                 let mut image_ref = image_box.image.borrow_mut();
                 let mut bounds = absolute_box_bounds.clone();
                 bounds.origin.x = bounds.origin.x + self.noncontent_left()
@@ -1461,7 +1444,7 @@ impl Box {
                                                         image_box_info.dom_height,
                                                         Au::new(0));
 
-                let height = match (self.style().Box.width, 
+                let height = match (self.style().Box.width,
                                     image_box_info.dom_width,
                                     height) {
                     (LPA_Auto, None, Auto) => {
@@ -1545,7 +1528,7 @@ impl Box {
                 *value.right,
                 *value.bottom,
                 *value.left)
-    } 
+    }
 
     /// Sends the size and position of this iframe box to the constellation. This is out of line to
     /// guide inlining.
@@ -1571,4 +1554,3 @@ impl Box {
         layout_context.constellation_chan.send(msg)
     }
 }
-

src/components/main/layout/flow.rs

@@ -6,20 +6,20 @@
 //! layout constraints to obtain positions and display attributes of tree nodes. Positions are
 //! computed in several tree traversals driven by the fundamental data dependencies required by
 /// inline and block layout.
-/// 
+///
 /// Flows are interior nodes in the layout tree and correspond closely to *flow contexts* in the
 /// CSS specification. Flows are responsible for positioning their child flow contexts and boxes.
 /// Flows have purpose-specific fields, such as auxiliary line box structs, out-of-flow child
 /// lists, and so on.
 ///
 /// Currently, the important types of flows are:
-/// 
+///
 /// * `BlockFlow`: A flow that establishes a block context. It has several child flows, each of
 ///   which are positioned according to block formatting context rules (CSS block boxes). Block
 ///   flows also contain a single `GenericBox` to represent their rendered borders, padding, etc.
 ///   The BlockFlow at the root of the tree has special behavior: it stretches to the boundaries of
 ///   the viewport.
-///   
+///
 /// * `InlineFlow`: A flow that establishes an inline context. It has a flat list of child
 ///   boxes/flows that are subject to inline layout and line breaking and structs to represent
 ///   line breaks and mapping to CSS boxes, for the purpose of handling `getClientRects()` and
@@ -167,6 +167,9 @@ pub trait ImmutableFlowUtils {
     /// Returns the number of children that this flow possesses.
     fn child_count(self) -> uint;
 
+    /// Return true if this flow is a Block Container.
+    fn is_block_container(self) -> bool;
+
     /// Returns true if this flow is a block flow, an inline flow, or a float flow.
     fn starts_block_flow(self) -> bool;
 
@@ -604,6 +607,23 @@ impl<'a> ImmutableFlowUtils for &'a Flow {
         base(self).children.len()
     }
 
+    /// Return true if this flow is a Block Container.
+    ///
+    /// Except for table boxes and replaced elements, block-level boxes (`BlockFlow`) are
+    /// also block container boxes.
+    /// Non-replaced inline blocks and non-replaced table cells are also block
+    /// containers.
+    fn is_block_container(self) -> bool {
+        match self.class() {
+            // TODO: Change this when inline-blocks are supported.
+            InlineFlowClass => false,
+            BlockFlowClass => {
+                // FIXME: Actually check the type of the node
+                self.child_count() != 0
+            }
+        }
+    }
+
     /// Returns true if this flow is a block flow, an inline-block flow, or a float flow.
     fn starts_block_flow(self) -> bool {
         match self.class() {
@@ -698,6 +718,11 @@ impl<'a> MutableFlowUtils for &'a mut Flow {
         mut_base(self).overflow = overflow
     }
 
+    /// Push display items for current flow and its children onto `list`.
+    ///
+    /// For InlineFlow, add display items for all its boxes onto list`.
+    /// For BlockFlow, add a ClipDisplayItemClass for itself and its children,
+    /// plus any other display items like border.
     fn build_display_lists<E:ExtraDisplayListData>(
                           self,
                           builder: &DisplayListBuilder,
@@ -715,32 +740,36 @@ impl<'a> MutableFlowUtils for &'a mut Flow {
             return true;
         }
 
-        let mut child_lists = DisplayListCollection::new();
+        if self.is_block_container() {
-        child_lists.add_list(DisplayList::new());
+            let mut child_lists = DisplayListCollection::new();
-        let child_lists = RefCell::new(child_lists);
+            child_lists.add_list(DisplayList::new());
-        for kid in child_iter(self) {
+            let child_lists = RefCell::new(child_lists);
-            kid.build_display_lists(builder, dirty, 0u, &child_lists);
+            for kid in child_iter(self) {
-        }
+                kid.build_display_lists(builder, dirty, 0u, &child_lists);
-
-        let mut child_lists = Some(child_lists.unwrap());
-        lists.with_mut(|lists| {
-            let mut child_lists = child_lists.take_unwrap();
-            let result = lists.lists[index].list.mut_rev_iter().position(|item| {
-                match *item {
-                    ClipDisplayItemClass(ref mut item) => {
-                        item.child_list.push_all_move(child_lists.lists.shift().list);
-                        true
-                    },
-                    _ => false,
-                }
-            });
-
-            if result.is_none() {
-                fail!("fail to find parent item");
             }
 
-            lists.lists.push_all_move(child_lists.lists);
+            let mut child_lists = Some(child_lists.unwrap());
-        });
+            // Find parent ClipDisplayItemClass and push all child display items
+            // under it
+            lists.with_mut(|lists| {
+                let mut child_lists = child_lists.take_unwrap();
+                let result = lists.lists[index].list.mut_rev_iter().position(|item| {
+                    match *item {
+                        ClipDisplayItemClass(ref mut item) => {
+                            item.child_list.push_all_move(child_lists.lists.shift().list);
+                            true
+                        },
+                        _ => false,
+                    }
+                });
+
+                if result.is_none() {
+                    fail!("fail to find parent item");
+                }
+
+                lists.lists.push_all_move(child_lists.lists);
+            });
+        }
         true
     }
 
@@ -848,4 +877,3 @@ impl FlowLeafSet {
         self.set.iter()
     }
 }
-