From 4dc9d8b1c5a4e68eee09af547ae7069455c9abe9 Mon Sep 17 00:00:00 2001 From: Matt Brubeck Date: Mon, 29 Feb 2016 13:05:46 -0800 Subject: [PATCH] Fix confusing `push_or_mutate` API This fixes a bug when recalculating border collapsing for an existing table now. The bug was caused by using `push_or_mutate` which has no effect if there is already a value at the specified index. The fix switches incorrect `push_or_mutate` calls to use `push_or_set` instead. It also renames `push_or_mutate` to `get_mut_or_push` which I think is a less-confusing name for this method. --- components/layout/table.rs | 13 +++++++------ components/layout/table_row.rs | 7 ++++--- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/components/layout/table.rs b/components/layout/table.rs index 07b606733e8..15decaed28a 100644 --- a/components/layout/table.rs +++ b/components/layout/table.rs @@ -658,21 +658,22 @@ pub struct ColumnComputedInlineSize { } pub trait VecExt { - fn push_or_set(&mut self, index: usize, value: T); - fn push_or_mutate(&mut self, index: usize, zero: T) -> &mut T; + fn push_or_set(&mut self, index: usize, value: T) -> &mut T; + fn get_mut_or_push(&mut self, index: usize, zero: T) -> &mut T; } impl VecExt for Vec { - fn push_or_set(&mut self, index: usize, value: T) { + fn push_or_set(&mut self, index: usize, value: T) -> &mut T { if index < self.len() { self[index] = value } else { debug_assert!(index == self.len()); self.push(value) } + &mut self[index] } - fn push_or_mutate(&mut self, index: usize, zero: T) -> &mut T { + fn get_mut_or_push(&mut self, index: usize, zero: T) -> &mut T { if index >= self.len() { debug_assert!(index == self.len()); self.push(zero) @@ -697,7 +698,7 @@ fn perform_border_collapse_for_row(child_table_row: &mut TableRowFlow, .enumerate() { child_table_row.final_collapsed_borders.inline.push_or_set(i, *this_inline_border); - let inline_spacing = inline_spacing.push_or_mutate(i, Au(0)); + let inline_spacing = inline_spacing.get_mut_or_push(i, Au(0)); *inline_spacing = cmp::max(*inline_spacing, this_inline_border.width) } @@ -733,7 +734,7 @@ fn perform_border_collapse_for_row(child_table_row: &mut TableRowFlow, .block_end .iter() .enumerate() { - let next_block = next_block.push_or_mutate(i, *this_block_border); + let next_block = next_block.push_or_set(i, *this_block_border); match next_block_borders { NextBlockCollapsedBorders::FromNextRow(next_block_borders) => { if next_block_borders.len() > i { diff --git a/components/layout/table_row.rs b/components/layout/table_row.rs index ceeed531a8e..a7da3b19749 100644 --- a/components/layout/table_row.rs +++ b/components/layout/table_row.rs @@ -823,7 +823,8 @@ fn perform_inline_direction_border_collapse_for_row( child_table_cell: &mut TableCellFlow, iterator: &mut Peekable>, preliminary_collapsed_borders: &mut CollapsedBordersForRow) { - let inline_collapsed_border = preliminary_collapsed_borders.inline.push_or_mutate( + println!(" perform_inline_direction_border_collapse_for_row"); + let inline_collapsed_border = preliminary_collapsed_borders.inline.push_or_set( child_index + 1, CollapsedBorder::inline_end(&*child_table_cell.block_flow.fragment.style, CollapsedBorderProvenance::FromPreviousTableCell)); @@ -838,9 +839,9 @@ fn perform_inline_direction_border_collapse_for_row( let block_start_border = CollapsedBorder::block_start(&*child_table_cell.block_flow.fragment.style, CollapsedBorderProvenance::FromNextTableCell); - preliminary_collapsed_borders.block_start.push_or_mutate(child_index, block_start_border); + preliminary_collapsed_borders.block_start.push_or_set(child_index, block_start_border); let block_end_border = CollapsedBorder::block_end(&*child_table_cell.block_flow.fragment.style, CollapsedBorderProvenance::FromPreviousTableCell); - preliminary_collapsed_borders.block_end.push_or_mutate(child_index, block_end_border); + preliminary_collapsed_borders.block_end.push_or_set(child_index, block_end_border); }