auto merge of #1699 : kmcallister/servo/border-box, r=pcwalton

I think this is the only change necessary to make block and inline boxes consistent.  But I'm finding it hard to test due to other bugs (#1696, #1697, #1698).

src/components/main/layout/block.rs

@@ -386,7 +386,7 @@ impl BlockFlow {
 
         let mut noncontent_height = Au::new(0);
         for box_ in self.box_.iter() {
-            let mut position = box_.position.get();
+            let mut position = box_.border_box.get();
             let mut margin = box_.margin.get();
 
             // The associated box is the border box of this flow.
@@ -421,7 +421,7 @@ impl BlockFlow {
 
             noncontent_height = noncontent_height + clearance + margin.top + margin.bottom;
 
-            box_.position.set(position);
+            box_.border_box.set(position);
             box_.margin.set(margin);
         }
 
@@ -450,7 +450,7 @@ impl BlockFlow {
         let mut margin_height = Au(0);
 
         for box_ in self.box_.iter() {
-            height = box_.position.get().size.height;
+            height = box_.border_box.get().size.height;
             clearance = match box_.clear() {
                 None => Au(0),
                 Some(clear) => self.base.floats_in.clearance(clear),
@@ -507,7 +507,7 @@ impl BlockFlow {
 
         let mut noncontent_height;
         let box_ = self.box_.as_ref().unwrap();
-        let mut position = box_.position.get();
+        let mut position = box_.border_box.get();
 
         // The associated box is the border box of this flow.
         position.origin.y = box_.margin.get().top;
@@ -523,7 +523,7 @@ impl BlockFlow {
         debug!("assign_height_float -- height: {}", height);
 
         position.size.height = height;
-        box_.position.set(position);
+        box_.border_box.set(position);
     }
 
     pub fn build_display_list_block<E:ExtraDisplayListData>(
@@ -756,7 +756,7 @@ impl Flow for BlockFlow {
             remaining_width = w;
 
             // The associated box is the border box of this flow.
-            let mut position_ref = box_.position.borrow_mut();
+            let mut position_ref = box_.border_box.borrow_mut();
             if self.is_fixed {
                 position_ref.get().origin.x = x_offset + box_.margin.get().left;
                 x_offset = x_offset + box_.padding.get().left;

src/components/main/layout/box_.rs

@@ -72,7 +72,8 @@ pub struct Box {
     style: Arc<ComputedValues>,
 
     /// The position of this box relative to its owning flow.
-    position: RefCell<Rect<Au>>,
+    /// The size includes padding and border, but not margin.
+    border_box: RefCell<Rect<Au>>,
 
     /// The border of the content box.
     ///
@@ -304,6 +305,80 @@ pub struct InlineParentInfo {
     node: OpaqueNode,
 }
 
+// FIXME: Take just one parameter and use concat_ident! (mozilla/rust#12249)
+macro_rules! def_noncontent( ($side:ident, $get:ident, $inline_get:ident) => (
+    impl Box {
+        pub fn $get(&self) -> Au {
+            self.border.get().$side + self.padding.get().$side
+        }
+
+        pub fn $inline_get(&self) -> Au {
+            let mut val = Au::new(0);
+            let info = self.inline_info.borrow();
+            match info.get() {
+                &Some(ref info) => {
+                    for info in info.parent_info.iter() {
+                        val = val + info.border.$side + info.padding.$side;
+                    }
+                },
+                &None => {}
+            }
+            val
+        }
+    }
+))
+
+macro_rules! def_noncontent_horiz( ($side:ident, $merge:ident, $clear:ident) => (
+    impl Box {
+        pub fn $merge(&self, other_box: &Box) {
+            let mut info = self.inline_info.borrow_mut();
+            let other_info = other_box.inline_info.borrow();
+
+            match other_info.get() {
+                &Some(ref other_info) => {
+                    match info.get() {
+                        &Some(ref mut info) => {
+                            for other_item in other_info.parent_info.iter() {
+                                for item in info.parent_info.mut_iter() {
+                                    if item.node == other_item.node {
+                                        item.border.$side = other_item.border.$side;
+                                        item.padding.$side = other_item.padding.$side;
+                                        item.margin.$side = other_item.margin.$side;
+                                        break;
+                                    }
+                                }
+                            }
+                        },
+                        &None => {}
+                    }
+                },
+                &None => {}
+            }
+        }
+
+        pub fn $clear(&self) {
+            let mut info = self.inline_info.borrow_mut();
+            match info.get() {
+                &Some(ref mut info) => {
+                    for item in info.parent_info.mut_iter() {
+                        item.border.$side = Au::new(0);
+                        item.padding.$side = Au::new(0);
+                        item.margin.$side = Au::new(0);
+                    }
+                },
+                &None => {}
+            }
+        }
+    }
+))
+
+def_noncontent!(left,   noncontent_left,   noncontent_inline_left)
+def_noncontent!(right,  noncontent_right,  noncontent_inline_right)
+def_noncontent!(top,    noncontent_top,    noncontent_inline_top)
+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)
 
 impl Box {
     /// Constructs a new `Box` instance.
@@ -311,7 +386,7 @@ impl Box {
         Box {
             node: OpaqueNode::from_thread_safe_layout_node(&node),
             style: node.style().clone(),
-            position: RefCell::new(Au::zero_rect()),
+            border_box: RefCell::new(Au::zero_rect()),
             border: RefCell::new(Zero::zero()),
             padding: RefCell::new(Zero::zero()),
             margin: RefCell::new(Zero::zero()),
@@ -330,7 +405,7 @@ impl Box {
         Box {
             node: node,
             style: style,
-            position: RefCell::new(Au::zero_rect()),
+            border_box: RefCell::new(Au::zero_rect()),
             border: RefCell::new(Zero::zero()),
             padding: RefCell::new(Zero::zero()),
             margin: RefCell::new(Zero::zero()),
@@ -427,7 +502,7 @@ impl Box {
         Box {
             node: self.node,
             style: self.style.clone(),
-            position: RefCell::new(Rect(self.position.get().origin, size)),
+            border_box: RefCell::new(Rect(self.border_box.get().origin, size)),
             border: RefCell::new(self.border.get()),
             padding: RefCell::new(self.padding.get()),
             margin: RefCell::new(self.margin.get()),
@@ -536,157 +611,6 @@ impl Box {
         self.noncontent_top() + self.noncontent_bottom()
     }
 
-    pub fn noncontent_left(&self) -> Au {
-        self.margin.get().left + self.border.get().left + self.padding.get().left
-    }
-
-    pub fn noncontent_right(&self) -> Au {
-        self.margin.get().right + self.border.get().right + self.padding.get().right
-    }
-
-    pub fn noncontent_top(&self) -> Au {
-        self.margin.get().top + self.border.get().top + self.padding.get().top
-    }
-
-    pub fn noncontent_bottom(&self) -> Au {
-        self.margin.get().bottom + self.border.get().bottom + self.padding.get().bottom
-    }
-
-    pub fn noncontent_inline_left(&self) -> Au {
-        let mut left = Au::new(0);
-        let info = self.inline_info.borrow();
-        match info.get() {
-            &Some(ref info) => {
-                for info in info.parent_info.iter() {
-                    left = left + info.margin.left + info.border.left + info.padding.left;
-                }
-            },
-            &None => {}
-        }
-        left
-    }
-
-    pub fn noncontent_inline_right(&self) -> Au {
-        let mut right = Au::new(0);
-        let info = self.inline_info.borrow();
-        match info.get() {
-            &Some(ref info) => {
-                for info in info.parent_info.iter() {
-                    right = right + info.margin.right + info.border.right + info.padding.right;
-                }
-            },
-            &None => {}
-        }
-        right
-    }
-
-    pub fn noncontent_inline_top(&self) -> Au {
-        let mut top = Au::new(0);
-        let info = self.inline_info.borrow();
-        match info.get() {
-            &Some(ref info) => {
-                for info in info.parent_info.iter() {
-                    top = top + info.margin.top + info.border.top + info.padding.top;
-                }
-            },
-            &None => {}
-        }
-        top
-    }
-
-    pub fn noncontent_inline_bottom(&self) -> Au {
-        let mut bottom = Au::new(0);
-        let info = self.inline_info.borrow();
-        match info.get() {
-            &Some(ref info) => {
-                for info in info.parent_info.iter() {
-                    bottom = bottom + info.margin.bottom + info.border.bottom + info.padding.bottom;
-                }
-            },
-            &None => {}
-        }
-        bottom
-    }
-
-    pub fn merge_noncontent_inline_right(&self, other_box: &Box) {
-        let mut info = self.inline_info.borrow_mut();
-        let other_info = other_box.inline_info.borrow();
-
-        match other_info.get() {
-            &Some(ref other_info) => {
-                match info.get() {
-                    &Some(ref mut info) => {
-                        for other_item in other_info.parent_info.iter() {
-                            for item in info.parent_info.mut_iter() {
-                                if item.node == other_item.node {
-                                    item.border.right = other_item.border.right;
-                                    item.padding.right = other_item.padding.right;
-                                    item.margin.right = other_item.margin.right;
-                                    break;
-                                }
-                            }
-                        }
-                    },
-                    &None => {}
-                }
-            },
-            &None => {}
-        }
-    }
-
-    pub fn merge_noncontent_inline_left(&self, other_box: &Box) {
-        let mut info = self.inline_info.borrow_mut();
-        let other_info = other_box.inline_info.borrow();
-
-        match other_info.get() {
-            &Some(ref other_info) => {
-                match info.get() {
-                    &Some(ref mut info) => {
-                        for other_item in other_info.parent_info.iter() {
-                            for item in info.parent_info.mut_iter() {
-                                if item.node == other_item.node {
-                                    item.border.left = other_item.border.left;
-                                    item.padding.left = other_item.padding.left;
-                                    item.margin.left = other_item.margin.left;
-                                    break;
-                                }
-                            }
-                        }
-                    },
-                    &None => {}
-                }
-            },
-            &None => {}
-        }
-    }
-
-    pub fn clear_noncontent_inline_right(&self) {
-        let mut info = self.inline_info.borrow_mut();
-        match info.get() {
-            &Some(ref mut info) => {
-                for item in info.parent_info.mut_iter() {
-                    item.border.right = Au::new(0);
-                    item.padding.right = Au::new(0);
-                    item.margin.right = Au::new(0);
-                }
-            },
-            &None => {}
-        }
-    }
-
-    pub fn clear_noncontent_inline_left(&self) {
-        let mut info = self.inline_info.borrow_mut();
-        match info.get() {
-            &Some(ref mut info) => {
-                for item in info.parent_info.mut_iter() {
-                    item.border.left = Au::new(0);
-                    item.padding.left = Au::new(0);
-                    item.margin.left = Au::new(0);
-                }
-            },
-            &None => {}
-        }
-    }
     pub fn relative_position(&self, container_block_size: &Size2D<Au>) -> Point2D<Au> {
         fn left_right(style: &ComputedValues, block_width: Au) -> Au {
             // TODO(ksh8281) : consider RTL(right-to-left) culture
@@ -1068,7 +992,7 @@ impl Box {
                               flow: &Flow,
                               index: uint,
                               lists: &RefCell<DisplayListCollection<E>>) {
-        let box_bounds = self.position.get();
+        let box_bounds = self.border_box.get();
         let absolute_box_bounds = box_bounds.translate(&offset);
         debug!("Box::build_display_list at rel={}, abs={}: {:s}",
                box_bounds, absolute_box_bounds, self.debug_str());
@@ -1471,7 +1395,7 @@ impl Box {
                 let left_box = if left_range.length() > 0 {
                     let new_text_box_info = ScannedTextBoxInfo::new(text_box_info.run.clone(), left_range);
                     let mut new_metrics = new_text_box_info.run.get().metrics_for_range(&left_range);
-                    new_metrics.bounding_box.size.height = self.position.get().size.height;
+                    new_metrics.bounding_box.size.height = self.border_box.get().size.height;
                     Some(self.transform(new_metrics.bounding_box.size,
                                         ScannedTextBox(new_text_box_info)))
                 } else {
@@ -1481,7 +1405,7 @@ impl Box {
                 let right_box = right_range.map_default(None, |range: Range| {
                     let new_text_box_info = ScannedTextBoxInfo::new(text_box_info.run.clone(), range);
                     let mut new_metrics = new_text_box_info.run.get().metrics_for_range(&range);
-                    new_metrics.bounding_box.size.height = self.position.get().size.height;
+                    new_metrics.bounding_box.size.height = self.border_box.get().size.height;
                     Some(self.transform(new_metrics.bounding_box.size,
                                         ScannedTextBox(new_text_box_info)))
                 });
@@ -1540,14 +1464,14 @@ impl Box {
                     }
                 };
 
-                let mut position = self.position.borrow_mut();
+                let mut position = self.border_box.borrow_mut();
                 position.get().size.width = width + self.noncontent_width() +
                     self.noncontent_inline_left() + self.noncontent_inline_right();
                 image_box_info.computed_width.set(Some(width));
             }
             ScannedTextBox(_) => {
                 // Scanned text boxes will have already had their content_widths assigned by this point.
-                let mut position = self.position.borrow_mut();
+                let mut position = self.border_box.borrow_mut();
                 position.get().size.width = position.get().size.width + self.noncontent_width() +
                     self.noncontent_inline_left() + self.noncontent_inline_right();
             }
@@ -1584,13 +1508,13 @@ impl Box {
                     }
                 };
 
-                let mut position = self.position.borrow_mut();
+                let mut position = self.border_box.borrow_mut();
                 image_box_info.computed_height.set(Some(height));
                 position.get().size.height = height + self.noncontent_height()
             }
             ScannedTextBox(_) => {
                 // Scanned text boxes will have already had their widths assigned by this point
-                let mut position = self.position.borrow_mut();
+                let mut position = self.border_box.borrow_mut();
                 position.get().size.height
                     = position.get().size.height + self.noncontent_height()
             }
@@ -1665,8 +1589,8 @@ impl Box {
             self.padding.get().left;
         let top = offset.y + self.margin.get().top + self.border.get().top +
             self.padding.get().top;
-        let width = self.position.get().size.width - self.noncontent_width();
-        let height = self.position.get().size.height - self.noncontent_height();
+        let width = self.border_box.get().size.width - self.noncontent_width();
+        let height = self.border_box.get().size.height - self.noncontent_height();
         let origin = Point2D(geometry::to_frac_px(left) as f32, geometry::to_frac_px(top) as f32);
         let size = Size2D(geometry::to_frac_px(width) as f32, geometry::to_frac_px(height) as f32);
         let rect = Rect(origin, size);

src/components/main/layout/flow.rs

@@ -757,7 +757,7 @@ impl<'a> MutableFlowUtils for &'a mut Flow {
             let container_block_size = match self.class() {
                 BlockFlowClass => {
                     if self.as_block().box_.is_some() {
-                        self.as_block().box_.get_ref().position.get().size
+                        self.as_block().box_.get_ref().border_box.get().size
                     } else {
                         base(self).position.size
                     }

src/components/main/layout/inline.rs

@@ -179,7 +179,7 @@ impl LineboxScanner {
                               -> (Rect<Au>, Au) {
         debug!("LineboxScanner: Trying to place first box of line {}", self.lines.len());
 
-        let first_box_size = first_box.position.get().size;
+        let first_box_size = first_box.border_box.get().size;
         let splittable = first_box.can_split();
         debug!("LineboxScanner: box size: {}, splittable: {}", first_box_size, splittable);
         let line_is_empty: bool = self.pending_line.range.length() == 0;
@@ -234,9 +234,9 @@ impl LineboxScanner {
 
                 debug!("LineboxScanner: case=box split and fit");
                 let actual_box_width = match (left, right) {
-                    (Some(l_box), Some(_))  => l_box.position.get().size.width,
-                    (Some(l_box), None)     => l_box.position.get().size.width,
-                    (None, Some(r_box))     => r_box.position.get().size.width,
+                    (Some(l_box), Some(_))  => l_box.border_box.get().size.width,
+                    (Some(l_box), None)     => l_box.border_box.get().size.width,
+                    (None, Some(r_box))     => r_box.border_box.get().size.width,
                     (None, None)            => fail!("This case makes no sense.")
                 };
                 return (line_bounds, actual_box_width);
@@ -248,9 +248,9 @@ impl LineboxScanner {
 
                 debug!("LineboxScanner: case=box split and fit didn't fit; trying to push it down");
                 let actual_box_width = match (left, right) {
-                    (Some(l_box), Some(_))  => l_box.position.get().size.width,
-                    (Some(l_box), None)     => l_box.position.get().size.width,
-                    (None, Some(r_box))     => r_box.position.get().size.width,
+                    (Some(l_box), Some(_))  => l_box.border_box.get().size.width,
+                    (Some(l_box), None)     => l_box.border_box.get().size.width,
+                    (None, Some(r_box))     => r_box.border_box.get().size.width,
                     (None, None)            => fail!("This case makes no sense.")
                 };
 
@@ -349,7 +349,7 @@ impl LineboxScanner {
         debug!("LineboxScanner: Trying to append box to line {:u} (box size: {}, green zone: \
                 {}): {:s}",
                self.lines.len(),
-               in_box.position.get().size,
+               in_box.border_box.get().size,
                self.pending_line.green_zone,
                in_box.debug_str());
 
@@ -369,7 +369,7 @@ impl LineboxScanner {
         // horizontally. We'll try to place the whole box on this line and break somewhere if it
         // doesn't fit.
 
-        let new_width = self.pending_line.bounds.size.width + in_box.position.get().size.width;
+        let new_width = self.pending_line.bounds.size.width + in_box.border_box.get().size.width;
         if new_width <= green_zone.width {
             debug!("LineboxScanner: case=box fits without splitting");
             self.push_box_to_line(in_box);
@@ -440,9 +440,9 @@ impl LineboxScanner {
         }
         self.pending_line.range.extend_by(1);
         self.pending_line.bounds.size.width = self.pending_line.bounds.size.width +
-            box_.position.get().size.width;
+            box_.border_box.get().size.width;
         self.pending_line.bounds.size.height = Au::max(self.pending_line.bounds.size.height,
-                                                       box_.position.get().size.height);
+                                                       box_.border_box.get().size.height);
         self.new_boxes.push(box_);
     }
 }
@@ -603,8 +603,8 @@ impl InlineFlow {
 
         for i in line.range.eachi() {
             let box_ = &boxes[i];
-            let size = box_.position.get().size;
-            box_.position.set(Rect(Point2D(offset_x, box_.position.get().origin.y), size));
+            let size = box_.border_box.get().size;
+            box_.border_box.set(Rect(Point2D(offset_x, box_.border_box.get().origin.y), size));
             offset_x = offset_x + size.width;
         }
     }
@@ -764,12 +764,12 @@ impl Flow for InlineFlow {
 
                         // Offset from the top of the box is 1/2 of the leading + ascent
                         let text_offset = text_ascent + (line_height - em_size).scale_by(0.5);
-                        text_bounds.translate(&Point2D(cur_box.position.get().origin.x, Au(0)));
+                        text_bounds.translate(&Point2D(cur_box.border_box.get().origin.x, Au(0)));
 
                         (text_offset, line_height - text_offset, text_ascent)
                     },
                     GenericBox | IframeBox(_) => {
-                        let height = cur_box.position.get().size.height;
+                        let height = cur_box.border_box.get().size.height;
                         (height, Au::new(0), height)
                     },
                     UnscannedTextBox(_) => {
@@ -824,7 +824,7 @@ impl Flow for InlineFlow {
                     bottommost = bottom_from_base;
                 }
 
-                cur_box.position.borrow_mut().get().origin.y = line.bounds.origin.y + offset + top;
+                cur_box.border_box.borrow_mut().get().origin.y = line.bounds.origin.y + offset + top;
             }
 
             // Calculate the distance from baseline to the top of the biggest box with 'bottom'
@@ -853,7 +853,7 @@ impl Flow for InlineFlow {
                     _ => baseline_offset,
                 };
 
-                cur_box.position.borrow_mut().get().origin.y = cur_box.position.get().origin.y +
+                cur_box.border_box.borrow_mut().get().origin.y = cur_box.border_box.get().origin.y +
                     adjust_offset;
 
                 if cur_box.inline_info.with(|info| info.is_none()) {

src/test/harness/reftest/reftest.rs

@@ -71,6 +71,11 @@ fn parse_lists(filenames: &[~str]) -> ~[TestDescAndFn] {
         };
 
         for line in contents.lines() {
+            // ignore comments
+            if line.starts_with("#") {
+                continue;
+            }
+
             let parts: ~[&str] = line.split(' ').filter(|p| !p.is_empty()).collect();
 
             if parts.len() != 3 {
@@ -80,7 +85,6 @@ fn parse_lists(filenames: &[~str]) -> ~[TestDescAndFn] {
             let kind = match parts[0] {
                 "==" => Same,
                 "!=" => Different,
-                "#" => continue,
                 _ => fail!("reftest line: '{:s}' has invalid kind '{:s}'",
                            line, parts[0])
             };

src/test/html/simple_iframe.html

@@ -1,7 +0,0 @@
-<html>
-<body>
-  <iframe src="simple_iframe_inner.html"
-          style="display:block; border: 1px; width: 400px; height: 400px">
-  </iframe>
-</body>
-</html>

src/test/html/simple_iframe_inner.html

@@ -1,5 +0,0 @@
-<html>
-<body>
-Just a simple little iframe.
-</body>
-</html>

src/test/ref/500x300_green.html

@@ -0,0 +1,5 @@
+<html>
+<body>
+<div style="width: 500px; height: 300px; background-color: #0f0;"></div>
+</body>
+</html>

src/test/ref/basic.list

@@ -31,3 +31,5 @@
 == data_img_a.html data_img_b.html
 == background_style_attr.html background_ref.html
 == background_external_stylesheet.html background_ref.html
+== block_image.html 500x300_green.html
+# == simple_iframe.html simple_iframe_ref.html -- disabled due to iframe crashiness

src/test/ref/block_image.html

@@ -0,0 +1,6 @@
+<html>
+<body>
+<img style="display: block; width: 500px; height: 300px; background-color: red;"
+     src="400x400_green.png">
+</body>
+</html>

src/test/ref/simple_iframe.html

@@ -0,0 +1,7 @@
+<html>
+<body>
+  <iframe src="data:text/html,%3Cspan%3EJust%20a%20simple%20little%20iframe.%3C%2Fspan%3E"
+          style="display: block; border: 1px solid black; width: 500px; height: 300px; margin-left: 10px; margin-top: 20px;">
+  </iframe>
+</body>
+</html>

src/test/ref/simple_iframe_ref.html

@@ -0,0 +1,7 @@
+<html>
+<body>
+  <div style="border: 1px solid black; width: 500px; height: 300px; margin-left: 10px; margin-top: 20px;">
+    <div style="margin: 8px; /* matches user-agent body */">Just a simple little iframe.</div>
+  </div>
+</body>
+</html>