Skip to content

Commit

Permalink
auto merge of #5233 : mbrubeck/servo/rtl-position, r=SimonSapin
Browse files Browse the repository at this point in the history
This fixes a bug in finding the top left corner of an RTL block in physical coordinates.  (The old code used the `start` point of the `position` rect, which is not always the top left.)

It also fixes the setting of `position.start.i` in certain mixed LTR/RTL cases.

There is still a bug related to `position.size` for RTL blocks with margins.  See the FIXME comments for details.

r? @pcwalton or @SimonSapin
  • Loading branch information
bors-servo committed Mar 17, 2015
2 parents 7bd6cb0 + 531bcb1 commit b255b49
Show file tree
Hide file tree
Showing 9 changed files with 80 additions and 7 deletions.
30 changes: 23 additions & 7 deletions components/layout/block.rs
Expand Up @@ -1237,6 +1237,7 @@ impl BlockFlow {
&mut self,
layout_context: &LayoutContext,
inline_start_content_edge: Au,
inline_end_content_edge: Au,
content_inline_size: Au,
table_info: Option<table::ChildInlineSizeInfo>) {
// Keep track of whether floats could impact each child.
Expand Down Expand Up @@ -1340,8 +1341,8 @@ impl BlockFlow {
inline_start_content_edge
} else {
// The kid's inline 'start' is at the parent's 'end'
inline_start_content_edge + content_inline_size
}
inline_end_content_edge
};
}
kid_base.block_container_inline_size = content_inline_size;
kid_base.block_container_writing_mode = containing_block_mode;
Expand All @@ -1352,7 +1353,7 @@ impl BlockFlow {
inline_start_content_edge
} else {
// The kid's inline 'start' is at the parent's 'end'
inline_start_content_edge + content_inline_size
inline_end_content_edge
}
}

Expand Down Expand Up @@ -1630,15 +1631,25 @@ impl Flow for BlockFlow {
self.base.flags.remove(IMPACTED_BY_RIGHT_FLOATS);
}
}

// Move in from the inline-start border edge.
let inline_start_content_edge = self.fragment.border_box.start.i +
self.fragment.border_padding.inline_start;

let padding_and_borders = self.fragment.border_padding.inline_start_end();

// Distance from the inline-end margin edge to the inline-end content edge.
let inline_end_content_edge =
self.base.block_container_inline_size -
self.fragment.margin.inline_end -
self.fragment.border_box.size.inline -
self.fragment.border_box.start.i -
padding_and_borders;

let content_inline_size = self.fragment.border_box.size.inline - padding_and_borders;

