Auto merge of #10450 - pcwalton:overflow-scroll-non-positioned, r=mbrubeck

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.

r? @mbrubeck

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10450)
<!-- Reviewable:end -->

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)

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

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, _) |

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: <scrolling>` or vice versa!
-        match (self.style.get_box().overflow_x, is_absolutely_positioned) {
+        if let overflow_x::T::hidden = self.style.get_box().overflow_x {
-            (overflow_x::T::hidden, _) |
+            let mut bounds = current_clip.bounding_rect();
-            (overflow_x::T::auto, false) |
+            let max_x = cmp::min(bounds.max_x(), overflow_clip_rect.max_x());
-            (overflow_x::T::scroll, false) => {
+            bounds.origin.x = cmp::max(bounds.origin.x, overflow_clip_rect.origin.x);
-                let mut bounds = current_clip.bounding_rect();
+            bounds.size.width = max_x - bounds.origin.x;
-                let max_x = cmp::min(bounds.max_x(), overflow_clip_rect.max_x());
+            current_clip.intersect_rect(&bounds)
-                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) {
+        if let overflow_x::T::hidden = self.style.get_box().overflow_y.0 {
-            (overflow_x::T::hidden, _) |
+            let mut bounds = current_clip.bounding_rect();
-            (overflow_x::T::auto, false) |
+            let max_y = cmp::min(bounds.max_y(), overflow_clip_rect.max_y());
-            (overflow_x::T::scroll, false) => {
+            bounds.origin.y = cmp::max(bounds.origin.y, overflow_clip_rect.origin.y);
-                let mut bounds = current_clip.bounding_rect();
+            bounds.size.height = max_y - bounds.origin.y;
-                let max_y = cmp::min(bounds.max_y(), overflow_clip_rect.max_y());
+            current_clip.intersect_rect(&bounds)
-                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,

tests/html/overflow_scroll_relative_position.html

@@ -0,0 +1,38 @@
+<html>
+  <head>
+    <link rel=match href=overflow_simple_b.html>
+    <style>
+      #first {
+      height: 100px;
+      width: 100px;
+      overflow: scroll;
+      position: relative;
+      }
+      #second {
+      position: absolute;
+      height: 100px;
+      width: 90px;
+      top: 0;
+      left: 0;
+      background: green;
+      }
+      #third {
+      position: absolute;
+      height: 100px;
+      width: 110px;
+      top: 0;
+      left: 90px;
+      background: orange;
+      }
+    </style>
+  </head>
+  <body>
+    This element should be scrollable.
+    <div id="first">
+      <div id="second">
+      </div>
+      <div id="third">
+      </div>
+    </div>
+  </body>
+</html>