Fix direction of columns in mixed LTR/RTL tables.

Table columns should be layed out according to the 'direction' property of the
table flow, regardless of the 'direction' property of any table-row,
table-rowgroup, etc. flows.

This fixes a number of the `direction-applies-to-*` tests in the CSS2.1 test
suite.

This also simplifies `propagate_column_inline_sizes_to_child` by separating
the code used for table cells from the code for non-cell flows.

r? @pcwalton

components/layout/block.rs

@@ -1207,6 +1207,7 @@ impl BlockFlow {
                                                                         usize,
                                                                         Au,
                                                                         WritingMode,
+                                                                        &mut Au,
                                                                         &mut Au) {
         // Keep track of whether floats could impact each child.
         let mut inline_start_floats_impact_child =
@@ -1259,6 +1260,7 @@ impl BlockFlow {
 
         // This value is used only for table cells.
         let mut inline_start_margin_edge = inline_start_content_edge;
+        let mut inline_end_margin_edge = inline_end_content_edge;
 
         let mut iterator = self.base.child_iter().enumerate().peekable();
         while let Some((i, kid)) = iterator.next() {
@@ -1332,7 +1334,8 @@ impl BlockFlow {
                      i,
                      content_inline_size,
                      containing_block_mode,
-                     &mut inline_start_margin_edge);
+                     &mut inline_start_margin_edge,
+                     &mut inline_end_margin_edge);
 
             // Per CSS 2.1 § 16.3.1, text alignment propagates to all children in flow.
             //
@@ -1602,7 +1605,7 @@ impl Flow for BlockFlow {
                                                         inline_start_content_edge,
                                                         inline_end_content_edge,
                                                         content_inline_size,
-                                                        |_, _, _, _, _| {});
+                                                        |_, _, _, _, _, _| {});
     }
 
     fn place_float_if_applicable<'a>(&mut self, _: &'a LayoutContext<'a>) {

components/layout/table.rs

