Fix border collapsing at the end of a table-row-group

This fixes the border-end calculation for table rows whose borders are
collapsed with rows in different rowgroups.  The border collapsing code now
uses an iterator that yields all the rows as a flat sequence, regardless of
how they are grouped in rowgroups.  It gets rid of
`TableRowGroupFlow::preliminary_collapsed_borders` which was never correct.
(It was read but never written.)

This may fix #8120 but I'm not 100% certain. (I haven't managed to reproduce
the intermittent failure locally, and my reduced test case still fails but in
a different way.)
This commit is contained in:
Matt Brubeck 2016-02-29 14:33:41 -08:00
parent 4dc9d8b1c5
commit 6c684a5ac7
7 changed files with 181 additions and 139 deletions

View file

@ -12,8 +12,9 @@ use block::{ISizeConstraintInput, ISizeConstraintSolution};
use context::LayoutContext;
use display_list_builder::{BlockFlowDisplayListBuilding, BorderPaintingMode};
use euclid::Point2D;
use flow::{IMPACTED_BY_RIGHT_FLOATS, ImmutableFlowUtils, OpaqueFlow};
use flow::{BaseFlow, IMPACTED_BY_RIGHT_FLOATS, ImmutableFlowUtils, OpaqueFlow};
use flow::{self, EarlyAbsolutePositionInfo, Flow, FlowClass, IMPACTED_BY_LEFT_FLOATS};
use flow_list::MutFlowListIterator;
use fragment::{Fragment, FragmentBorderBoxIterator, Overflow};
use gfx::display_list::DisplayList;
use incremental::{REFLOW, REFLOW_OUT_OF_FLOW};
@ -133,7 +134,7 @@ impl TableFlow {
/// Updates the minimum and preferred inline-size calculation for a single row. This is
/// factored out into a separate function because we process children of rowgroups too.
fn update_column_inline_sizes_for_row(child: &mut Flow,
fn update_column_inline_sizes_for_row(row: &mut TableRowFlow,
column_inline_sizes: &mut Vec<ColumnIntrinsicInlineSize>,
computation: &mut IntrinsicISizesContribution,
first_row: bool,
@ -142,8 +143,6 @@ impl TableFlow {
// not defined in the column group.
//
// FIXME: Need to read inline-sizes from either table-header-group OR the first table-row.
debug_assert!(child.is_table_row());
let row = child.as_table_row();
match table_layout {
TableLayout::Fixed => {
// Fixed table layout only looks at the first row.
@ -230,6 +229,28 @@ impl Flow for TableFlow {
// Don't use `compute_intrinsic_inline_sizes` here because that will count padding as
// part of the table, which we don't want to do—it belongs to the table wrapper instead.
// Get column inline sizes from colgroups
for kid in self.block_flow.base.child_iter().filter(|kid| kid.is_table_colgroup()) {
for specified_inline_size in &kid.as_mut_table_colgroup().inline_sizes {
self.column_intrinsic_inline_sizes.push(ColumnIntrinsicInlineSize {
minimum_length: match *specified_inline_size {
LengthOrPercentageOrAuto::Auto |
LengthOrPercentageOrAuto::Calc(_) |
LengthOrPercentageOrAuto::Percentage(_) => Au(0),
LengthOrPercentageOrAuto::Length(length) => length,
},
percentage: match *specified_inline_size {
LengthOrPercentageOrAuto::Auto |
LengthOrPercentageOrAuto::Calc(_) |
LengthOrPercentageOrAuto::Length(_) => 0.0,
LengthOrPercentageOrAuto::Percentage(percentage) => percentage,
},
preferred: Au(0),
constrained: false,
})
}
}
self.collapsed_inline_direction_border_widths_for_table = Vec::new();
self.collapsed_block_direction_border_widths_for_table = vec![Au(0)];
@ -250,133 +271,47 @@ impl Flow for TableFlow {
};
let mut computation = IntrinsicISizesContribution::new();
let mut previous_collapsed_block_end_borders = if collapsing_borders {
let mut previous_collapsed_block_end_borders =
PreviousBlockCollapsedBorders::FromTable(CollapsedBorder::block_start(
&*self.block_flow.fragment.style,
CollapsedBorderProvenance::FromTable))
} else {
PreviousBlockCollapsedBorders::NotCollapsingBorders
};
CollapsedBorderProvenance::FromTable));
let mut first_row = true;
{
let mut iterator = self.block_flow.base.child_iter().peekable();
while let Some(kid) = iterator.next() {
if kid.is_table_colgroup() {
for specified_inline_size in &kid.as_mut_table_colgroup().inline_sizes {
self.column_intrinsic_inline_sizes.push(ColumnIntrinsicInlineSize {
minimum_length: match *specified_inline_size {
LengthOrPercentageOrAuto::Auto |
LengthOrPercentageOrAuto::Calc(_) |
LengthOrPercentageOrAuto::Percentage(_) => Au(0),
LengthOrPercentageOrAuto::Length(length) => length,
},
percentage: match *specified_inline_size {
LengthOrPercentageOrAuto::Auto |
LengthOrPercentageOrAuto::Calc(_) |
LengthOrPercentageOrAuto::Length(_) => 0.0,
LengthOrPercentageOrAuto::Percentage(percentage) => percentage,
},
preferred: Au(0),
constrained: false,
})
}
} else if kid.is_table_row() {
TableFlow::update_column_inline_sizes_for_row(
kid,
&mut self.column_intrinsic_inline_sizes,
&mut computation,
first_row,
self.table_layout);
if collapsing_borders {
let next_index_and_sibling = iterator.peek();
let next_collapsed_borders_in_block_direction =
match next_index_and_sibling {
Some(next_sibling) => {
if next_sibling.is_table_rowgroup() {
NextBlockCollapsedBorders::FromNextRow(
&next_sibling.as_table_rowgroup()
.preliminary_collapsed_borders
.block_start)
} else {
NextBlockCollapsedBorders::FromNextRow(
&next_sibling.as_table_row()
.preliminary_collapsed_borders
.block_start)
}
}
None => {
NextBlockCollapsedBorders::FromTable(
CollapsedBorder::block_end(&*self.block_flow.fragment.style,
CollapsedBorderProvenance::FromTable))
}
};
perform_border_collapse_for_row(
kid.as_mut_table_row(),
table_inline_collapsed_borders.as_ref().unwrap(),
previous_collapsed_block_end_borders,
next_collapsed_borders_in_block_direction,
&mut self.collapsed_inline_direction_border_widths_for_table,
&mut self.collapsed_block_direction_border_widths_for_table);
previous_collapsed_block_end_borders =
PreviousBlockCollapsedBorders::FromPreviousRow(
kid.as_table_row().final_collapsed_borders.block_end.clone())
}
first_row = false
} else if kid.is_table_rowgroup() {
let mut iterator = flow::mut_base(kid).child_iter().peekable();
while let Some(grandkid) = iterator.next() {
let grandkid_next_sibling = iterator.peek();
let next_collapsed_borders_in_block_direction = if collapsing_borders {
match grandkid_next_sibling {
Some(grandkid_next_sibling) => {
if grandkid_next_sibling.is_table_rowgroup() {
NextBlockCollapsedBorders::FromNextRow(
&grandkid_next_sibling.as_table_rowgroup()
.preliminary_collapsed_borders
.block_start)
} else {
NextBlockCollapsedBorders::FromNextRow(
&grandkid_next_sibling.as_table_row()
.preliminary_collapsed_borders
.block_start)
}
}
None => {
NextBlockCollapsedBorders::FromTable(
CollapsedBorder::block_end(
&*self.block_flow.fragment.style,
CollapsedBorderProvenance::FromTable))
}
let mut iterator = TableRowIterator::new(&mut self.block_flow.base).peekable();
while let Some(row) = iterator.next() {
TableFlow::update_column_inline_sizes_for_row(row,
&mut self.column_intrinsic_inline_sizes,
&mut computation,
first_row,
self.table_layout);
if collapsing_borders {
let next_index_and_sibling = iterator.peek();
let next_collapsed_borders_in_block_direction =
match next_index_and_sibling {
Some(next_sibling) => {
NextBlockCollapsedBorders::FromNextRow(
&next_sibling.as_table_row()
.preliminary_collapsed_borders
.block_start)
}
None => {
NextBlockCollapsedBorders::FromTable(
CollapsedBorder::block_end(&*self.block_flow.fragment.style,
CollapsedBorderProvenance::FromTable))
}
} else {
NextBlockCollapsedBorders::NotCollapsingBorders
};
TableFlow::update_column_inline_sizes_for_row(
grandkid,
&mut self.column_intrinsic_inline_sizes,
&mut computation,
first_row,
self.table_layout);
if collapsing_borders {
perform_border_collapse_for_row(
grandkid.as_mut_table_row(),
table_inline_collapsed_borders.as_ref().unwrap(),
previous_collapsed_block_end_borders,
next_collapsed_borders_in_block_direction,
&mut self.collapsed_inline_direction_border_widths_for_table,
&mut self.collapsed_block_direction_border_widths_for_table);
previous_collapsed_block_end_borders =
PreviousBlockCollapsedBorders::FromPreviousRow(
grandkid.as_table_row()
.final_collapsed_borders
.block_end
.clone())
}
first_row = false
}
perform_border_collapse_for_row(row,
table_inline_collapsed_borders.as_ref().unwrap(),
previous_collapsed_block_end_borders,
next_collapsed_borders_in_block_direction,
&mut self.collapsed_inline_direction_border_widths_for_table,
&mut self.collapsed_block_direction_border_widths_for_table);
previous_collapsed_block_end_borders =
PreviousBlockCollapsedBorders::FromPreviousRow(
row.final_collapsed_borders.block_end.clone())
}
first_row = false
}
}
@ -723,7 +658,6 @@ fn perform_border_collapse_for_row(child_table_row: &mut TableRowFlow,
child_table_row.final_collapsed_borders.block_start =
vec![collapsed_border; child_table_row.block_flow.base.children.len()]
}
PreviousBlockCollapsedBorders::NotCollapsingBorders => {}
}
// Compute block-end borders.
@ -744,7 +678,6 @@ fn perform_border_collapse_for_row(child_table_row: &mut TableRowFlow,
NextBlockCollapsedBorders::FromTable(ref next_block_borders) => {
next_block.combine(next_block_borders);
}
NextBlockCollapsedBorders::NotCollapsingBorders => {}
}
*block_spacing = cmp::max(*block_spacing, next_block.width)
}
@ -851,11 +784,52 @@ struct TableInlineCollapsedBorders {
enum PreviousBlockCollapsedBorders {
FromPreviousRow(Vec<CollapsedBorder>),
FromTable(CollapsedBorder),
NotCollapsingBorders,
}
enum NextBlockCollapsedBorders<'a> {
FromNextRow(&'a [CollapsedBorder]),
FromTable(CollapsedBorder),
NotCollapsingBorders,
}
/// Iterator over all the rows of a table
struct TableRowIterator<'a> {
kids: MutFlowListIterator<'a>,
grandkids: Option<MutFlowListIterator<'a>>,
}
impl<'a> TableRowIterator<'a> {
fn new(base: &'a mut BaseFlow) -> Self {
TableRowIterator {
kids: base.child_iter(),
grandkids: None,
}
}
}
impl<'a> Iterator for TableRowIterator<'a> {
type Item = &'a mut TableRowFlow;
#[inline]
fn next(&mut self) -> Option<Self::Item> {
// If we're inside a rowgroup, iterate through the rowgroup's children.
if let Some(ref mut grandkids) = self.grandkids {
if let Some(grandkid) = grandkids.next() {
return Some(grandkid.as_mut_table_row())
}
}
// Otherwise, iterate through the table's children.
self.grandkids = None;
match self.kids.next() {
Some(kid) => {
if kid.is_table_rowgroup() {
self.grandkids = Some(flow::mut_base(kid).child_iter());
self.next()
} else if kid.is_table_row() {
Some(kid.as_mut_table_row())
} else {
self.next() // Skip children that are not rows or rowgroups
}
}
None => None
}
}
}

View file

@ -21,7 +21,7 @@ use style::computed_values::{border_collapse, border_spacing};
use style::logical_geometry::{LogicalSize, WritingMode};
use style::properties::ComputedValues;
use table::{ColumnComputedInlineSize, ColumnIntrinsicInlineSize, InternalTable, TableLikeFlow};
use table_row::{self, CollapsedBordersForRow};
use table_row;
use util::print_tree::PrintTree;
/// A table formatting context.
@ -42,10 +42,6 @@ pub struct TableRowGroupFlow {
/// 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,
/// The final width of the borders in the inline direction for each cell, computed by the
/// entire table and pushed down into each row during inline size computation.
pub collapsed_inline_direction_border_widths_for_table: Vec<Au>,
@ -73,7 +69,6 @@ impl TableRowGroupFlow {
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(),
}