self.propagate_assigned_inline_size_to_children(layout_context,
inline_start_content_edge,
inline_end_content_edge,
content_inline_size,
None);
}
Expand Down Expand Up @@ -1820,9 +1831,11 @@ impl Flow for BlockFlow {
for kid in self.base.child_iter() {
if !flow::base(kid).flags.contains(IS_ABSOLUTELY_POSITIONED) {
let kid_base = flow::mut_base(kid);
kid_base.stacking_relative_position = origin_for_children +
kid_base.position.start.to_physical(kid_base.writing_mode,
container_size_for_children);
// FIXME (mbrubeck): `position.size` is inflated by the inline margin size, making
// this incorrect for RTL blocks (see `set_inline_size_constraint_solutions`).
let physical_position = kid_base.position.to_physical(kid_base.writing_mode,
container_size_for_children);
kid_base.stacking_relative_position = origin_for_children + physical_position.origin;
}

flow::mut_base(kid).absolute_position_info = absolute_position_info_for_children;
Expand Down Expand Up @@ -2109,6 +2122,9 @@ pub trait ISizeAndMarginsComputer {
// We also resize the block itself, to ensure that overflow is not calculated
// as the inline-size of our parent. We might be smaller and we might be larger if we
// overflow.
//
// FIXME (mbrubeck): The margin is included in position.size but not position.start, which
// throws off position.to_physical results (especially for RTL blocks).
flow::mut_base(block).position.size.inline = inline_size + extra_inline_size_from_margin;
}

Expand Down
2 changes: 2 additions & 0 deletions components/layout/table.rs
Expand Up @@ -300,6 +300,7 @@ impl Flow for TableFlow {
containing_block_inline_size);

let inline_start_content_edge = self.block_flow.fragment.border_padding.inline_start;
let inline_end_content_edge = self.block_flow.fragment.border_padding.inline_end;
let padding_and_borders = self.block_flow.fragment.border_padding.inline_start_end();
let spacing_per_cell = self.spacing();
let spacing = spacing_per_cell.horizontal *
Expand Down Expand Up @@ -352,6 +353,7 @@ impl Flow for TableFlow {
};
self.block_flow.propagate_assigned_inline_size_to_children(layout_context,
inline_start_content_edge,
inline_end_content_edge,
content_inline_size,
Some(info));
}
Expand Down
5 changes: 5 additions & 0 deletions components/layout/table_cell.rs
Expand Up @@ -128,12 +128,17 @@ impl Flow for TableCellFlow {
let inline_start_content_edge =
self.block_flow.fragment.border_box.start.i +
self.block_flow.fragment.border_padding.inline_start;
let inline_end_content_edge =
self.block_flow.base.block_container_inline_size -
self.block_flow.fragment.border_padding.inline_start_end() -
self.block_flow.fragment.border_box.size.inline;
let padding_and_borders = self.block_flow.fragment.border_padding.inline_start_end();
let content_inline_size =
self.block_flow.fragment.border_box.size.inline - padding_and_borders;

self.block_flow.propagate_assigned_inline_size_to_children(layout_context,
inline_start_content_edge,
inline_end_content_edge,
content_inline_size,
None);
}
Expand Down
2 changes: 2 additions & 0 deletions components/layout/table_row.rs
Expand Up @@ -238,6 +238,7 @@ impl Flow for TableRowFlow {
// FIXME: In case of border-collapse: collapse, inline_start_content_edge should be
// border_inline_start.
let inline_start_content_edge = Au(0);
let inline_end_content_edge = Au(0);

let inline_size_computer = InternalTable;
inline_size_computer.compute_used_inline_size(&mut self.block_flow,
Expand Down Expand Up @@ -285,6 +286,7 @@ impl Flow for TableRowFlow {
};
self.block_flow.propagate_assigned_inline_size_to_children(layout_context,
inline_start_content_edge,
inline_end_content_edge,
containing_block_inline_size,
Some(info));
}
Expand Down
2 changes: 2 additions & 0 deletions components/layout/table_rowgroup.rs
Expand Up @@ -102,6 +102,7 @@ impl Flow for TableRowGroupFlow {
// FIXME: In case of border-collapse: collapse, inline-start_content_edge should be
// the border width on the inline-start side.
let inline_start_content_edge = Au::new(0);
let inline_end_content_edge = Au::new(0);
let content_inline_size = containing_block_inline_size;

let inline_size_computer = InternalTable;
Expand All @@ -115,6 +116,7 @@ impl Flow for TableRowGroupFlow {
};
self.block_flow.propagate_assigned_inline_size_to_children(layout_context,
inline_start_content_edge,
inline_end_content_edge,
content_inline_size,
Some(info));
}
Expand Down
8 changes: 8 additions & 0 deletions components/layout/table_wrapper.rs
Expand Up @@ -293,6 +293,12 @@ impl Flow for TableWrapperFlow {
let inline_start_content_edge = self.block_flow.fragment.border_box.start.i;
let content_inline_size = self.block_flow.fragment.border_box.size.inline;

// FIXME (mbrubeck): Test mixed RTL/LTR table layout, make sure this is right.
let inline_end_content_edge = self.block_flow.base.block_container_inline_size -
self.block_flow.fragment.margin.inline_end -
content_inline_size -
inline_start_content_edge;

// In case of fixed layout, column inline-sizes are calculated in table flow.
let assigned_column_inline_sizes = match self.table_layout {
TableLayout::Fixed => None,
Expand All @@ -310,6 +316,7 @@ impl Flow for TableWrapperFlow {
self.block_flow.propagate_assigned_inline_size_to_children(
layout_context,
inline_start_content_edge,
inline_end_content_edge,
content_inline_size,
None)
}
Expand All @@ -321,6 +328,7 @@ impl Flow for TableWrapperFlow {
self.block_flow
.propagate_assigned_inline_size_to_children(layout_context,
inline_start_content_edge,
inline_end_content_edge,
content_inline_size,
Some(info));
}
Expand Down
1 change: 1 addition & 0 deletions tests/ref/basic.list
Expand Up @@ -236,6 +236,7 @@ experimental != overconstrained_block.html overconstrained_block_ref.html
== root_height_a.html root_height_b.html
== root_margin_collapse_a.html root_margin_collapse_b.html
== root_pseudo_a.html root_pseudo_b.html
experimental == rtl_body.html rtl_body_ref.html
experimental == rtl_simple.html rtl_simple_ref.html
== setattribute_id_restyle_a.html setattribute_id_restyle_b.html
== stacking_context_overflow_a.html stacking_context_overflow_ref.html
Expand Down
17 changes: 17 additions & 0 deletions tests/ref/rtl_body.html
@@ -0,0 +1,17 @@
<head>
<style>
body {
direction: rtl;
margin: 0;
}
#outer {
background: green;

width: 300px;
height: 100px;
}
</style>
</head>
<body>
<div id="outer"></div>
</body>
20 changes: 20 additions & 0 deletions tests/ref/rtl_body_ref.html
@@ -0,0 +1,20 @@
<head>
<style>
body {
margin: 0;
}
#outer {
background: green;

width: 300px;
height: 100px;

position:absolute;
top: 0;
right: 0;
}
</style>
</head>
<body>
<div id="outer"></div>
</body>

0 comments on commit b255b49

Please sign in to comment.