Auto merge of #25715 - mrobinson:position-fixed, r=SimonSapin

Improve position:fixed support in layout_2020

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #___ (GitHub issue number if applicable)

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

components/layout_2020/display_list/mod.rs

@@ -14,8 +14,10 @@ use mitochondria::OnceCell;
 use net_traits::image_cache::UsePlaceholder;
 use std::sync::Arc;
 use style::computed_values::overflow_x::T as ComputedOverflow;
+use style::computed_values::position::T as ComputedPosition;
 use style::dom::OpaqueNode;
 use style::properties::ComputedValues;
+
 use style::values::computed::{BorderStyle, Length, LengthPercentage};
 use style::values::specified::ui::CursorKind;
 use webrender_api::{self as wr, units};
@@ -244,51 +246,15 @@ impl<'a> BuilderForBoxFragment<'a> {
     }
 
     fn build(&mut self, builder: &mut DisplayListBuilder) {
-        let hit_info = hit_info(&self.fragment.style, self.fragment.tag, Cursor::Default);
-        if hit_info.is_some() {
-            let mut common = builder.common_properties(self.border_rect);
-            common.hit_info = hit_info;
-            if let Some(clip_id) = self.border_edge_clip(builder) {
-                common.clip_id = clip_id
-            }
-            builder.wr.push_hit_test(&common)
-        }
-
-        self.build_background(builder);
-        self.build_border(builder);
-
         builder.clipping_and_scrolling_scope(|builder| {
-            let overflow_x = self.fragment.style.get_box().overflow_x;
+            self.adjust_spatial_id_for_positioning(builder);
-            let overflow_y = self.fragment.style.get_box().overflow_y;
+            self.build_hit_test(builder);
-            let original_scroll_and_clip_info = builder.current_space_and_clip;
+            self.build_background(builder);
-            if overflow_x != ComputedOverflow::Visible || overflow_y != ComputedOverflow::Visible {
+            self.build_border(builder);
-                // TODO(mrobinson): We should use the correct fragment type, once we generate
-                // fragments from ::before and ::after generated content selectors.
-                let id = combine_id_with_fragment_type(
-                    self.fragment.tag.id() as usize,
-                    FragmentType::FragmentBody,
-                ) as u64;
-                let external_id = wr::ExternalScrollId(id, builder.wr.pipeline_id);
 
-                let sensitivity = if ComputedOverflow::Hidden == overflow_x &&
+            // We want to build the scroll frame after the background and border, because
-                    ComputedOverflow::Hidden == overflow_y
+            // they shouldn't scroll with the rest of the box content.
-                {
+            self.build_scroll_frame_if_necessary(builder);
-                    wr::ScrollSensitivity::Script
-                } else {
-                    wr::ScrollSensitivity::ScriptAndInputEvents
-                };
-
-                builder.current_space_and_clip = builder.wr.define_scroll_frame(
-                    &original_scroll_and_clip_info,
-                    Some(external_id),
-                    self.fragment.scrollable_overflow().to_webrender(),
-                    *self.padding_rect(),
-                    vec![], // complex_clips
-                    None,   // image_mask
-                    sensitivity,
-                    wr::units::LayoutVector2D::zero(),
-                );
-            }
 
             let content_rect = self
                 .fragment
@@ -301,6 +267,64 @@ impl<'a> BuilderForBoxFragment<'a> {
         });
     }
 
+    fn adjust_spatial_id_for_positioning(&self, builder: &mut DisplayListBuilder) {
+        if self.fragment.style.get_box().position != ComputedPosition::Fixed {
+            return;
+        }
+
+        // TODO(mrobinson): Eventually this should use the spatial id of the reference
+        // frame that is the parent of this one once we have full support for stacking
+        // contexts and transforms.
+        builder.current_space_and_clip.spatial_id =
+            wr::SpatialId::root_reference_frame(builder.wr.pipeline_id);
+    }
+
+    fn build_scroll_frame_if_necessary(&self, builder: &mut DisplayListBuilder) {
+        let overflow_x = self.fragment.style.get_box().overflow_x;
+        let overflow_y = self.fragment.style.get_box().overflow_y;
+        let original_scroll_and_clip_info = builder.current_space_and_clip;
+        if overflow_x != ComputedOverflow::Visible || overflow_y != ComputedOverflow::Visible {
+            // TODO(mrobinson): We should use the correct fragment type, once we generate
+            // fragments from ::before and ::after generated content selectors.
+            let id = combine_id_with_fragment_type(
+                self.fragment.tag.id() as usize,
+                FragmentType::FragmentBody,
+            ) as u64;
+            let external_id = wr::ExternalScrollId(id, builder.wr.pipeline_id);
+
+            let sensitivity = if ComputedOverflow::Hidden == overflow_x &&
+                ComputedOverflow::Hidden == overflow_y
+            {
+                wr::ScrollSensitivity::Script
+            } else {
+                wr::ScrollSensitivity::ScriptAndInputEvents
+            };
+
+            builder.current_space_and_clip = builder.wr.define_scroll_frame(
+                &original_scroll_and_clip_info,
+                Some(external_id),
+                self.fragment.scrollable_overflow().to_webrender(),
+                *self.padding_rect(),
+                vec![], // complex_clips
+                None,   // image_mask
+                sensitivity,
+                wr::units::LayoutVector2D::zero(),
+            );
+        }
+    }
+
+    fn build_hit_test(&self, builder: &mut DisplayListBuilder) {
+        let hit_info = hit_info(&self.fragment.style, self.fragment.tag, Cursor::Default);
+        if hit_info.is_some() {
+            let mut common = builder.common_properties(self.border_rect);
+            common.hit_info = hit_info;
+            if let Some(clip_id) = self.border_edge_clip(builder) {
+                common.clip_id = clip_id
+            }
+            builder.wr.push_hit_test(&common)
+        }
+    }
+
     fn build_background(&mut self, builder: &mut DisplayListBuilder) {
         use style::values::computed::image::{Image, ImageLayer};
         let b = self.fragment.style.get_background();

tests/wpt/mozilla/meta/MANIFEST.json

@@ -1,5 +1,14 @@
 {
  "items": {
+  "conformancechecker": {
+   "css/position_fixed_scroll.html": []
+  },
+  "crashtest": {
+   "css/position_fixed_scroll.html": []
+  },
+  "manual": {
+   "css/position_fixed_scroll.html": []
+  },
   "reftest": {
    "css/abs-overflow-stackingcontext.html": [
     [
@@ -5057,6 +5066,18 @@
      {}
     ]
    ],
+   "css/position_fixed_scroll.html": [
+    [
+     "css/position_fixed_scroll.html",
+     [
+      [
+       "/_mozilla/css/position_fixed_scroll_ref.html",
+       "=="
+      ]
+     ],
+     {}
+    ]
+   ],
    "css/position_fixed_simple_a.html": [
     [
      "css/position_fixed_simple_a.html",
@@ -8977,6 +8998,10 @@
    "css/position_fixed_overflow_b.html": [
     []
    ],
+   "css/position_fixed_scroll.html": [],
+   "css/position_fixed_scroll_ref.html": [
+    []
+   ],
    "css/position_fixed_simple_b.html": [
     []
    ],
@@ -11253,6 +11278,7 @@
      {}
     ]
    ],
+   "css/position_fixed_scroll.html": [],
    "css/stylesheet_media_queries.html": [
     [
      "css/stylesheet_media_queries.html",
@@ -12596,6 +12622,12 @@
      {}
     ]
    ]
+  },
+  "visual": {
+   "css/position_fixed_scroll.html": []
+  },
+  "wdspec": {
+   "css/position_fixed_scroll.html": []
   }
  },
  "paths": {
@@ -16959,6 +16991,14 @@
    "a8947566153c187ba3191084f89d0163bc5b666e",
    "support"
   ],
+  "css/position_fixed_scroll.html": [
+   "712265957f2c2ff0102eab09219b44bc5bf7f032",
+   "reftest"
+  ],
+  "css/position_fixed_scroll_ref.html": [
+   "dc2e72a96ac2d82f038605812c79b9f78c1547dd",
+   "support"
+  ],
   "css/position_fixed_simple_a.html": [
    "d01b955091107990d014e7f7be9c1181e5b06d66",
    "reftest"

tests/wpt/mozilla/tests/css/position_fixed_scroll.html

@@ -0,0 +1,17 @@
+<html>
+  <head>
+    <link rel=match href=position_fixed_scroll_ref.html>
+      <style>
+        body {
+            margin: 0;
+        }
+      </style>
+  </head>
+  <body>
+      <div style="height: 100px; width: 100px; background: green; position: fixed;"></div>
+      <div style="height: 5000px;"></div>
+      <script>
+           window.scrollTo(0, 100);
+      </script>
+  </body>
+</html>

tests/wpt/mozilla/tests/css/position_fixed_scroll_ref.html

@@ -0,0 +1,12 @@
+<html>
+  <head>
+      <style>
+        body {
+            margin: 0;
+        }
+      </style>
+  </head>
+  <body>
+      <div style="height: 100px; width: 100px; background: green;"></div>
+  </body>
+</html>