Skip to content

Commit

Permalink
Include leading margin in inline block size
Browse files Browse the repository at this point in the history
According to the documentation for Fragment::position, the inline axis
should include margin size, so we include it for blocks. Also fix
place_float which assumed that it was not included and
assign_inline_sizes which overrode the size set in
set_inline_size_constraint_solutions.

Typically this issue was hidden by large tile sizes, but fitted tiles
makes it more common.
  • Loading branch information
mrobinson committed Oct 13, 2014
1 parent c87f34f commit 4c453ac
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 9 deletions.
19 changes: 10 additions & 9 deletions components/layout/block.rs
Expand Up @@ -1041,8 +1041,7 @@ impl BlockFlow {
let info = PlacementInfo {
size: LogicalSize::new(
self.fragment.style.writing_mode,
self.base.position.size.inline + self.fragment.margin.inline_start_end() +
self.fragment.border_padding.inline_start_end(),
self.base.position.size.inline,
block_size + self.fragment.margin.block_start_end()),
ceiling: clearance + float_info.float_ceiling,
max_inline_size: float_info.containing_inline_size,
Expand Down Expand Up @@ -1619,10 +1618,6 @@ impl Flow for BlockFlow {
let padding_and_borders = self.fragment.border_padding.inline_start_end();
let content_inline_size = self.fragment.border_box.size.inline - padding_and_borders;

if self.is_float() {
self.base.position.size.inline = content_inline_size;
}

self.propagate_assigned_inline_size_to_children(inline_start_content_edge, content_inline_size, None);
}

Expand Down Expand Up @@ -1955,6 +1950,7 @@ pub trait ISizeAndMarginsComputer {
block: &mut BlockFlow,
solution: ISizeConstraintSolution) {
let inline_size;
let extra_inline_size_from_margin;
{
let fragment = block.fragment();
fragment.margin.inline_start = solution.margin_inline_start;
Expand All @@ -1965,14 +1961,19 @@ pub trait ISizeAndMarginsComputer {

// The associated fragment has the border box of this flow.
inline_size = solution.inline_size + fragment.border_padding.inline_start_end();
fragment.border_box.size.inline = inline_size
fragment.border_box.size.inline = inline_size;

// To calculate the total size of this block, we also need to account for any additional
// size contribution from positive margins. Negative margins means the block isn't made
// larger at all by the margin.
extra_inline_size_from_margin = max(Au(0), fragment.margin.inline_start) +
max(Au(0), fragment.margin.inline_end);
}

// 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.
let flow = flow::mut_base(block);
flow.position.size.inline = inline_size;
flow::mut_base(block).position.size.inline = inline_size + extra_inline_size_from_margin;
}

/// Set the x coordinate of the given flow if it is absolutely positioned.
Expand Down
1 change: 1 addition & 0 deletions tests/ref/basic.list
Expand Up @@ -62,6 +62,7 @@
== position_fixed_overflow_a.html position_fixed_overflow_b.html
== position_fixed_tile_edge.html position_fixed_tile_edge_ref.html
== position_fixed_tile_edge_2.html position_fixed_tile_edge_ref.html
== position_fixed_tile_edge_3.html position_fixed_tile_edge_ref.html
== position_relative_a.html position_relative_b.html
== position_relative_top_percentage_a.html position_relative_top_percentage_b.html
== background_none_a.html background_none_b.html
Expand Down
11 changes: 11 additions & 0 deletions tests/ref/position_fixed_tile_edge_3.html
@@ -0,0 +1,11 @@
<html>
<body>
<div style="position: absolute; top: 0px; left: 0px;">
<div style="position: absolute; background: green; margin-left: 512px; width: 20px; height: 20px;"></div>

<!-- This position:fixed sibling should force its sibling to be layerized. -->
<div style="position: fixed;"></div>
</div>
</body>
</html>

5 comments on commit 4c453ac

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from pcwalton
at mrobinson@4c453ac

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging mrobinson/servo/layer-sizing = 4c453ac into auto

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mrobinson/servo/layer-sizing = 4c453ac merged ok, testing candidate = 7f26c67

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fast-forwarding master to auto = 7f26c67

Please sign in to comment.