From 1e884ddc6969989fd2837d5262ff8cceb0ca2f9f Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Wed, 6 Apr 2016 17:53:11 -0700 Subject: [PATCH 1/2] layout: Allow non-absolutely-positioned elements with `overflow: scroll` set to be scrolled. This makes them establish stacking contexts, which is a CSS 2.1 spec violation. However, we were already violating the spec here for absolutely-positioned elements with `overflow: scroll`. It will probably be easier to fix this spec violation once we either switch entirely to WebRender or we have multiple layers per stacking context. Closes #2742. --- components/gfx_traits/lib.rs | 4 ++ components/layout/block.rs | 4 -- components/layout/display_list_builder.rs | 35 ++++++----------- .../overflow_scroll_relative_position.html | 38 +++++++++++++++++++ 4 files changed, 54 insertions(+), 27 deletions(-) create mode 100644 tests/html/overflow_scroll_relative_position.html diff --git a/components/gfx_traits/lib.rs b/components/gfx_traits/lib.rs index 5833e847a91..6f5abcb1963 100644 --- a/components/gfx_traits/lib.rs +++ b/components/gfx_traits/lib.rs @@ -105,6 +105,10 @@ impl LayerId { let LayerId(layer_type, id, _) = *self; LayerId(layer_type, id, 0) } + + pub fn kind(&self) -> LayerType { + self.0 + } } /// All layer-specific information that the painting task sends to the compositor other than the diff --git a/components/layout/block.rs b/components/layout/block.rs index 390810e9831..519a76c08e7 100644 --- a/components/layout/block.rs +++ b/components/layout/block.rs @@ -1563,10 +1563,6 @@ impl BlockFlow { } pub fn has_scrolling_overflow(&self) -> bool { - if !self.base.flags.contains(IS_ABSOLUTELY_POSITIONED) { - return false; - } - match (self.fragment.style().get_box().overflow_x, self.fragment.style().get_box().overflow_y.0) { (overflow_x::T::auto, _) | (overflow_x::T::scroll, _) | diff --git a/components/layout/display_list_builder.rs b/components/layout/display_list_builder.rs index 7f8fe5df579..0c04c839450 100644 --- a/components/layout/display_list_builder.rs +++ b/components/layout/display_list_builder.rs @@ -1470,33 +1470,22 @@ impl FragmentDisplayListBuilding for Fragment { // Clip according to the values of `overflow-x` and `overflow-y`. // - // TODO(pcwalton): Support scrolling of non-absolutely-positioned elements. // FIXME(pcwalton): This may be more complex than it needs to be, since it seems to be // impossible with the computed value rules as they are to have `overflow-x: visible` with // `overflow-y: ` or vice versa! - match (self.style.get_box().overflow_x, is_absolutely_positioned) { - (overflow_x::T::hidden, _) | - (overflow_x::T::auto, false) | - (overflow_x::T::scroll, false) => { - let mut bounds = current_clip.bounding_rect(); - let max_x = cmp::min(bounds.max_x(), overflow_clip_rect.max_x()); - bounds.origin.x = cmp::max(bounds.origin.x, overflow_clip_rect.origin.x); - bounds.size.width = max_x - bounds.origin.x; - current_clip.intersect_rect(&bounds) - } - _ => {} + if let overflow_x::T::hidden = self.style.get_box().overflow_x { + let mut bounds = current_clip.bounding_rect(); + let max_x = cmp::min(bounds.max_x(), overflow_clip_rect.max_x()); + bounds.origin.x = cmp::max(bounds.origin.x, overflow_clip_rect.origin.x); + bounds.size.width = max_x - bounds.origin.x; + current_clip.intersect_rect(&bounds) } - match (self.style.get_box().overflow_y.0, is_absolutely_positioned) { - (overflow_x::T::hidden, _) | - (overflow_x::T::auto, false) | - (overflow_x::T::scroll, false) => { - let mut bounds = current_clip.bounding_rect(); - let max_y = cmp::min(bounds.max_y(), overflow_clip_rect.max_y()); - bounds.origin.y = cmp::max(bounds.origin.y, overflow_clip_rect.origin.y); - bounds.size.height = max_y - bounds.origin.y; - current_clip.intersect_rect(&bounds) - } - _ => {} + if let overflow_x::T::hidden = self.style.get_box().overflow_y.0 { + let mut bounds = current_clip.bounding_rect(); + let max_y = cmp::min(bounds.max_y(), overflow_clip_rect.max_y()); + bounds.origin.y = cmp::max(bounds.origin.y, overflow_clip_rect.origin.y); + bounds.size.height = max_y - bounds.origin.y; + current_clip.intersect_rect(&bounds) } let border_radii = build_border_radius(stacking_relative_border_box, diff --git a/tests/html/overflow_scroll_relative_position.html b/tests/html/overflow_scroll_relative_position.html new file mode 100644 index 00000000000..83c4b8c1675 --- /dev/null +++ b/tests/html/overflow_scroll_relative_position.html @@ -0,0 +1,38 @@ + + + + + + + This element should be scrollable. +
+
+
+
+
+
+ + From 3518472f7c5e4ad8eb61eb2b9263f151f8b6c797 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Wed, 6 Apr 2016 17:55:11 -0700 Subject: [PATCH 2/2] gfx: When dumping the display list, mention whether each layer scrolls its overflow area. --- components/gfx/display_list/mod.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/components/gfx/display_list/mod.rs b/components/gfx/display_list/mod.rs index 2a132cf7f61..16ac9a29ea7 100644 --- a/components/gfx/display_list/mod.rs +++ b/components/gfx/display_list/mod.rs @@ -725,8 +725,15 @@ impl fmt::Debug for StackingContext { "Pseudo-StackingContext" }; - write!(f, "{} at {:?} with overflow {:?}: {:?}", + let scrollable_string = if self.scrolls_overflow_area { + " (scrolls overflow area)" + } else { + "" + }; + + write!(f, "{}{} at {:?} with overflow {:?}: {:?}", type_string, + scrollable_string, self.bounds, self.overflow, self.id)