Address review comments

components/layout/block.rs

@@ -1589,7 +1589,8 @@ impl Flow for BlockFlow {
 
         // Our inline-size was set to the inline-size of the containing block by the flow's parent.
         // Now compute the real value.
-        self.propagate_and_compute_used_inline_size(layout_context, border_collapse::T::separate);
+        let border_collapse = self.fragment.style.get_inheritedtable().border_collapse;
+        self.propagate_and_compute_used_inline_size(layout_context, border_collapse);
 
         // Formatting contexts are never impacted by floats.
         match self.formatting_context_type() {

components/layout/inline.rs

@@ -28,8 +28,8 @@ use std::num::ToPrimitive;
 use std::ops::{Add, Sub, Mul, Div, Rem, Neg, Shl, Shr, Not, BitOr, BitAnd, BitXor};
 use std::sync::Arc;
 use std::u16;
-use style::computed_values::{border_collapse, display, overflow_x, text_align, text_justify};
+use style::computed_values::{display, overflow_x, text_align, text_justify, text_overflow};
-use style::computed_values::{text_overflow, vertical_align, white_space};
+use style::computed_values::{vertical_align, white_space};
 use style::properties::ComputedValues;
 use util::geometry::{Au, MAX_AU, ZERO_RECT};
 use util::logical_geometry::{LogicalRect, LogicalSize, WritingMode};
@@ -1197,7 +1197,8 @@ impl Flow for InlineFlow {
         {
             let this = &mut *self;
             for fragment in this.fragments.fragments.iter_mut() {
-                fragment.compute_border_and_padding(inline_size, border_collapse::T::separate);
+                let border_collapse = fragment.style.get_inheritedtable().border_collapse;
+                fragment.compute_border_and_padding(inline_size, border_collapse);
                 fragment.compute_block_direction_margins(inline_size);
                 fragment.compute_inline_direction_margins(inline_size);
                 fragment.assign_replaced_inline_size_if_necessary(inline_size);

components/layout/table_row.rs

@@ -30,7 +30,7 @@ use style::computed_values::{border_collapse, border_spacing, border_top_style};
 use style::properties::ComputedValues;
 use style::values::computed::LengthOrPercentageOrAuto;
 use util::geometry::Au;
-use util::logical_geometry::{LogicalRect, WritingMode};
+use util::logical_geometry::{LogicalRect, PhysicalSide, WritingMode};
 
 /// A single row of a table.
 pub struct TableRowFlow {
@@ -593,58 +593,53 @@ impl CollapsedBorder {
         }
     }
 
+    /// Creates a collapsed border style from the given physical side.
+    fn from_side(side: PhysicalSide,
+                 css_style: &ComputedValues,
+                 provenance: CollapsedBorderProvenance)
+                 -> CollapsedBorder {
+        match side {
+            PhysicalSide::Top => CollapsedBorder::top(css_style, provenance),
+            PhysicalSide::Right => CollapsedBorder::right(css_style, provenance),
+            PhysicalSide::Bottom => CollapsedBorder::bottom(css_style, provenance),
+            PhysicalSide::Left => CollapsedBorder::left(css_style, provenance),
+        }
+    }
+
     /// Creates a collapsed border style from the inline-start border described in the given CSS
     /// style object.
     pub fn inline_start(css_style: &ComputedValues, provenance: CollapsedBorderProvenance)
                         -> CollapsedBorder {
-        let writing_mode = css_style.writing_mode;
+        CollapsedBorder::from_side(css_style.writing_mode.inline_start_physical_side(),
-        match (writing_mode.is_vertical(),
+                                   css_style,
-               writing_mode.is_inline_tb(),
+                                   provenance)
-               writing_mode.is_bidi_ltr()) {
-            (false, _, true) => CollapsedBorder::left(css_style, provenance),
-            (false, _, false) => CollapsedBorder::right(css_style, provenance),
-            (true, true, _) => CollapsedBorder::top(css_style, provenance),
-            (true, false, _) => CollapsedBorder::bottom(css_style, provenance),
-        }
     }
 
     /// Creates a collapsed border style from the inline-start border described in the given CSS
     /// style object.
     pub fn inline_end(css_style: &ComputedValues, provenance: CollapsedBorderProvenance)
                       -> CollapsedBorder {
-        let writing_mode = css_style.writing_mode;
+        CollapsedBorder::from_side(css_style.writing_mode.inline_end_physical_side(),
-        match (writing_mode.is_vertical(),
+                                   css_style,
-               writing_mode.is_inline_tb(),
+                                   provenance)
-               writing_mode.is_bidi_ltr()) {
-            (false, _, true) => CollapsedBorder::right(css_style, provenance),
-            (false, _, false) => CollapsedBorder::left(css_style, provenance),
-            (true, true, _) => CollapsedBorder::bottom(css_style, provenance),
-            (true, false, _) => CollapsedBorder::top(css_style, provenance),
-        }
     }
 
     /// Creates a collapsed border style from the block-start border described in the given CSS
     /// style object.
     pub fn block_start(css_style: &ComputedValues, provenance: CollapsedBorderProvenance)
                        -> CollapsedBorder {
-        let writing_mode = css_style.writing_mode;
+        CollapsedBorder::from_side(css_style.writing_mode.block_start_physical_side(),
-        match (writing_mode.is_vertical(), writing_mode.is_vertical_lr()) {
+                                   css_style,
-            (false, _) => CollapsedBorder::top(css_style, provenance),
+                                   provenance)
-            (true, true) => CollapsedBorder::left(css_style, provenance),
-            (true, false) => CollapsedBorder::right(css_style, provenance),
-        }
     }
 
     /// Creates a collapsed border style from the block-end border described in the given CSS style
     /// object.
     pub fn block_end(css_style: &ComputedValues, provenance: CollapsedBorderProvenance)
                      -> CollapsedBorder {
-        let writing_mode = css_style.writing_mode;
+        CollapsedBorder::from_side(css_style.writing_mode.block_end_physical_side(),
-        match (writing_mode.is_vertical(), writing_mode.is_vertical_lr()) {
+                                   css_style,
-            (false, _) => CollapsedBorder::bottom(css_style, provenance),
+                                   provenance)
-            (true, true) => CollapsedBorder::right(css_style, provenance),
-            (true, false) => CollapsedBorder::left(css_style, provenance),
-        }
     }
 
     /// If `other` has a higher priority per CSS 2.1 § 17.6.2.1, replaces `self` with it.
@@ -659,8 +654,8 @@ impl CollapsedBorder {
             // Step 3.
             _ if self.width > other.width => {}
             _ if self.width < other.width => *self = *other,
-            (this_style, other_style) if (this_style as i8) > other_style as i8 => {}
+            (this_style, other_style) if this_style > other_style => {}
-            (this_style, other_style) if (this_style as i8) < other_style as i8 => *self = *other,
+            (this_style, other_style) if this_style < other_style => *self = *other,
             // Step 4.
             _ if (self.provenance as i8) >= other.provenance as i8 => {}
             _ => *self = *other,

components/style/values.rs

@@ -51,7 +51,7 @@ macro_rules! define_numbered_css_keyword_enum {
     };
     ($name: ident: $( $css: expr => $variant: ident = $value: expr ),+) => {
         #[allow(non_camel_case_types)]
-        #[derive(Clone, Eq, PartialEq, FromPrimitive, Copy, RustcEncodable)]
+        #[derive(Clone, Eq, PartialEq, PartialOrd, Ord, FromPrimitive, Copy, RustcEncodable)]
         pub enum $name {
             $( $variant = $value ),+
         }

components/util/logical_geometry.rs

@@ -47,6 +47,44 @@ impl WritingMode {
     pub fn is_sideways_left(&self) -> bool {
         self.intersects(FLAG_SIDEWAYS_LEFT)
     }
+
+    #[inline]
+    pub fn inline_start_physical_side(&self) -> PhysicalSide {
+        match (self.is_vertical(), self.is_inline_tb(), self.is_bidi_ltr()) {
+            (false, _, true) => PhysicalSide::Left,
+            (false, _, false) => PhysicalSide::Right,
+            (true, true, _) => PhysicalSide::Top,
+            (true, false, _) => PhysicalSide::Bottom,
+        }
+    }
+
+    #[inline]
+    pub fn inline_end_physical_side(&self) -> PhysicalSide {
+        match (self.is_vertical(), self.is_inline_tb(), self.is_bidi_ltr()) {
+            (false, _, true) => PhysicalSide::Right,
+            (false, _, false) => PhysicalSide::Left,
+            (true, true, _) => PhysicalSide::Bottom,
+            (true, false, _) => PhysicalSide::Top,
+        }
+    }
+
+    #[inline]
+    pub fn block_start_physical_side(&self) -> PhysicalSide {
+        match (self.is_vertical(), self.is_vertical_lr()) {
+            (false, _) => PhysicalSide::Top,
+            (true, true) => PhysicalSide::Left,
+            (true, false) => PhysicalSide::Right,
+        }
+    }
+
+    #[inline]
+    pub fn block_end_physical_side(&self) -> PhysicalSide {
+        match (self.is_vertical(), self.is_vertical_lr()) {
+            (false, _) => PhysicalSide::Bottom,
+            (true, true) => PhysicalSide::Right,
+            (true, false) => PhysicalSide::Left,
+        }
+    }
 }
 
 impl Debug for WritingMode {
@@ -965,3 +1003,12 @@ impl<T: Copy + Add<T, Output=T> + Sub<T, Output=T>> Sub<LogicalMargin<T>> for Lo
         }
     }
 }
+
+#[derive(Copy, Clone, PartialEq, Debug)]
+pub enum PhysicalSide {
+    Top,
+    Right,
+    Bottom,
+    Left,
+}
+