Allow overflow:scroll without a stacking context

Fix the long-standing bug where items that are positioned and have
overflow:scroll or overflow:auto automatically create stacking
contexts. In order to do this we need to fix another bug where display
list sorting can put a Clip or ScrollFrame definition after the first
time it is used in a display list.

components/layout/display_list_builder.rs

@@ -169,6 +169,9 @@ pub struct DisplayListBuildState<'a> {
     /// recursively building and processing the display list.
     pub current_stacking_context_id: StackingContextId,
 
+    /// The current stacking real context id, which doesn't include pseudo-stacking contexts.
+    pub current_real_stacking_context_id: StackingContextId,
+
     /// The current clip and scroll info, used to keep track of state when
     /// recursively building and processing the display list.
     pub current_clip_and_scroll_info: ClipAndScrollInfo,
@@ -202,6 +205,7 @@ impl<'a> DisplayListBuildState<'a> {
             scroll_root_parents: HashMap::new(),
             processing_scroll_root_element: false,
             current_stacking_context_id: StackingContextId::root(),
+            current_real_stacking_context_id: StackingContextId::root(),
             current_clip_and_scroll_info: root_clip_info,
             containing_block_clip_and_scroll_info: root_clip_info,
             iframe_sizes: Vec::new(),
@@ -228,10 +232,14 @@ impl<'a> DisplayListBuildState<'a> {
         self.scroll_root_parents.contains_key(&id)
     }
 
-    fn add_scroll_root(&mut self, scroll_root: ScrollRoot, stacking_context_id: StackingContextId) {
+    fn add_scroll_root(&mut self, scroll_root: ScrollRoot) {
+        // We want the scroll root to be defined by another other possible item that could use it,
+        // so we make sure that it is added to the beginning of the parent "real" (non-pseudo)
+        // stacking context. This ensures that item reordering will not result in an item using
+        // the scroll root before it is defined.
         self.scroll_root_parents.insert(scroll_root.id, scroll_root.parent_id);
         let info = self.stacking_context_info
-                       .entry(stacking_context_id)
+                       .entry(self.current_real_stacking_context_id)
                        .or_insert(StackingContextInfo::new());
         info.scroll_roots.push(scroll_root);
     }
@@ -547,7 +555,7 @@ pub trait FragmentDisplayListBuilding {
                                id: StackingContextId,
                                base_flow: &BaseFlow,
                                scroll_policy: ScrollPolicy,
-                               mode: StackingContextCreationMode,
+                               context_type: StackingContextType,
                                parent_clip_and_scroll_info: ClipAndScrollInfo)
                                -> StackingContext;
 
@@ -2035,7 +2043,7 @@ impl FragmentDisplayListBuilding for Fragment {
                                id: StackingContextId,
                                base_flow: &BaseFlow,
                                scroll_policy: ScrollPolicy,
-                               mode: StackingContextCreationMode,
+                               context_type: StackingContextType,
                                parent_clip_and_scroll_info: ClipAndScrollInfo)
                                -> StackingContext {
         let border_box =
@@ -2059,12 +2067,6 @@ impl FragmentDisplayListBuilding for Fragment {
             filters.push(Filter::Opacity(effects.opacity.into()))
         }
 
-        let context_type = match mode {
-            StackingContextCreationMode::PseudoFloat => StackingContextType::PseudoFloat,
-            StackingContextCreationMode::PseudoPositioned => StackingContextType::PseudoPositioned,
-            _ => StackingContextType::Real,
-        };
-
         StackingContext::new(id,
                              context_type,
                              &border_box,
@@ -2290,6 +2292,7 @@ pub trait BlockFlowDisplayListBuilding {
 /// TODO(mrobinson): It would be nice to use RAII here to avoid having to call restore.
 pub struct PreservedDisplayListState {
     stacking_context_id: StackingContextId,
+    real_stacking_context_id: StackingContextId,
     clip_and_scroll_info: ClipAndScrollInfo,
     containing_block_clip_and_scroll_info: ClipAndScrollInfo,
     clips_pushed: usize,
@@ -2300,6 +2303,7 @@ impl PreservedDisplayListState {
     fn new(state: &mut DisplayListBuildState) -> PreservedDisplayListState {
         PreservedDisplayListState {
             stacking_context_id: state.current_stacking_context_id,
+            real_stacking_context_id: state.current_real_stacking_context_id,
             clip_and_scroll_info: state.current_clip_and_scroll_info,
             containing_block_clip_and_scroll_info: state.containing_block_clip_and_scroll_info,
             clips_pushed: 0,
@@ -2315,6 +2319,7 @@ impl PreservedDisplayListState {
 
     fn restore(self, state: &mut DisplayListBuildState) {
         state.current_stacking_context_id = self.stacking_context_id;
+        state.current_real_stacking_context_id = self.real_stacking_context_id;
         state.current_clip_and_scroll_info = self.clip_and_scroll_info;
         state.containing_block_clip_and_scroll_info = self.containing_block_clip_and_scroll_info;
 
@@ -2418,6 +2423,10 @@ impl BlockFlowDisplayListBuilding for BlockFlow {
         };
         state.current_stacking_context_id = self.base.stacking_context_id;
 
+        if block_stacking_context_type == BlockStackingContextType::StackingContext {
+            state.current_real_stacking_context_id = self.base.stacking_context_id;
+        }
+
         // We are getting the id of the scroll root that contains us here, not the id of
         // any scroll root that we create. If we create a scroll root, its id will be
         // stored in state.current_clip_and_scroll_info. If we create a stacking context,
@@ -2552,7 +2561,6 @@ impl BlockFlowDisplayListBuilding for BlockFlow {
                 content_rect: Rect::new(content_box.origin, content_size),
                 root_type: ScrollRootType::ScrollFrame(sensitivity),
             },
-            self.base.stacking_context_id
         );
 
         let new_clip_and_scroll_info = ClipAndScrollInfo::simple(new_scroll_root_id);
@@ -2613,7 +2621,6 @@ impl BlockFlowDisplayListBuilding for BlockFlow {
                 content_rect: Rect::zero(), // content_rect isn't important for clips.
                 root_type: ScrollRootType::Clip,
             },
-            self.base.stacking_context_id
         );
 
         let new_clip_and_scroll_info = ClipAndScrollInfo::new(new_scroll_root_id, new_scroll_root_id);
@@ -2627,10 +2634,10 @@ impl BlockFlowDisplayListBuilding for BlockFlow {
                                                 state: &mut DisplayListBuildState) {
         let creation_mode = if self.base.flags.contains(IS_ABSOLUTELY_POSITIONED) ||
                                self.fragment.style.get_box().position != position::T::static_ {
-            StackingContextCreationMode::PseudoPositioned
+            StackingContextType::PseudoPositioned
         } else {
             assert!(self.base.flags.is_float());
-            StackingContextCreationMode::PseudoFloat
+            StackingContextType::PseudoFloat
         };
 
         let new_context = self.fragment.create_stacking_context(self.base.stacking_context_id,
@@ -2669,7 +2676,7 @@ impl BlockFlowDisplayListBuilding for BlockFlow {
             self.base.stacking_context_id,
             &self.base,
             scroll_policy,
-            StackingContextCreationMode::Normal,
+            StackingContextType::Real,
             parent_clip_and_scroll_info);
 
         state.add_stacking_context(parent_stacking_context_id, stacking_context);
@@ -2754,7 +2761,7 @@ impl InlineFlowDisplayListBuilding for InlineFlow {
                     let stacking_context = fragment.create_stacking_context(fragment.stacking_context_id,
                                                                             &self.base,
                                                                             ScrollPolicy::Scrollable,
-                                                                            StackingContextCreationMode::Normal,
+                                                                            StackingContextType::Real,
                                                                             state.current_clip_and_scroll_info);
 
                     state.add_stacking_context(current_stacking_context_id,
@@ -2962,10 +2969,3 @@ pub enum BorderPaintingMode<'a> {
     /// Paint no borders.
     Hidden,
 }
-
-#[derive(Clone, Copy, PartialEq)]
-pub enum StackingContextCreationMode {
-    Normal,
-    PseudoPositioned,
-    PseudoFloat,
-}

components/layout/fragment.rs

@@ -2526,27 +2526,17 @@ impl Fragment {
             return true
         }
 
-        match (self.style().get_box().position,
-               self.style().get_position().z_index,
-               self.style().get_box().overflow_x,
-               self.style().get_box().overflow_y) {
-            (position::T::absolute,
-             Either::Second(Auto),
-             overflow_x::T::visible,
-             overflow_x::T::visible) |
-            (position::T::fixed,
-             Either::Second(Auto),
-             overflow_x::T::visible,
-             overflow_x::T::visible) |
-            (position::T::relative,
-             Either::Second(Auto),
-             overflow_x::T::visible,
-             overflow_x::T::visible) => false,
-            (position::T::absolute, _, _, _) |
-            (position::T::fixed, _, _, _) |
-            (position::T::relative, _, _, _) => true,
-            (position::T::static_, _, _, _) => false
+        // Statically positioned fragments don't establish stacking contexts if the previous
+        // conditions are not fulfilled. Furthermore, z-index doesn't apply to statically
+        // positioned fragments.
+        if self.style().get_box().position == position::T::static_ {
+            return false;
         }
+
+        // For absolutely and relatively positioned fragments we only establish a stacking
+        // context if there is a z-index set.
+        // See https://www.w3.org/TR/CSS2/visuren.html#z-index
+        self.style().get_position().z_index != Either::Second(Auto)
     }
 
     // Get the effective z-index of this fragment. Z-indices only apply to positioned element

tests/wpt/mozilla/meta/MANIFEST.json

    "support"
   ],
   "css/stacked_layers.html": [
-   "db8e8e17e980bbeb6f7f0858be6fbe9aeede378c",
+   "9060686b4edb2bb051e0bd60116800de0a18bb7d",
    "reftest"
   ],
   "css/stacked_layers_ref.html": [
-   "cfc3b330a7672edb48b04b2fbc7ca2c05d2e9cd3",
+   "2e36ba51990322feb0da6c5ee14d329f16037018",
    "support"
   ],
   "css/stacking_context_overflow_a.html": [

tests/wpt/mozilla/tests/css/stacked_layers.html

@@ -74,6 +74,15 @@
             <div class="gray box" style="position: relative; left: 20px; top: -30px;"> </div>
         </div>
 
+        <!-- The child of div with overflow scroll should be painted last since
+             overflow:scroll does not create a stacking context and capture it. -->
+        <div class="test grayest box">
+            <div class="box" style="position: relative; left: 20px; top: 20px; overflow: scroll;">
+                <div class="gray box" style="position: relative; z-index: 3;"></div>
+            </div>
+            <div class="grayer box" style="position: relative; left: 10px; top: -40px; z-index: 2;"></div>
+        </div>
+
         <script>
             function paintCanvas(id) {
                 var canvas = document.getElementById(id);

tests/wpt/mozilla/tests/css/stacked_layers_ref.html

@@ -54,5 +54,10 @@
         <div class="grayer box" style="margin-left: 10px; margin-top: 10px;"></div>
         <div class="gray box" style="margin-left: 20px; margin-top: -40px;"></div>
     </div>
+
+    <div class="test grayest box">
+        <div class="grayer box" style="margin-left: 10px; margin-top: 10px;"></div>
+        <div class="gray box" style="margin-left: 20px; margin-top: -40px;"></div>
+    </div>
 </body>
 </html>