diff options
author | Patrick Walton <pcwalton@mimiga.net> | 2015-08-10 18:05:29 -0700 |
---|---|---|
committer | Patrick Walton <pcwalton@mimiga.net> | 2015-08-12 08:28:35 -0700 |
commit | 8640cf55889c44b0d252eedab3da5f6153cd217b (patch) | |
tree | 8c704b9f545d0941c1ecb8cafe572aeecb9f0a1f | |
parent | 3ad49fc689ceb6067cd6dea1aa0d004321704b8e (diff) | |
download | servo-8640cf55889c44b0d252eedab3da5f6153cd217b.tar.gz servo-8640cf55889c44b0d252eedab3da5f6153cd217b.zip |
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.
11 files changed, 103 insertions, 89 deletions
diff --git a/components/layout/construct.rs b/components/layout/construct.rs index ce4417a5a00..5976045e740 100644 --- a/components/layout/construct.rs +++ b/components/layout/construct.rs @@ -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) { self.fragments.fragments.push_back(fragment) } @@ -686,7 +700,15 @@ impl<'a> FlowConstructor<'a> { fragments: &mut IntermediateInlineFragments, node: &ThreadSafeLayoutNode, 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 { ContentItem::String(string) => { let info = UnscannedTextFragmentInfo::from_text(string); @@ -910,11 +932,18 @@ impl<'a> FlowConstructor<'a> { _ => 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( block_flow)); - let fragment = Fragment::new(node, fragment_info); - - let mut fragment_accumulator = InlineFragmentsAccumulator::from_inline_node(node); + 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_and_style(node, modified_style); fragment_accumulator.fragments.fragments.push_back(fragment); let construction_item = @@ -1317,6 +1346,12 @@ impl<'a> FlowConstructor<'a> { inline_absolute_fragment.flow_ref .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() { properties::modify_style_for_replaced_content(&mut style); diff --git a/components/layout/fragment.rs b/components/layout/fragment.rs index acec659b76a..ffbcd1845a2 100644 --- a/components/layout/fragment.rs +++ b/components/layout/fragment.rs @@ -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::{position, text_align, text_decoration, transform_style, white_space}; 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::{LengthOrPercentageOrNone}; 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. pub fn from_opaque_node_and_style(node: OpaqueNode, pseudo: PseudoElementType<()>, diff --git a/components/style/properties.mako.rs b/components/style/properties.mako.rs index d7d1d71cfd0..93b73fecf65 100644 --- a/components/style/properties.mako.rs +++ b/components/style/properties.mako.rs @@ -6385,43 +6385,6 @@ pub fn cascade(viewport_size: Size2D<Au>, }, 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 /// 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 @@ -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 { 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_).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>`. @@ -6528,6 +6492,32 @@ pub fn modify_style_for_anonymous_table_object( 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 { match_ignore_ascii_case! { property, % for property in SHORTHANDS + LONGHANDS[:-1]: diff --git a/tests/ref/basic.list b/tests/ref/basic.list index 2442cbb26ba..6d5bba85d1e 100644 --- a/tests/ref/basic.list +++ b/tests/ref/basic.list @@ -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_3.html position_fixed_tile_edge_ref.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_top_percentage_a.html position_relative_top_percentage_b.html == pre_ignorable_whitespace_a.html pre_ignorable_whitespace_ref.html diff --git a/tests/ref/position_relative_inline_block_a.html b/tests/ref/position_relative_inline_block_a.html new file mode 100644 index 00000000000..cddeceef9f5 --- /dev/null +++ b/tests/ref/position_relative_inline_block_a.html @@ -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> + diff --git a/tests/ref/position_relative_inline_block_ref.html b/tests/ref/position_relative_inline_block_ref.html new file mode 100644 index 00000000000..49bc6cad631 --- /dev/null +++ b/tests/ref/position_relative_inline_block_ref.html @@ -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> + diff --git a/tests/wpt/metadata-css/css21_dev/html4/floats-153.htm.ini b/tests/wpt/metadata-css/css21_dev/html4/floats-153.htm.ini deleted file mode 100644 index 28ff68c3845..00000000000 --- a/tests/wpt/metadata-css/css21_dev/html4/floats-153.htm.ini +++ /dev/null @@ -1,3 +0,0 @@ -[floats-153.htm] - type: reftest - expected: FAIL diff --git a/tests/wpt/metadata-css/css21_dev/html4/margin-collapse-001.htm.ini b/tests/wpt/metadata-css/css21_dev/html4/margin-collapse-001.htm.ini deleted file mode 100644 index 50450150f83..00000000000 --- a/tests/wpt/metadata-css/css21_dev/html4/margin-collapse-001.htm.ini +++ /dev/null @@ -1,3 +0,0 @@ -[margin-collapse-001.htm] - type: reftest - expected: FAIL diff --git a/tests/wpt/metadata-css/css21_dev/html4/position-relative-032.htm.ini b/tests/wpt/metadata-css/css21_dev/html4/position-relative-032.htm.ini deleted file mode 100644 index 45604a15527..00000000000 --- a/tests/wpt/metadata-css/css21_dev/html4/position-relative-032.htm.ini +++ /dev/null @@ -1,3 +0,0 @@ -[position-relative-032.htm] - type: reftest - expected: FAIL diff --git a/tests/wpt/metadata-css/css21_dev/html4/run-in-relpos-between-001.htm.ini b/tests/wpt/metadata-css/css21_dev/html4/run-in-relpos-between-001.htm.ini deleted file mode 100644 index 22f5320ed02..00000000000 --- a/tests/wpt/metadata-css/css21_dev/html4/run-in-relpos-between-001.htm.ini +++ /dev/null @@ -1,3 +0,0 @@ -[run-in-relpos-between-001.htm] - type: reftest - expected: FAIL diff --git a/tests/wpt/metadata-css/css21_dev/html4/run-in-relpos-between-002.htm.ini b/tests/wpt/metadata-css/css21_dev/html4/run-in-relpos-between-002.htm.ini deleted file mode 100644 index 81d078b5356..00000000000 --- a/tests/wpt/metadata-css/css21_dev/html4/run-in-relpos-between-002.htm.ini +++ /dev/null @@ -1,3 +0,0 @@ -[run-in-relpos-between-002.htm] - type: reftest - expected: FAIL |