Skip to content

Commit

Permalink
Do not compound iframe margins when positioning
Browse files Browse the repository at this point in the history
Instead of taking margin size into account twice when positioning
layers, just rely on the absolute position calculated during display
list construction.
  • Loading branch information
mrobinson committed Oct 14, 2014
1 parent 6e3c776 commit e53093e
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 12 deletions.
21 changes: 9 additions & 12 deletions components/layout/fragment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1236,7 +1236,7 @@ impl Fragment {
match self.specific {
IframeFragment(ref iframe_fragment) => {
self.finalize_position_and_size_of_iframe(iframe_fragment,
flow_origin,
absolute_fragment_bounds.origin,
layout_context)
}
_ => {}
Expand Down Expand Up @@ -1741,23 +1741,20 @@ impl Fragment {
iframe_fragment: &IframeFragmentInfo,
offset: Point2D<Au>,
layout_context: &LayoutContext) {
let mbp = (self.margin + self.border_padding).to_physical(self.style.writing_mode);
let border_padding = (self.border_padding).to_physical(self.style.writing_mode);
let content_size = self.content_box().size.to_physical(self.style.writing_mode);

let left = offset.x + mbp.left;
let top = offset.y + mbp.top;
let width = content_size.width;
let height = content_size.height;
let origin = Point2D(geometry::to_frac_px(left) as f32, geometry::to_frac_px(top) as f32);
let size = Size2D(geometry::to_frac_px(width) as f32, geometry::to_frac_px(height) as f32);
let rect = Rect(origin, size);
let iframe_rect = Rect(Point2D(geometry::to_frac_px(offset.x + border_padding.left) as f32,
geometry::to_frac_px(offset.y + border_padding.top) as f32),
Size2D(geometry::to_frac_px(content_size.width) as f32,
geometry::to_frac_px(content_size.height) as f32));

debug!("finalizing position and size of iframe for {:?},{:?}",
iframe_fragment.pipeline_id,
iframe_fragment.subpage_id);
let msg = FrameRectMsg(iframe_fragment.pipeline_id, iframe_fragment.subpage_id, rect);
let ConstellationChan(ref chan) = layout_context.shared.constellation_chan;
chan.send(msg)
chan.send(FrameRectMsg(iframe_fragment.pipeline_id,
iframe_fragment.subpage_id,
iframe_rect));
}

/// Returns true if and only if this is the *primary fragment* for the fragment's style object
Expand Down
1 change: 1 addition & 0 deletions tests/ref/basic.list
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ fragment=top != ../html/acid2.html acid2_ref.html
== iframe/simple.html iframe/simple_ref.html
== iframe/multiple_external.html iframe/multiple_external_ref.html
== iframe/overflow.html iframe/overflow_ref.html
== iframe/positioning_margin.html iframe/positioning_margin_ref.html

== floated_generated_content_a.html floated_generated_content_b.html
== inline_block_margin_a.html inline_block_margin_ref.html
Expand Down
8 changes: 8 additions & 0 deletions tests/ref/iframe/positioning_margin.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<html>
<body style="margin: 0px;">
<iframe src="data:text/html,%3Chtml%3E%3Cbody%20style%3D%22margin%3A%200px%3B%22%3E%3Cdiv%20style%3D%22background%3A%20green%3B%20height%3A%20100px%3B%20width%3A%20100px%3B%22%3E%3C%2Fdiv%3E%3C%2Fbody%3E%3C%2Fhtml%3E"
style="display: block; border: 0px; width: 100px; height: 100px; margin-top: 100px;">
</iframe>
</body>
</html>

5 changes: 5 additions & 0 deletions tests/ref/iframe/positioning_margin_ref.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<html>
<body>
<div style="position: absolute; width: 100px; height: 100px; background: green; top: 100px; left: 0px;"></div>
</body>
</html>

13 comments on commit e53093e

@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@e53093e

@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/iframe-margin = e53093e 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/iframe-margin = e53093e merged ok, testing candidate = 10d7afa

@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.

saw approval from pcwalton
at mrobinson@e53093e

@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/iframe-margin = e53093e 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/iframe-margin = e53093e merged ok, testing candidate = f6a7b1b

@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.

saw approval from pcwalton
at mrobinson@e53093e

@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/iframe-margin = e53093e 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/iframe-margin = e53093e merged ok, testing candidate = 834df4e

@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 = 834df4e

Please sign in to comment.