layout: Take relative position offsets for inlines and inline-blocks

into account only once.

There were two bugs here: (1) relative position applied to
scanned/unscanned text fragments independently of the container element
that applied that relative position, causing double-counting; (2)
relative position applied to inline block fragments independently of the
wrapped block itself, causing double-counting.

This commit also removes the `cascade_anonymous` function and the
related `Fragment` constructor. They were unused, and their
functionality has been replaced by the `modify_style_for_*` series of
functions.

Closes #7067.
This commit is contained in:
Patrick Walton 2015-08-10 18:05:29 -07:00
parent 3ad49fc689
commit 8640cf5588
11 changed files with 102 additions and 88 deletions

View file

@ -227,6 +227,20 @@ impl InlineFragmentsAccumulator {
} }
} }
fn from_inline_node_and_style(node: &ThreadSafeLayoutNode, style: Arc<ComputedValues>)
-> InlineFragmentsAccumulator {
InlineFragmentsAccumulator {
fragments: IntermediateInlineFragments::new(),
enclosing_node: Some(InlineFragmentNodeInfo {
address: node.opaque(),
pseudo: node.get_pseudo_element_type().strip(),
style: style,
}),
bidi_control_chars: None,
restyle_damage: node.restyle_damage(),
}
}
fn push(&mut self, fragment: Fragment) { fn push(&mut self, fragment: Fragment) {
self.fragments.fragments.push_back(fragment) self.fragments.fragments.push_back(fragment)
} }
@ -686,7 +700,15 @@ impl<'a> FlowConstructor<'a> {
fragments: &mut IntermediateInlineFragments, fragments: &mut IntermediateInlineFragments,
node: &ThreadSafeLayoutNode, node: &ThreadSafeLayoutNode,
style: &Arc<ComputedValues>) { style: &Arc<ComputedValues>) {
for content_item in node.text_content().into_iter() { // Fast path: If there is no text content, return immediately.
let text_content = node.text_content();
if text_content.len() == 0 {
return
}
let mut style = (*style).clone();
properties::modify_style_for_text(&mut style);
for content_item in text_content.into_iter() {
let specific = match content_item { let specific = match content_item {
ContentItem::String(string) => { ContentItem::String(string) => {
let info = UnscannedTextFragmentInfo::from_text(string); let info = UnscannedTextFragmentInfo::from_text(string);
@ -910,11 +932,18 @@ impl<'a> FlowConstructor<'a> {
_ => unreachable!() _ => unreachable!()
}; };
let mut modified_style = (*node.style()).clone();
properties::modify_style_for_outer_inline_block_fragment(&mut modified_style);
let fragment_info = SpecificFragmentInfo::InlineBlock(InlineBlockFragmentInfo::new( let fragment_info = SpecificFragmentInfo::InlineBlock(InlineBlockFragmentInfo::new(
block_flow)); block_flow));
let fragment = Fragment::new(node, fragment_info); let fragment = Fragment::from_opaque_node_and_style(node.opaque(),
node.get_pseudo_element_type().strip(),
modified_style.clone(),
node.restyle_damage(),
fragment_info);
let mut fragment_accumulator = InlineFragmentsAccumulator::from_inline_node(node); let mut fragment_accumulator =
InlineFragmentsAccumulator::from_inline_node_and_style(node, modified_style);
fragment_accumulator.fragments.fragments.push_back(fragment); fragment_accumulator.fragments.fragments.push_back(fragment);
let construction_item = let construction_item =
@ -1317,6 +1346,12 @@ impl<'a> FlowConstructor<'a> {
inline_absolute_fragment.flow_ref inline_absolute_fragment.flow_ref
.repair_style_and_bubble_inline_sizes(&style); .repair_style_and_bubble_inline_sizes(&style);
} }
SpecificFragmentInfo::ScannedText(_) |
SpecificFragmentInfo::UnscannedText(_) => {
properties::modify_style_for_text(&mut style);
properties::modify_style_for_replaced_content(&mut style);
fragment.repair_style(&style);
}
_ => { _ => {
if node.is_replaced_content() { if node.is_replaced_content() {
properties::modify_style_for_replaced_content(&mut style); properties::modify_style_for_replaced_content(&mut style);

View file

@ -39,7 +39,7 @@ use style::computed_values::content::ContentItem;
use style::computed_values::{border_collapse, clear, mix_blend_mode, overflow_wrap, overflow_x}; use style::computed_values::{border_collapse, clear, mix_blend_mode, overflow_wrap, overflow_x};
use style::computed_values::{position, text_align, text_decoration, transform_style, white_space}; use style::computed_values::{position, text_align, text_decoration, transform_style, white_space};
use style::computed_values::{word_break, z_index}; use style::computed_values::{word_break, z_index};
use style::properties::{self, ComputedValues, cascade_anonymous}; use style::properties::{self, ComputedValues};
use style::values::computed::{LengthOrPercentage, LengthOrPercentageOrAuto}; use style::values::computed::{LengthOrPercentage, LengthOrPercentageOrAuto};
use style::values::computed::{LengthOrPercentageOrNone}; use style::values::computed::{LengthOrPercentageOrNone};
use text::TextRunScanner; use text::TextRunScanner;
@ -761,37 +761,6 @@ impl Fragment {
} }
} }
/// Constructs a new `Fragment` instance for an anonymous table object.
pub fn new_anonymous_from_specific_info(node: &ThreadSafeLayoutNode,
specific: SpecificFragmentInfo)
-> Fragment {
// CSS 2.1 § 17.2.1 This is for non-inherited properties on anonymous table fragments
// example:
//
// <div style="display: table">
// Foo
// </div>
//
// Anonymous table fragments, SpecificFragmentInfo::TableRow and
// SpecificFragmentInfo::TableCell, are generated around `Foo`, but they shouldn't inherit
// the border.
let node_style = cascade_anonymous(&**node.style());
let writing_mode = node_style.writing_mode;
Fragment {
node: node.opaque(),
style: Arc::new(node_style),
restyle_damage: node.restyle_damage(),
border_box: LogicalRect::zero(writing_mode),
border_padding: LogicalMargin::zero(writing_mode),
margin: LogicalMargin::zero(writing_mode),
specific: specific,
inline_context: None,
pseudo: node.get_pseudo_element_type().strip(),
debug_id: layout_debug::generate_unique_debug_id(),
}
}
/// Constructs a new `Fragment` instance from an opaque node. /// Constructs a new `Fragment` instance from an opaque node.
pub fn from_opaque_node_and_style(node: OpaqueNode, pub fn from_opaque_node_and_style(node: OpaqueNode,
pseudo: PseudoElementType<()>, pseudo: PseudoElementType<()>,

View file

@ -6385,43 +6385,6 @@ pub fn cascade(viewport_size: Size2D<Au>,
}, cacheable) }, cacheable)
} }
/// Equivalent to `cascade()` with an empty `applicable_declarations`
/// Performs the CSS cascade for an anonymous box.
///
/// * `parent_style`: Computed style of the element this anonymous box inherits from.
pub fn cascade_anonymous(parent_style: &ComputedValues) -> ComputedValues {
let initial_values = &*INITIAL_VALUES;
let mut result = ComputedValues {
% for style_struct in STYLE_STRUCTS:
${style_struct.ident}:
% if style_struct.inherited:
parent_style
% else:
initial_values
% endif
.${style_struct.ident}.clone(),
% endfor
shareable: false,
writing_mode: parent_style.writing_mode,
root_font_size: parent_style.root_font_size,
};
{
let border = Arc::make_unique(&mut result.border);
% for side in ["top", "right", "bottom", "left"]:
// Like calling to_computed_value, which wouldn't type check.
border.border_${side}_width = Au(0);
% endfor
}
{
// Initial value of outline-style is always none for anonymous box.
let outline = Arc::make_unique(&mut result.outline);
outline.outline_width = Au(0);
}
// None of the teaks on 'display' apply here.
result
}
/// Alters the given style to accommodate replaced content. This is called in flow construction. It /// Alters the given style to accommodate replaced content. This is called in flow construction. It
/// handles cases like `<div style="position: absolute">foo bar baz</div>` (in which `foo`, `bar`, /// handles cases like `<div style="position: absolute">foo bar baz</div>` (in which `foo`, `bar`,
/// and `baz` must not be absolutely-positioned) and cases like `<sup>Foo</sup>` (in which the /// and `baz` must not be absolutely-positioned) and cases like `<sup>Foo</sup>` (in which the
@ -6434,7 +6397,8 @@ pub fn modify_style_for_replaced_content(style: &mut Arc<ComputedValues>) {
if style.box_.display != longhands::display::computed_value::T::inline { if style.box_.display != longhands::display::computed_value::T::inline {
let mut style = Arc::make_unique(style); let mut style = Arc::make_unique(style);
Arc::make_unique(&mut style.box_).display = longhands::display::computed_value::T::inline; Arc::make_unique(&mut style.box_).display = longhands::display::computed_value::T::inline;
Arc::make_unique(&mut style.box_).position = longhands::position::computed_value::T::static_; Arc::make_unique(&mut style.box_).position =
longhands::position::computed_value::T::static_;
} }
// Reset `vertical-align` to handle cases like `<sup>foo</sup>`. // Reset `vertical-align` to handle cases like `<sup>foo</sup>`.
@ -6528,6 +6492,32 @@ pub fn modify_style_for_anonymous_table_object(
box_style.position = longhands::position::computed_value::T::static_; box_style.position = longhands::position::computed_value::T::static_;
} }
/// Adjusts the `position` property as necessary for the outer fragment wrapper of an inline-block.
#[inline]
pub fn modify_style_for_outer_inline_block_fragment(style: &mut Arc<ComputedValues>) {
let mut style = Arc::make_unique(style);
let box_style = Arc::make_unique(&mut style.box_);
box_style.position = longhands::position::computed_value::T::static_
}
/// Adjusts the `position` property as necessary to account for text.
///
/// Text is never directly relatively positioned; it's always contained within an element that is
/// itself relatively positioned.
#[inline]
pub fn modify_style_for_text(style: &mut Arc<ComputedValues>) {
if style.box_.position == longhands::position::computed_value::T::relative {
// We leave the `position` property set to `relative` so that we'll still establish a
// containing block if needed. But we reset all position offsets to `auto`.
let mut style = Arc::make_unique(style);
let mut position_offsets = Arc::make_unique(&mut style.positionoffsets);
position_offsets.top = computed::LengthOrPercentageOrAuto::Auto;
position_offsets.right = computed::LengthOrPercentageOrAuto::Auto;
position_offsets.bottom = computed::LengthOrPercentageOrAuto::Auto;
position_offsets.left = computed::LengthOrPercentageOrAuto::Auto;
}
}
pub fn is_supported_property(property: &str) -> bool { pub fn is_supported_property(property: &str) -> bool {
match_ignore_ascii_case! { property, match_ignore_ascii_case! { property,
% for property in SHORTHANDS + LONGHANDS[:-1]: % for property in SHORTHANDS + LONGHANDS[:-1]:

View file

@ -272,6 +272,7 @@ flaky_cpu == linebreak_simple_a.html linebreak_simple_b.html
== position_fixed_tile_edge_2.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_fixed_tile_edge_3.html position_fixed_tile_edge_ref.html
== position_relative_a.html position_relative_b.html == position_relative_a.html position_relative_b.html
== position_relative_inline_block_a.html position_relative_inline_block_ref.html
== position_relative_painting_order_a.html position_relative_painting_order_ref.html == position_relative_painting_order_a.html position_relative_painting_order_ref.html
== position_relative_top_percentage_a.html position_relative_top_percentage_b.html == position_relative_top_percentage_a.html position_relative_top_percentage_b.html
== pre_ignorable_whitespace_a.html pre_ignorable_whitespace_ref.html == pre_ignorable_whitespace_a.html pre_ignorable_whitespace_ref.html

View file

@ -0,0 +1,17 @@
<!DOCTYPE html>
<style>
body, html {
margin: 0;
padding: 0;
}
div {
display: inline-block;
position: relative;
top: 20px;
width: 20px;
height: 20px;
background: red;
}
</style>
<div></div>

View file

@ -0,0 +1,17 @@
<!DOCTYPE html>
<style>
body, html {
margin: 0;
padding: 0;
}
div {
display: block;
position: absolute;
top: 20px;
width: 20px;
height: 20px;
background: red;
}
</style>
<div></div>

View file

@ -1,3 +0,0 @@
[floats-153.htm]
type: reftest
expected: FAIL

View file

@ -1,3 +0,0 @@
[margin-collapse-001.htm]
type: reftest
expected: FAIL

View file

@ -1,3 +0,0 @@
[position-relative-032.htm]
type: reftest
expected: FAIL

View file

@ -1,3 +0,0 @@
[run-in-relpos-between-001.htm]
type: reftest
expected: FAIL

View file

@ -1,3 +0,0 @@
[run-in-relpos-between-002.htm]
type: reftest
expected: FAIL