Auto merge of #5940 - pcwalton:inline-margins, r=glennw

Improves the Google SERPs.

r? @glennw

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/5940)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2015-05-07 19:10:31 -05:00
commit 0872ed922f
10 changed files with 190 additions and 57 deletions

View file

@ -334,7 +334,9 @@ impl<'a> FlowConstructor<'a> {
if child.is_table() {
let fragment = Fragment::new(child_node, SpecificFragmentInfo::TableWrapper);
let mut new_child =
FlowRef::new(box TableWrapperFlow::from_node_and_fragment(child_node, fragment, None));
FlowRef::new(box TableWrapperFlow::from_node_and_fragment(child_node,
fragment,
None));
new_child.add_new_child(child.clone());
child.finish();
*child = new_child
@ -376,8 +378,8 @@ impl<'a> FlowConstructor<'a> {
// Build a list of all the inline-block fragments before fragments is moved.
let mut inline_block_flows = vec!();
for f in fragments.iter() {
match f.specific {
for fragment in fragments.iter() {
match fragment.specific {
SpecificFragmentInfo::InlineBlock(ref info) => {
inline_block_flows.push(info.flow_ref.clone())
}
@ -511,13 +513,15 @@ impl<'a> FlowConstructor<'a> {
inline_fragment_accumulator.push_all(successor_fragments);
abs_descendants.push_descendants(kid_abs_descendants);
}
ConstructionResult::ConstructionItem(ConstructionItem::Whitespace(whitespace_node,
whitespace_style,
whitespace_damage)) => {
ConstructionResult::ConstructionItem(ConstructionItem::Whitespace(
whitespace_node,
mut whitespace_style,
whitespace_damage)) => {
// Add whitespace results. They will be stripped out later on when
// between block elements, and retained when between inline elements.
let fragment_info = SpecificFragmentInfo::UnscannedText(
UnscannedTextFragmentInfo::from_text(" ".to_owned()));
properties::modify_style_for_replaced_content(&mut whitespace_style);
let fragment = Fragment::from_opaque_node_and_style(whitespace_node,
whitespace_style,
whitespace_damage,
@ -729,11 +733,12 @@ impl<'a> FlowConstructor<'a> {
}
ConstructionResult::ConstructionItem(ConstructionItem::Whitespace(
whitespace_node,
whitespace_style,
mut whitespace_style,
whitespace_damage)) => {
// Instantiate the whitespace fragment.
let fragment_info = SpecificFragmentInfo::UnscannedText(
UnscannedTextFragmentInfo::from_text(" ".to_owned()));
properties::modify_style_for_replaced_content(&mut whitespace_style);
let fragment = Fragment::from_opaque_node_and_style(whitespace_node,
whitespace_style,
whitespace_damage,

View file

@ -21,7 +21,6 @@ use text;
use opaque_node::OpaqueNodeMethods;
use wrapper::{TLayoutNode, ThreadSafeLayoutNode};
use geom::num::Zero;
use geom::{Point2D, Rect, Size2D};
use gfx::display_list::{BLUR_INFLATION_FACTOR, OpaqueNode};
use gfx::text::glyph::CharIndex;
@ -42,7 +41,7 @@ use style::computed_values::content::ContentItem;
use style::computed_values::{border_collapse, clear, mix_blend_mode, overflow_wrap, position};
use style::computed_values::{text_align, text_decoration, white_space, word_break};
use style::node::{TElement, TNode};
use style::properties::{ComputedValues, cascade_anonymous, make_border};
use style::properties::{self, ComputedValues, cascade_anonymous};
use style::values::computed::{LengthOrPercentage, LengthOrPercentageOrAuto};
use style::values::computed::{LengthOrPercentageOrNone};
use text::TextRunScanner;
@ -855,18 +854,7 @@ impl Fragment {
self.inline_context = Some(InlineFragmentContext::new());
}
if !first_frag || !last_frag {
// Set the border width to zero and the border style to none on
// border sides that are not the outermost for a node container.
// Because with multiple inline fragments they don't have interior
// borders separating each other.
let mut border_width = node_info.style.logical_border_width();
if !last_frag {
border_width.set_right(node_info.style.writing_mode, Zero::zero());
}
if !first_frag {
border_width.set_left(node_info.style.writing_mode, Zero::zero());
}
node_info.style = Arc::new(make_border(&*node_info.style, border_width))
properties::modify_style_for_inline_sides(&mut node_info.style, first_frag, last_frag)
};
self.inline_context.as_mut().unwrap().nodes.push(node_info);
}
@ -1018,21 +1006,36 @@ impl Fragment {
/// (for example, via constraint solving for blocks).
pub fn compute_inline_direction_margins(&mut self, containing_block_inline_size: Au) {
match self.specific {
SpecificFragmentInfo::InlineBlock(_) |
SpecificFragmentInfo::Table |
SpecificFragmentInfo::TableCell |
SpecificFragmentInfo::TableRow |
SpecificFragmentInfo::TableColumn(_) => {
self.margin.inline_start = Au(0);
self.margin.inline_end = Au(0)
self.margin.inline_end = Au(0);
return
}
_ => {
let margin = self.style().logical_margin();
self.margin.inline_start =
MaybeAuto::from_style(margin.inline_start, containing_block_inline_size)
.specified_or_zero();
self.margin.inline_end =
MaybeAuto::from_style(margin.inline_end, containing_block_inline_size)
.specified_or_zero();
_ => {}
}
let margin = self.style().logical_margin();
self.margin.inline_start =
MaybeAuto::from_style(margin.inline_start,
containing_block_inline_size).specified_or_zero();
self.margin.inline_end =
MaybeAuto::from_style(margin.inline_end,
containing_block_inline_size).specified_or_zero();
if let Some(ref inline_context) = self.inline_context {
for node in inline_context.nodes.iter() {
let margin = node.style.logical_margin();
self.margin.inline_start = self.margin.inline_start +
MaybeAuto::from_style(margin.inline_start,
containing_block_inline_size).specified_or_zero();
self.margin.inline_end = self.margin.inline_end +
MaybeAuto::from_style(margin.inline_end,
containing_block_inline_size).specified_or_zero();
}
}
}
@ -2027,6 +2030,11 @@ impl Fragment {
pub fn inline_styles<'a>(&'a self) -> InlineStyleIterator<'a> {
InlineStyleIterator::new(self)
}
/// Returns the inline-size of this fragment's margin box.
pub fn margin_box_inline_size(&self) -> Au {
self.border_box.size.inline + self.margin.inline_start_end()
}
}
impl fmt::Debug for Fragment {

View file

@ -378,13 +378,15 @@ impl LineBreaker {
let mut range = &mut scanned_text_fragment_info.range;
strip_trailing_whitespace_if_necessary(&**scanned_text_fragment_info.run, range);
let old_fragment_inline_size = fragment.border_box.size.inline;
let old_fragment_inline_size = fragment.border_box.size.inline +
fragment.margin.inline_start_end();
scanned_text_fragment_info.content_size.inline =
scanned_text_fragment_info.run.metrics_for_range(range).advance_width;
fragment.border_box.size.inline = scanned_text_fragment_info.content_size.inline +
fragment.border_padding.inline_start_end();
self.pending_line.bounds.size.inline = self.pending_line.bounds.size.inline -
(old_fragment_inline_size - fragment.border_box.size.inline)
(old_fragment_inline_size - (fragment.border_box.size.inline +
fragment.margin.inline_start_end()))
}
}
@ -420,7 +422,7 @@ impl LineBreaker {
let placement_inline_size = if first_fragment.can_split() {
Au(0)
} else {
first_fragment.border_box.size.inline + self.indentation_for_pending_fragment()
first_fragment.margin_box_inline_size() + self.indentation_for_pending_fragment()
};
// Try to place the fragment between floats.
@ -434,9 +436,9 @@ impl LineBreaker {
});
// Simple case: if the fragment fits, then we can stop here.
if line_bounds.size.inline > first_fragment.border_box.size.inline {
if line_bounds.size.inline > first_fragment.margin_box_inline_size() {
debug!("LineBreaker: fragment fits on line {}", self.lines.len());
return (line_bounds, first_fragment.border_box.size.inline);
return (line_bounds, first_fragment.margin_box_inline_size());
}
// If not, but we can't split the fragment, then we'll place the line here and it will
@ -445,7 +447,7 @@ impl LineBreaker {
debug!("LineBreaker: line doesn't fit, but is unsplittable");
}
(line_bounds, first_fragment.border_box.size.inline)
(line_bounds, first_fragment.margin_box_inline_size())
}
/// Performs float collision avoidance. This is called when adding a fragment is going to
@ -545,7 +547,7 @@ impl LineBreaker {
// it doesn't fit.
let indentation = self.indentation_for_pending_fragment();
let new_inline_size = self.pending_line.bounds.size.inline +
fragment.border_box.size.inline + indentation;
fragment.margin_box_inline_size() + indentation;
if new_inline_size <= green_zone.inline {
debug!("LineBreaker: fragment fits without splitting");
self.push_fragment_to_line(layout_context, fragment, line_flush_mode);
@ -632,7 +634,7 @@ impl LineBreaker {
fragment.style().get_box().overflow_x) {
(text_overflow::T::clip, _) | (_, overflow_x::T::visible) => {}
(text_overflow::T::ellipsis, _) => {
need_ellipsis = fragment.border_box.size.inline > available_inline_size;
need_ellipsis = fragment.margin_box_inline_size() > available_inline_size;
}
}
@ -642,7 +644,7 @@ impl LineBreaker {
let ellipsis = fragment.transform_into_ellipsis(layout_context);
if let Some(truncation_info) =
fragment.truncate_to_inline_size(available_inline_size -
ellipsis.border_box.size.inline) {
ellipsis.margin_box_inline_size()) {
let fragment = fragment.transform_with_split_info(&truncation_info.split,
truncation_info.text_run);
self.push_fragment_to_line_ignoring_text_overflow(fragment, layout_context);
@ -663,7 +665,7 @@ impl LineBreaker {
let indentation = self.indentation_for_pending_fragment();
self.pending_line.range.extend_by(FragmentIndex(1));
self.pending_line.bounds.size.inline = self.pending_line.bounds.size.inline +
fragment.border_box.size.inline +
fragment.margin_box_inline_size() +
indentation;
self.pending_line.inline_metrics =
self.new_inline_metrics_for_line(&fragment, layout_context);
@ -928,14 +930,16 @@ impl InlineFlow {
for fragment_index in line.range.each_index() {
let fragment = fragments.get_mut(fragment_index.to_usize());
let size = fragment.border_box.size;
inline_start_position_for_fragment = inline_start_position_for_fragment +
fragment.margin.inline_start;
fragment.border_box = LogicalRect::new(fragment.style.writing_mode,
inline_start_position_for_fragment,
fragment.border_box.start.b,
size.inline,
size.block);
fragment.border_box.size.inline,
fragment.border_box.size.block);
fragment.update_late_computed_inline_position_if_necessary();
inline_start_position_for_fragment = inline_start_position_for_fragment + size.inline;
inline_start_position_for_fragment = inline_start_position_for_fragment +
fragment.border_box.size.inline + fragment.margin.inline_end;
}
}

View file

@ -15,12 +15,11 @@ use std::hash::{Hash, Hasher};
use std::sync::Arc;
use util::fnv::FnvHasher;
use util::logical_geometry::{WritingMode, LogicalMargin};
use util::logical_geometry::{LogicalMargin, PhysicalSide, WritingMode};
use util::geometry::Au;
use url::Url;
use cssparser::{Parser, Color, RGBA, AtRuleParser, DeclarationParser,
DeclarationListParser, parse_important, ToCss};
use geom::num::Zero;
use geom::SideOffsets2D;
use geom::size::Size2D;
@ -5723,21 +5722,78 @@ pub fn modify_style_for_replaced_content(style: &mut Arc<ComputedValues>) {
style.box_.make_unique().vertical_align =
longhands::vertical_align::computed_value::T::baseline
}
// Reset margins.
if style.margin.margin_top != computed::LengthOrPercentageOrAuto::Length(Au(0)) ||
style.margin.margin_left != computed::LengthOrPercentageOrAuto::Length(Au(0)) ||
style.margin.margin_bottom != computed::LengthOrPercentageOrAuto::Length(Au(0)) ||
style.margin.margin_right != computed::LengthOrPercentageOrAuto::Length(Au(0)) {
let mut style = style.make_unique();
let margin = style.margin.make_unique();
margin.margin_top = computed::LengthOrPercentageOrAuto::Length(Au(0));
margin.margin_left = computed::LengthOrPercentageOrAuto::Length(Au(0));
margin.margin_bottom = computed::LengthOrPercentageOrAuto::Length(Au(0));
margin.margin_right = computed::LengthOrPercentageOrAuto::Length(Au(0));
}
}
/// Sets `border_${side}_width` to the passed in values.
/// If `border_${side}_width` == 0 also sets `border_${side}_style` = none.
/// Adjusts borders, padding, and margins as appropriate to account for a fragment's status as the
/// first or last fragment within the range of an element.
///
/// Specifically, this function sets border/padding/margin widths to zero on the sides for which
/// the fragment is not outermost.
#[inline]
pub fn make_border(style: &ComputedValues, border_width: LogicalMargin<Au>) -> ComputedValues {
let mut style = (*style).clone();
let physical_border = LogicalMargin::to_physical(&border_width, style.writing_mode);
% for side in ["top", "right", "bottom", "left"]:
style.border.make_unique().border_${side}_width = physical_border.${side};
if physical_border.${side} == Zero::zero() {
style.border.make_unique().border_${side}_style = BorderStyle::none;
pub fn modify_style_for_inline_sides(style: &mut Arc<ComputedValues>,
is_first_fragment_of_element: bool,
is_last_fragment_of_element: bool) {
fn modify_side(style: &mut Arc<ComputedValues>, side: PhysicalSide) {
let mut style = style.make_unique();
let border = style.border.make_unique();
match side {
PhysicalSide::Left => {
border.border_left_width = Au(0);
border.border_left_style = BorderStyle::none;
style.padding.make_unique().padding_left =
computed::LengthOrPercentage::Length(Au(0));
style.margin.make_unique().margin_left =
computed::LengthOrPercentageOrAuto::Length(Au(0))
}
PhysicalSide::Right => {
border.border_right_width = Au(0);
border.border_right_style = BorderStyle::none;
style.padding.make_unique().padding_right =
computed::LengthOrPercentage::Length(Au(0));
style.margin.make_unique().margin_right =
computed::LengthOrPercentageOrAuto::Length(Au(0))
}
PhysicalSide::Bottom => {
border.border_bottom_width = Au(0);
border.border_bottom_style = BorderStyle::none;
style.padding.make_unique().padding_bottom =
computed::LengthOrPercentage::Length(Au(0));
style.margin.make_unique().margin_bottom =
computed::LengthOrPercentageOrAuto::Length(Au(0))
}
PhysicalSide::Top => {
border.border_top_width = Au(0);
border.border_top_style = BorderStyle::none;
style.padding.make_unique().padding_top =
computed::LengthOrPercentage::Length(Au(0));
style.margin.make_unique().margin_top =
computed::LengthOrPercentageOrAuto::Length(Au(0))
}
}
% endfor
style
}
if !is_first_fragment_of_element {
let side = style.writing_mode.inline_start_physical_side();
modify_side(style, side)
}
if !is_last_fragment_of_element {
let side = style.writing_mode.inline_end_physical_side();
modify_side(style, side)
}
}
pub fn is_supported_property(property: &str) -> bool {

View file

@ -151,6 +151,8 @@ flaky_cpu == append_style_a.html append_style_b.html
# inline_border_a.html inline_border_b.html
== inline_element_border_a.html inline_element_border_ref.html
== inline_hypothetical_box_a.html inline_hypothetical_box_ref.html
== inline_margin_multiple_fragments_a.html inline_margin_multiple_fragments_ref.html
== inline_margins_a.html inline_margins_ref.html
== inline_padding_a.html inline_padding_b.html
# inline_text_align_a.html inline_text_align_b.html
== inline_whitespace_a.html inline_whitespace_ref.html

View file

@ -0,0 +1,15 @@
<!DOCTYPE html>
<html>
<head>
<style>
span {
margin-left: 128px;
margin-right: 128px;
}
</style>
</head>
<body>
<span><img src=rust.png><img src=rust.png><img src=rust.png></span>
</body>
</html>

View file

@ -0,0 +1,18 @@
<!DOCTYPE html>
<html>
<head>
<style>
#first {
margin-left: 128px;
}
#last {
margin-right: 128px;
}
</style>
</head>
<body>
<span id=first><img src=rust.png></span><img src=rust.png><span id=last><img src=rust.png></span>
</body>
</html>

View file

@ -0,0 +1,14 @@
<!DOCTYPE html>
<html>
<head>
<link rel="stylesheet" type="text/css" href="css/ahem.css">
<style>
span {
margin-left: 100px;
margin-right: 100px;
}
</style>
</head>
<body>x<span>x</span>x</body>
</html>

View file

@ -0,0 +1,8 @@
<!DOCTYPE html>
<html>
<head>
<link rel="stylesheet" type="text/css" href="css/ahem.css">
</head>
<body>x x x</body>
</html>

View file

@ -0,0 +1,3 @@
[space.html]
type: reftest
expected: FAIL