From ec1c90df414b5cf3e9d76231fe466c543777c9fb Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Wed, 19 Aug 2015 14:57:42 -0700 Subject: [PATCH] layout: Centralize the logic that determines whether fragments get layers in the fragment, so that it can be activated when we're forcing the creation of extra layers due to positioned descendants that themselves have layers. The newly failing tests were tests that accidentally passed due to incorrect stacking order. Closes #7281. --- components/layout/block.rs | 60 +++++++++---- components/layout/display_list_builder.rs | 89 ++++++++----------- components/layout/fragment.rs | 17 ++++ tests/ref/basic.list | 1 + tests/ref/overflow_auto_stacking_order_a.html | 22 +++++ .../ref/overflow_auto_stacking_order_ref.html | 18 ++++ .../html4/abspos-overflow-010.htm.ini | 4 + .../html4/abspos-overflow-011.htm.ini | 3 + .../css21_dev/html4/max-height-107.htm.ini | 3 + .../css21_dev/html4/max-height-110.htm.ini | 3 + .../css21_dev/html4/min-height-104.htm.ini | 3 + .../css21_dev/html4/min-height-105.htm.ini | 3 + .../css21_dev/html4/min-height-106.htm.ini | 3 + .../min-width-not-important.html.ini | 5 ++ 14 files changed, 167 insertions(+), 67 deletions(-) create mode 100644 tests/ref/overflow_auto_stacking_order_a.html create mode 100644 tests/ref/overflow_auto_stacking_order_ref.html create mode 100644 tests/wpt/metadata-css/css21_dev/html4/abspos-overflow-010.htm.ini create mode 100644 tests/wpt/metadata-css/css21_dev/html4/abspos-overflow-011.htm.ini create mode 100644 tests/wpt/metadata-css/css21_dev/html4/max-height-107.htm.ini create mode 100644 tests/wpt/metadata-css/css21_dev/html4/max-height-110.htm.ini create mode 100644 tests/wpt/metadata-css/css21_dev/html4/min-height-104.htm.ini create mode 100644 tests/wpt/metadata-css/css21_dev/html4/min-height-105.htm.ini create mode 100644 tests/wpt/metadata-css/css21_dev/html4/min-height-106.htm.ini create mode 100644 tests/wpt/metadata/html/rendering/non-replaced-elements/the-fieldset-element-0/min-width-not-important.html.ini diff --git a/components/layout/block.rs b/components/layout/block.rs index abc169ef1e8..d697cad97a9 100644 --- a/components/layout/block.rs +++ b/components/layout/block.rs @@ -31,15 +31,17 @@ use context::LayoutContext; use display_list_builder::{BlockFlowDisplayListBuilding, BorderPaintingMode}; use display_list_builder::{FragmentDisplayListBuilding}; use floats::{ClearType, FloatKind, Floats, PlacementInfo}; -use flow::{BLOCK_POSITION_IS_STATIC, HAS_LEFT_FLOATED_DESCENDANTS, HAS_RIGHT_FLOATED_DESCENDANTS}; +use flow::{BLOCK_POSITION_IS_STATIC}; use flow::{CLEARS_LEFT, CLEARS_RIGHT}; +use flow::{HAS_LEFT_FLOATED_DESCENDANTS, HAS_RIGHT_FLOATED_DESCENDANTS}; use flow::{IMPACTED_BY_LEFT_FLOATS, IMPACTED_BY_RIGHT_FLOATS, INLINE_POSITION_IS_STATIC}; use flow::{IS_ABSOLUTELY_POSITIONED}; use flow::{ImmutableFlowUtils, MutableFlowUtils, OpaqueFlow, PreorderFlowTraversal}; use flow::{LAYERS_NEEDED_FOR_DESCENDANTS, NEEDS_LAYER}; use flow::{PostorderFlowTraversal, mut_base}; use flow::{self, AbsolutePositionInfo, BaseFlow, ForceNonfloatedFlag, FlowClass, Flow}; -use fragment::{CoordinateSystem, Fragment, FragmentBorderBoxIterator, SpecificFragmentInfo}; +use fragment::{CoordinateSystem, Fragment, FragmentBorderBoxIterator, HAS_LAYER}; +use fragment::{SpecificFragmentInfo}; use incremental::{REFLOW, REFLOW_OUT_OF_FLOW}; use layout_debug; use layout_task::DISPLAY_PORT_SIZE_FACTOR; @@ -787,6 +789,8 @@ impl BlockFlow { self.base.debug_id()); if self.base.restyle_damage.contains(REFLOW) { + self.determine_if_layer_needed(); + // Our current border-box position. let mut cur_b = Au(0); @@ -950,11 +954,6 @@ impl BlockFlow { } if self.base.flags.contains(IS_ABSOLUTELY_POSITIONED) { - // Fixed position layers get layers. - if self.is_fixed() { - self.base.flags.insert(NEEDS_LAYER); - } - // Store the content block-size for use in calculating the absolute flow's // dimensions later. // @@ -1564,6 +1563,36 @@ impl BlockFlow { } self.base.flags = flags } + + fn determine_if_layer_needed(&mut self) { + if self.base.flags.contains(IS_ABSOLUTELY_POSITIONED) { + // Fixed position layers get layers. + if self.is_fixed() { + self.base.flags.insert(NEEDS_LAYER); + return + } + } + + // This flow needs a layer if it has a 3d transform, or provides perspective + // to child layers. See http://dev.w3.org/csswg/css-transforms/#3d-rendering-contexts. + let has_3d_transform = self.transform_requires_layer(); + let has_perspective = self.fragment.style().get_effects().perspective != + LengthOrNone::None; + + if has_3d_transform || has_perspective { + self.base.flags.insert(NEEDS_LAYER); + return + } + + match (self.fragment.style().get_box().overflow_x, + self.fragment.style().get_box().overflow_y.0) { + (overflow_x::T::auto, _) | (overflow_x::T::scroll, _) | + (_, overflow_x::T::auto) | (_, overflow_x::T::scroll) => { + self.base.flags.insert(NEEDS_LAYER); + } + _ => {} + } + } } impl Flow for BlockFlow { @@ -1745,6 +1774,12 @@ impl Flow for BlockFlow { } fn compute_absolute_position(&mut self, layout_context: &LayoutContext) { + if (self.base.flags.contains(IS_ABSOLUTELY_POSITIONED) && + self.base.absolute_position_info.layers_needed_for_positioned_flows) || + self.base.flags.contains(NEEDS_LAYER) { + self.fragment.flags.insert(HAS_LAYER) + } + // FIXME (mbrubeck): Get the real container size, taking the container writing mode into // account. Must handle vertical writing modes. let container_size = Size2D::new(self.base.block_container_inline_size, Au(0)); @@ -1754,15 +1789,7 @@ impl Flow for BlockFlow { self.base.stacking_relative_position_of_display_port = MAX_RECT; } - // This flow needs a layer if it has a 3d transform, or provides perspective - // to child layers. See http://dev.w3.org/csswg/css-transforms/#3d-rendering-contexts. let transform_style = self.fragment.style().get_used_transform_style(); - let has_3d_transform = self.transform_requires_layer(); - let has_perspective = self.fragment.style().get_effects().perspective != LengthOrNone::None; - - if has_3d_transform || has_perspective { - self.base.flags.insert(NEEDS_LAYER); - } if self.base.flags.contains(IS_ABSOLUTELY_POSITIONED) { // `overflow: auto` and `overflow: scroll` force creation of layers, since we can only @@ -1771,7 +1798,6 @@ impl Flow for BlockFlow { self.fragment.style().get_box().overflow_y.0) { (overflow_x::T::auto, _) | (overflow_x::T::scroll, _) | (_, overflow_x::T::auto) | (_, overflow_x::T::scroll) => { - self.base.flags.insert(NEEDS_LAYER); self.base.clip = ClippingRegion::max(); self.base.stacking_relative_position_of_display_port = MAX_RECT; } @@ -1886,7 +1912,7 @@ impl Flow for BlockFlow { } let stacking_relative_position_of_display_port_for_children = - if (is_stacking_context && self.will_get_layer()) || self.is_root() { + if is_stacking_context || self.is_root() { let visible_rect = match layout_context.shared.visible_rects.get(&self.layer_id(0)) { Some(visible_rect) => *visible_rect, diff --git a/components/layout/display_list_builder.rs b/components/layout/display_list_builder.rs index e49b306169f..f7dde7598f8 100644 --- a/components/layout/display_list_builder.rs +++ b/components/layout/display_list_builder.rs @@ -14,9 +14,9 @@ use azure::azure_hl::Color; use block::BlockFlow; use context::LayoutContext; use flex::FlexFlow; -use flow::{self, BaseFlow, Flow, IS_ABSOLUTELY_POSITIONED, NEEDS_LAYER}; +use flow::{self, BaseFlow, Flow, IS_ABSOLUTELY_POSITIONED}; use flow_ref; -use fragment::{CoordinateSystem, Fragment, IframeFragmentInfo, ImageFragmentInfo}; +use fragment::{CoordinateSystem, Fragment, HAS_LAYER, IframeFragmentInfo, ImageFragmentInfo}; use fragment::{ScannedTextFragmentInfo, SpecificFragmentInfo}; use inline::InlineFlow; use list_item::ListItemFlow; @@ -1513,7 +1513,6 @@ pub trait BlockFlowDisplayListBuilding { display_list: Box, layout_context: &LayoutContext, border_painting_mode: BorderPaintingMode); - fn will_get_layer(&self) -> bool; } impl BlockFlowDisplayListBuilding for BlockFlow { @@ -1557,35 +1556,32 @@ impl BlockFlowDisplayListBuilding for BlockFlow { border_painting_mode, background_border_level); - self.base.display_list_building_result = if self.fragment.establishes_stacking_context() { - if self.will_get_layer() { - // If we got here, then we need a new layer. - let scroll_policy = if self.is_fixed() { - ScrollPolicy::FixedPosition - } else { - ScrollPolicy::Scrollable - }; + self.base.display_list_building_result = if self.fragment.flags.contains(HAS_LAYER) { + let scroll_policy = if self.is_fixed() { + ScrollPolicy::FixedPosition + } else { + ScrollPolicy::Scrollable + }; - let paint_layer = PaintLayer::new(self.layer_id(0), - color::transparent(), - scroll_policy); - let layer = StackingContextLayer::Existing(paint_layer); - let stacking_context = self.fragment.create_stacking_context( + let paint_layer = PaintLayer::new(self.layer_id(0), + color::transparent(), + scroll_policy); + let layer = StackingContextLayer::Existing(paint_layer); + let stacking_context = self.fragment.create_stacking_context( + &self.base, + display_list, + layout_context, + layer, + StackingContextCreationMode::Normal); + DisplayListBuildingResult::StackingContext(stacking_context) + } else if self.fragment.establishes_stacking_context() { + DisplayListBuildingResult::StackingContext( + self.fragment.create_stacking_context( &self.base, display_list, layout_context, - layer, - StackingContextCreationMode::Normal); - DisplayListBuildingResult::StackingContext(stacking_context) - } else { - DisplayListBuildingResult::StackingContext( - self.fragment.create_stacking_context( - &self.base, - display_list, - layout_context, - StackingContextLayer::IfCanvas(self.layer_id(0)), - StackingContextCreationMode::Normal)) - } + StackingContextLayer::IfCanvas(self.layer_id(0)), + StackingContextCreationMode::Normal)) } else { match self.fragment.style.get_box().position { position::T::static_ => {} @@ -1597,11 +1593,6 @@ impl BlockFlowDisplayListBuilding for BlockFlow { } } - fn will_get_layer(&self) -> bool { - self.base.absolute_position_info.layers_needed_for_positioned_flows || - self.base.flags.contains(NEEDS_LAYER) - } - fn build_display_list_for_absolutely_positioned_block( &mut self, mut display_list: Box, @@ -1654,23 +1645,21 @@ impl BlockFlowDisplayListBuilding for BlockFlow { } }; - if !self.fragment.establishes_stacking_context() { - display_list.form_pseudo_stacking_context_for_positioned_content(); - self.base.display_list_building_result = - DisplayListBuildingResult::Normal(display_list); - return - } - - if !self.will_get_layer() { - // We didn't need a layer. - self.base.display_list_building_result = - DisplayListBuildingResult::StackingContext( - self.fragment.create_stacking_context( - &self.base, - display_list, - layout_context, - StackingContextLayer::IfCanvas(self.layer_id(0)), - StackingContextCreationMode::Normal)); + if !self.fragment.flags.contains(HAS_LAYER) { + if !self.fragment.establishes_stacking_context() { + display_list.form_pseudo_stacking_context_for_positioned_content(); + self.base.display_list_building_result = + DisplayListBuildingResult::Normal(display_list); + } else { + self.base.display_list_building_result = + DisplayListBuildingResult::StackingContext( + self.fragment.create_stacking_context( + &self.base, + display_list, + layout_context, + StackingContextLayer::IfCanvas(self.layer_id(0)), + StackingContextCreationMode::Normal)); + } return } diff --git a/components/layout/fragment.rs b/components/layout/fragment.rs index 55ee934bb7e..833424e6656 100644 --- a/components/layout/fragment.rs +++ b/components/layout/fragment.rs @@ -111,6 +111,9 @@ pub struct Fragment { /// The pseudo-element that this fragment represents. pub pseudo: PseudoElementType<()>, + /// Various flags for this fragment. + pub flags: FragmentFlags, + /// A debug ID that is consistent for the life of this fragment (via transform etc). pub debug_id: u16, } @@ -761,6 +764,7 @@ impl Fragment { specific: specific, inline_context: None, pseudo: node.get_pseudo_element_type().strip(), + flags: FragmentFlags::empty(), debug_id: layout_debug::generate_unique_debug_id(), } } @@ -783,6 +787,7 @@ impl Fragment { specific: specific, inline_context: None, pseudo: pseudo, + flags: FragmentFlags::empty(), debug_id: layout_debug::generate_unique_debug_id(), } } @@ -816,6 +821,7 @@ impl Fragment { specific: info, inline_context: self.inline_context.clone(), pseudo: self.pseudo.clone(), + flags: FragmentFlags::empty(), debug_id: self.debug_id, } } @@ -1987,6 +1993,9 @@ impl Fragment { /// Returns true if this fragment establishes a new stacking context and false otherwise. pub fn establishes_stacking_context(&self) -> bool { + if self.flags.contains(HAS_LAYER) { + return true + } if self.style().get_effects().opacity != 1.0 { return true } @@ -2373,3 +2382,11 @@ impl WhitespaceStrippingResult { } } } + +bitflags! { + flags FragmentFlags: u8 { + /// Whether this fragment has a layer. + const HAS_LAYER = 0x01, + } +} + diff --git a/tests/ref/basic.list b/tests/ref/basic.list index 40c2e3bd3af..fc309d5f4c6 100644 --- a/tests/ref/basic.list +++ b/tests/ref/basic.list @@ -257,6 +257,7 @@ flaky_cpu == linebreak_simple_a.html linebreak_simple_b.html # Should be == with expected failure. See #2797 != overconstrained_block.html overconstrained_block_ref.html == overflow_auto.html overflow_simple_b.html +== overflow_auto_stacking_order_a.html overflow_auto_stacking_order_ref.html # Should be ==? != overflow_position_abs_inside_normal_a.html overflow_position_abs_inside_normal_b.html == overflow_position_abs_simple_a.html overflow_position_abs_simple_b.html diff --git a/tests/ref/overflow_auto_stacking_order_a.html b/tests/ref/overflow_auto_stacking_order_a.html new file mode 100644 index 00000000000..1e958302c35 --- /dev/null +++ b/tests/ref/overflow_auto_stacking_order_a.html @@ -0,0 +1,22 @@ + + +
+
+ diff --git a/tests/ref/overflow_auto_stacking_order_ref.html b/tests/ref/overflow_auto_stacking_order_ref.html new file mode 100644 index 00000000000..a8f5cd1287f --- /dev/null +++ b/tests/ref/overflow_auto_stacking_order_ref.html @@ -0,0 +1,18 @@ + + +
+ + diff --git a/tests/wpt/metadata-css/css21_dev/html4/abspos-overflow-010.htm.ini b/tests/wpt/metadata-css/css21_dev/html4/abspos-overflow-010.htm.ini new file mode 100644 index 00000000000..5504b7061bf --- /dev/null +++ b/tests/wpt/metadata-css/css21_dev/html4/abspos-overflow-010.htm.ini @@ -0,0 +1,4 @@ +[abspos-overflow-010.htm] + type: reftest + expected: + if os == "linux": FAIL diff --git a/tests/wpt/metadata-css/css21_dev/html4/abspos-overflow-011.htm.ini b/tests/wpt/metadata-css/css21_dev/html4/abspos-overflow-011.htm.ini new file mode 100644 index 00000000000..6ef62736b3c --- /dev/null +++ b/tests/wpt/metadata-css/css21_dev/html4/abspos-overflow-011.htm.ini @@ -0,0 +1,3 @@ +[abspos-overflow-011.htm] + type: reftest + expected: FAIL diff --git a/tests/wpt/metadata-css/css21_dev/html4/max-height-107.htm.ini b/tests/wpt/metadata-css/css21_dev/html4/max-height-107.htm.ini new file mode 100644 index 00000000000..c9bfff0eacd --- /dev/null +++ b/tests/wpt/metadata-css/css21_dev/html4/max-height-107.htm.ini @@ -0,0 +1,3 @@ +[max-height-107.htm] + type: reftest + expected: FAIL diff --git a/tests/wpt/metadata-css/css21_dev/html4/max-height-110.htm.ini b/tests/wpt/metadata-css/css21_dev/html4/max-height-110.htm.ini new file mode 100644 index 00000000000..21d4e382984 --- /dev/null +++ b/tests/wpt/metadata-css/css21_dev/html4/max-height-110.htm.ini @@ -0,0 +1,3 @@ +[max-height-110.htm] + type: reftest + expected: FAIL diff --git a/tests/wpt/metadata-css/css21_dev/html4/min-height-104.htm.ini b/tests/wpt/metadata-css/css21_dev/html4/min-height-104.htm.ini new file mode 100644 index 00000000000..1fec05c57f0 --- /dev/null +++ b/tests/wpt/metadata-css/css21_dev/html4/min-height-104.htm.ini @@ -0,0 +1,3 @@ +[min-height-104.htm] + type: reftest + expected: FAIL diff --git a/tests/wpt/metadata-css/css21_dev/html4/min-height-105.htm.ini b/tests/wpt/metadata-css/css21_dev/html4/min-height-105.htm.ini new file mode 100644 index 00000000000..165e8dea549 --- /dev/null +++ b/tests/wpt/metadata-css/css21_dev/html4/min-height-105.htm.ini @@ -0,0 +1,3 @@ +[min-height-105.htm] + type: reftest + expected: FAIL diff --git a/tests/wpt/metadata-css/css21_dev/html4/min-height-106.htm.ini b/tests/wpt/metadata-css/css21_dev/html4/min-height-106.htm.ini new file mode 100644 index 00000000000..83bdbfdf497 --- /dev/null +++ b/tests/wpt/metadata-css/css21_dev/html4/min-height-106.htm.ini @@ -0,0 +1,3 @@ +[min-height-106.htm] + type: reftest + expected: FAIL diff --git a/tests/wpt/metadata/html/rendering/non-replaced-elements/the-fieldset-element-0/min-width-not-important.html.ini b/tests/wpt/metadata/html/rendering/non-replaced-elements/the-fieldset-element-0/min-width-not-important.html.ini new file mode 100644 index 00000000000..24f32b30a6c --- /dev/null +++ b/tests/wpt/metadata/html/rendering/non-replaced-elements/the-fieldset-element-0/min-width-not-important.html.ini @@ -0,0 +1,5 @@ +[min-width-not-important.html] + type: reftest + reftype: == + refurl: /html/rendering/non-replaced-elements/the-fieldset-element-0/ref.html + expected: FAIL