@@ -473,19 +473,16 @@ impl Flow for TableFlow {
                                                                    inline_end_content_edge,
                                                                    content_inline_size,
                                                                    |child_flow,
-                                                                    child_index,
+                                                                    _child_index,
-                                                                    content_inline_size,
+                                                                    _content_inline_size,
                                                                     writing_mode,
-                                                                    inline_start_margin_edge| {
+                                                                    _inline_start_margin_edge,
+                                                                    _inline_end_margin_edge| {
             table_row::propagate_column_inline_sizes_to_child(
                 child_flow,
-                child_index,
-                content_inline_size,
                 writing_mode,
                 column_computed_inline_sizes,
-                &spacing_per_cell,
+                &spacing_per_cell);
-                &None,
-                inline_start_margin_edge);
             if child_flow.is_table_row() {
                 let child_table_row = child_flow.as_table_row();
                 child_table_row.populate_collapsed_border_spacing(

components/layout/table_cell.rs

@@ -157,7 +157,7 @@ impl Flow for TableCellFlow {
                                                                    inline_start_content_edge,
                                                                    inline_end_content_edge,
                                                                    content_inline_size,
-                                                                   |_, _, _, _, _| {});
+                                                                   |_, _, _, _, _, _| {});
     }
 
     fn assign_block_size<'a>(&mut self, ctx: &'a LayoutContext<'a>) {

components/layout/table_row.rs

@@ -9,7 +9,7 @@
 use block::{BlockFlow, ISizeAndMarginsComputer};
 use context::LayoutContext;
 use display_list_builder::{BlockFlowDisplayListBuilding, BorderPaintingMode};
-use flow::{self, BaseFlow, FlowClass, Flow, ImmutableFlowUtils};
+use flow::{self, FlowClass, Flow, ImmutableFlowUtils};
 use flow_list::MutFlowListIterator;
 use fragment::{Fragment, FragmentBorderBoxIterator};
 use layout_debug;
@@ -47,6 +47,10 @@ pub struct TableRowFlow {
     /// phase.
     pub spacing: border_spacing::T,
 
+    /// The direction of the columns, propagated down from the table during the inline-size
+    /// assignment phase.
+    pub table_writing_mode: WritingMode,
+
     /// Information about the borders for each cell that we bubble up to our parent. This is only
     /// computed if `border-collapse` is `collapse`.
     pub preliminary_collapsed_borders: CollapsedBordersForRow,
@@ -78,6 +82,7 @@ pub struct CellIntrinsicInlineSize {
 impl TableRowFlow {
     pub fn from_node_and_fragment(node: &ThreadSafeLayoutNode, fragment: Fragment)
                                   -> TableRowFlow {
+        let writing_mode = fragment.style().writing_mode;
         TableRowFlow {
             block_flow: BlockFlow::from_node_and_fragment(node, fragment, None),
             cell_intrinsic_inline_sizes: Vec::new(),
@@ -86,6 +91,7 @@ impl TableRowFlow {
                 horizontal: Au(0),
                 vertical: Au(0),
             },
+            table_writing_mode: writing_mode,
             preliminary_collapsed_borders: CollapsedBordersForRow::new(),
             final_collapsed_borders: CollapsedBordersForRow::new(),
             collapsed_border_spacing: CollapsedBorderSpacingForRow::new(),
@@ -366,6 +372,8 @@ impl Flow for TableRowFlow {
 
         // Push those inline sizes down to the cells.
         let spacing = self.spacing;
+        let row_writing_mode = self.block_flow.base.writing_mode;
+        let table_writing_mode = self.table_writing_mode;
         self.block_flow.propagate_assigned_inline_size_to_children(layout_context,
                                                                    inline_start_content_edge,
                                                                    inline_end_content_edge,
@@ -373,17 +381,20 @@ impl Flow for TableRowFlow {
                                                                    |child_flow,
                                                                     child_index,
                                                                     content_inline_size,
-                                                                    writing_mode,
+                                                                    _writing_mode,
-                                                                    inline_start_margin_edge| {
+                                                                    inline_start_margin_edge,
-            propagate_column_inline_sizes_to_child(
+                                                                    inline_end_margin_edge| {
+            set_inline_position_of_child_flow(
                 child_flow,
                 child_index,
-                content_inline_size,
+                row_writing_mode,
-                writing_mode,
+                table_writing_mode,
                 &computed_inline_size_for_cells,
                 &spacing,
                 &border_collapse_info,
-                inline_start_margin_edge)
+                content_inline_size,
+                inline_start_margin_edge,
+                inline_end_margin_edge);
         })
     }
 
@@ -666,49 +677,54 @@ impl CollapsedBorder {
 /// Pushes column inline size and border collapse info down to a child.
 pub fn propagate_column_inline_sizes_to_child(
         child_flow: &mut Flow,
-        child_index: usize,
+        table_writing_mode: WritingMode,
-        content_inline_size: Au,
-        writing_mode: WritingMode,
         column_computed_inline_sizes: &[ColumnComputedInlineSize],
-        border_spacing: &border_spacing::T,
+        border_spacing: &border_spacing::T) {
-        border_collapse_info: &Option<BorderCollapseInfoForChildTableCell>,
-        inline_start_margin_edge: &mut Au) {
     // If the child is a row group or a row, the column inline-size info should be copied from its
     // parent.
     //
     // FIXME(pcwalton): This seems inefficient. Reference count it instead?
-    let inline_size = match child_flow.class() {
+    match child_flow.class() {
         FlowClass::Table => {
             let child_table_flow = child_flow.as_table();
             child_table_flow.column_computed_inline_sizes = column_computed_inline_sizes.to_vec();
-            content_inline_size
         }
         FlowClass::TableRowGroup => {
             let child_table_rowgroup_flow = child_flow.as_table_rowgroup();
             child_table_rowgroup_flow.column_computed_inline_sizes =
                 column_computed_inline_sizes.to_vec();
             child_table_rowgroup_flow.spacing = *border_spacing;
-            content_inline_size
+            child_table_rowgroup_flow.table_writing_mode = table_writing_mode;
         }
         FlowClass::TableRow => {
             let child_table_row_flow = child_flow.as_table_row();
             child_table_row_flow.column_computed_inline_sizes =
                 column_computed_inline_sizes.to_vec();
             child_table_row_flow.spacing = *border_spacing;
-            content_inline_size
+            child_table_row_flow.table_writing_mode = table_writing_mode;
         }
-        FlowClass::TableCell => column_computed_inline_sizes[child_index].size,
+        c => warn!("unexpected flow in table {:?}", c)
-        _ => content_inline_size,
+    }
-    };
+}
 
+/// Lay out table cells inline according to the computer column sizes.
+fn set_inline_position_of_child_flow(
+        child_flow: &mut Flow,
+        child_index: usize,
+        row_writing_mode: WritingMode,
+        table_writing_mode: WritingMode,
+        column_computed_inline_sizes: &[ColumnComputedInlineSize],
+        border_spacing: &border_spacing::T,
+        border_collapse_info: &Option<BorderCollapseInfoForChildTableCell>,
+        parent_content_inline_size: Au,
+        inline_start_margin_edge: &mut Au,
+        inline_end_margin_edge: &mut Au) {
     if !child_flow.is_table_cell() {
-        set_inline_position_of_child_flow(flow::mut_base(child_flow),
-                                          inline_start_margin_edge,
-                                          inline_size,
-                                          writing_mode);
         return
     }
 
+    let reverse_column_order = table_writing_mode.is_bidi_ltr() != row_writing_mode.is_bidi_ltr();
+
     // Handle border collapsing, if necessary.
     let child_table_cell = child_flow.as_table_cell();
     match *border_collapse_info {
@@ -751,21 +767,38 @@ pub fn propagate_column_inline_sizes_to_child(
             };
 
             // Move over past the collapsed border.
-            *inline_start_margin_edge = *inline_start_margin_edge +
+            if reverse_column_order {
-                child_table_cell.collapsed_borders.inline_start_width
+                *inline_end_margin_edge = *inline_end_margin_edge +
+                    child_table_cell.collapsed_borders.inline_start_width
+            } else {
+                *inline_start_margin_edge = *inline_start_margin_edge +
+                    child_table_cell.collapsed_borders.inline_start_width
+            }
         }
         None => {
             // Take spacing into account.
-            *inline_start_margin_edge = *inline_start_margin_edge + border_spacing.horizontal
+            if reverse_column_order {
+                *inline_end_margin_edge = *inline_end_margin_edge + border_spacing.horizontal
+            } else {
+                *inline_start_margin_edge = *inline_start_margin_edge + border_spacing.horizontal
+            }
         }
     }
 
-    set_inline_position_of_child_flow(&mut child_table_cell.block_flow.base,
+    let column_inline_size = column_computed_inline_sizes[child_index].size;
-                                      inline_start_margin_edge,
+    let kid_base = &mut child_table_cell.block_flow.base;
-                                      inline_size,
+    kid_base.block_container_inline_size = column_inline_size;
-                                      writing_mode);
 
-    *inline_start_margin_edge = *inline_start_margin_edge + inline_size
+    if reverse_column_order {
+        // Columns begin from the inline-end edge.
+        kid_base.position.start.i =
+            parent_content_inline_size - *inline_end_margin_edge - column_inline_size;
+        *inline_end_margin_edge = *inline_end_margin_edge + column_inline_size;
+    } else {
+        // Columns begin from the inline-start edge.
+        kid_base.position.start.i = *inline_start_margin_edge;
+        *inline_start_margin_edge = *inline_start_margin_edge + column_inline_size;
+    }
 }
 
 #[derive(Copy, Clone)]
@@ -774,15 +807,6 @@ pub struct BorderCollapseInfoForChildTableCell<'a> {
     collapsed_border_spacing_for_row: &'a CollapsedBorderSpacingForRow,
 }
 
-fn set_inline_position_of_child_flow(child_flow: &mut BaseFlow,
-                                     inline_start_margin_edge: &mut Au,
-                                     inline_size: Au,
-                                     writing_mode: WritingMode) {
-    child_flow.position.start.i = *inline_start_margin_edge;
-    child_flow.block_container_inline_size = inline_size;
-    child_flow.block_container_writing_mode = writing_mode;
-}
-
 /// Performs border-collapse in the inline direction for all the cells' inside borders in the
 /// inline-direction cells and propagates the outside borders (the far left and right) up to the
 /// table row. This is done eagerly here so that at least the inline inside border collapse

components/layout/table_rowgroup.rs

@@ -23,7 +23,7 @@ use std::iter::{IntoIterator, Iterator, Peekable};
 use std::sync::Arc;
 use style::properties::ComputedValues;
 use util::geometry::Au;
-use util::logical_geometry::LogicalRect;
+use util::logical_geometry::{LogicalRect, WritingMode};
 
 /// A table formatting context.
 pub struct TableRowGroupFlow {
@@ -39,6 +39,10 @@ pub struct TableRowGroupFlow {
     /// The spacing for this rowgroup.
     pub spacing: border_spacing::T,
 
+    /// The direction of the columns, propagated down from the table during the inline-size
+    /// assignment phase.
+    pub table_writing_mode: WritingMode,
+
     /// Information about the borders for each cell that we bubble up to our parent. This is only
     /// computed if `border-collapse` is `collapse`.
     pub preliminary_collapsed_borders: CollapsedBordersForRow,
@@ -61,6 +65,7 @@ impl Encodable for TableRowGroupFlow {
 impl TableRowGroupFlow {
     pub fn from_node_and_fragment(node: &ThreadSafeLayoutNode, fragment: Fragment)
                                   -> TableRowGroupFlow {
+        let writing_mode = fragment.style().writing_mode;
         TableRowGroupFlow {
             block_flow: BlockFlow::from_node_and_fragment(node, fragment, None),
             column_intrinsic_inline_sizes: Vec::new(),
@@ -69,6 +74,7 @@ impl TableRowGroupFlow {
                 horizontal: Au(0),
                 vertical: Au(0),
             },
+            table_writing_mode: writing_mode,
             preliminary_collapsed_borders: CollapsedBordersForRow::new(),
             collapsed_inline_direction_border_widths_for_table: Vec::new(),
             collapsed_block_direction_border_widths_for_table: Vec::new(),
@@ -161,6 +167,7 @@ impl Flow for TableRowGroupFlow {
 
         let column_computed_inline_sizes = &self.column_computed_inline_sizes;
         let border_spacing = self.spacing;
+        let table_writing_mode = self.table_writing_mode;
         let collapsed_inline_direction_border_widths_for_table =
             &self.collapsed_inline_direction_border_widths_for_table;
         let mut collapsed_block_direction_border_widths_for_table =
@@ -170,19 +177,16 @@ impl Flow for TableRowGroupFlow {
                                                                    inline_end_content_edge,
                                                                    content_inline_size,
                                                                    |child_flow,
-                                                                    child_index,
+                                                                    _child_index,
-                                                                    content_inline_size,
+                                                                    _content_inline_size,
-                                                                    writing_mode,
+                                                                    _writing_mode,
-                                                                    inline_start_margin_edge| {
+                                                                    _inline_start_margin_edge,
+                                                                    _inline_end_margin_edge| {
             table_row::propagate_column_inline_sizes_to_child(
                 child_flow,
-                child_index,
+                table_writing_mode,
-                content_inline_size,
-                writing_mode,
                 column_computed_inline_sizes,
-                &border_spacing,
+                &border_spacing);
-                &None,
-                inline_start_margin_edge);
 
             if border_collapse == border_collapse::T::collapse {
                 let child_table_row = child_flow.as_table_row();

components/layout/table_wrapper.rs

@@ -341,7 +341,7 @@ impl Flow for TableWrapperFlow {
                                                                 inline_start_content_edge,
                                                                 inline_end_content_edge,
                                                                 content_inline_size,
-                                                                |_, _, _, _, _| {})
+                                                                |_, _, _, _, _, _| {})
             }
             Some(ref assigned_column_inline_sizes) => {
                 self.block_flow
@@ -350,19 +350,16 @@ impl Flow for TableWrapperFlow {
                                                                 inline_end_content_edge,
                                                                 content_inline_size,
                                                                 |child_flow,
-                                                                 child_index,
+                                                                 _child_index,
-                                                                 content_inline_size,
+                                                                 _content_inline_size,
                                                                  writing_mode,
-                                                                 inline_start_margin_edge| {
+                                                                 _inline_start_margin_edge,
+                                                                 _inline_end_margin_edge| {
                     table_row::propagate_column_inline_sizes_to_child(
                         child_flow,
-                        child_index,
-                        content_inline_size,
                         writing_mode,
                         assigned_column_inline_sizes,
-                        &border_spacing,
+                        &border_spacing);
-                        &None,
-                        inline_start_margin_edge)
                 })
             }
         }

tests/ref/basic.list

@@ -277,6 +277,7 @@ experimental == rtl_simple.html rtl_simple_ref.html
 == table_padding_a.html table_padding_ref.html
 == table_percentage_capping_a.html table_percentage_capping_ref.html
 == table_percentage_width_a.html table_percentage_width_ref.html
+experimental == table_row_direction_a.html table_row_direction_ref.html
 == text_align_complex_a.html text_align_complex_ref.html
 == text_align_justify_a.html text_align_justify_ref.html
 experimental == text_align_rtl.html text_align_rtl_ref.html

tests/ref/table_row_direction_a.html

@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="UTF-8">
+    <link rel="stylesheet" type="text/css" href="css/ahem.css">
+    <style>
+      * { margin: 0; padding: 0; border-spacing: 0; }
+      tr { direction: rtl; }
+      .r { color: red; }
+      .o { color: orange; }
+      .y { color: yellow; }
+    </style>
+  </head>
+  <body>
+    <table>
+      <tr>
+        <td class="r">R</td>
+        <td class="o">O</td>
+        <td class="y">Y</td>
+      </tr>
+    </table>
+  </body>
+</html>

tests/ref/table_row_direction_ref.html

@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="UTF-8">
+    <link rel="stylesheet" type="text/css" href="css/ahem.css">
+    <style>
+      * { margin: 0; padding: 0; border-spacing: 0; }
+      .r { color: red; }
+      .o { color: orange; }
+      .y { color: yellow; }
+    </style>
+  </head>
+  <body>
+    <table>
+      <tr>
+        <td class="r">R</td>
+        <td class="o">O</td>
+        <td class="y">Y</td>
+      </tr>
+    </table>
+  </body>
+</html>