Auto merge of #16317 - eloycoto:issue6799, r=emilio

Fix #6799: set stacking_context_position correctly on fragment_border_iterator

Hey,

First of all, this is my first PR to Servo project and I'm learning Rust, so sorry if you see something that it's not correct. I did that as best as I know.

This PR fix the issue #6799; I tried all the corner cases that I can think about it and always get the right result and the same as other browsers.

Related to the build:

- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #6799

In the other hand, I added the test in the cssom folder, is where getBoundingClientRect  is defined, so I think that is the best place.

I'm sure that the line 122 can be better, but I didn't find a way to transform a Point2D from f32 to px in a easy way.

I'm here to listen to your recommendations and fix any issue.
Thanks!

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

components/layout/block.rs

@@ -31,7 +31,7 @@ use app_units::{Au, MAX_AU};
 use context::LayoutContext;
 use display_list_builder::{BorderPaintingMode, DisplayListBuildState};
 use display_list_builder::BlockFlowDisplayListBuilding;
-use euclid::{Point2D, Size2D};
+use euclid::{Point2D, Size2D, Rect};
 use floats::{ClearType, FloatKind, Floats, PlacementInfo};
 use flow::{self, BaseFlow, EarlyAbsolutePositionInfo, Flow, FlowClass, ForceNonfloatedFlag};
 use flow::{BLOCK_POSITION_IS_STATIC, CLEARS_LEFT, CLEARS_RIGHT};
@@ -647,6 +647,14 @@ impl BlockFlow {
         &mut self.fragment
     }
 
+    pub fn stacking_relative_position(&self, coor: CoordinateSystem) -> Rect<Au> {
+        return self.fragment.stacking_relative_border_box(
+            &self.base.stacking_relative_position,
+            &self.base.early_absolute_position_info.relative_containing_block_size,
+            self.base.early_absolute_position_info.relative_containing_block_mode,
+            coor);
+    }
+
     /// Return the size of the containing block for the given immediate absolute descendant of this
     /// flow.
     ///

components/layout/display_list_builder.rs

@@ -2208,13 +2208,7 @@ impl BlockFlowDisplayListBuilding for BlockFlow {
         if state.clip_stack.is_empty() {
             return;
         }
-
+        let border_box = self.stacking_relative_position(CoordinateSystem::Parent);
-        let border_box = self.fragment.stacking_relative_border_box(
-            &self.base.stacking_relative_position,
-            &self.base.early_absolute_position_info.relative_containing_block_size,
-            self.base.early_absolute_position_info.relative_containing_block_mode,
-            CoordinateSystem::Parent);
-
         let transform = match self.fragment.transform_matrix(&border_box) {
             Some(transform) => transform,
             None => return,

components/layout/sequential.rs

@@ -12,7 +12,7 @@ use floats::SpeculatedFloatPlacement;
 use flow::{self, Flow, ImmutableFlowUtils, InorderFlowTraversal, MutableFlowUtils};
 use flow::{PostorderFlowTraversal, PreorderFlowTraversal};
 use flow::IS_ABSOLUTELY_POSITIONED;
-use fragment::FragmentBorderBoxIterator;
+use fragment::{FragmentBorderBoxIterator, CoordinateSystem};
 use generated_content::ResolveGeneratedContent;
 use incremental::RelayoutMode;
 use servo_config::opts;
@@ -24,7 +24,7 @@ pub use style::sequential::traverse_dom;
 pub fn resolve_generated_content(root: &mut Flow, layout_context: &LayoutContext) {
     fn doit(flow: &mut Flow, level: u32, traversal: &mut ResolveGeneratedContent) {
         if !traversal.should_process(flow) {
-            return
+            return;
         }
 
         traversal.process(flow, level);
@@ -38,9 +38,7 @@ pub fn resolve_generated_content(root: &mut Flow, layout_context: &LayoutContext
     doit(root, 0, &mut traversal)
 }
 
-pub fn traverse_flow_tree_preorder(root: &mut Flow,
+pub fn traverse_flow_tree_preorder(root: &mut Flow, layout_context: &LayoutContext, relayout_mode: RelayoutMode) {
-                                   layout_context: &LayoutContext,
-                                   relayout_mode: RelayoutMode) {
     fn doit(flow: &mut Flow,
             assign_inline_sizes: AssignISizes,
             assign_block_sizes: AssignBSizes,
@@ -48,7 +46,9 @@ pub fn traverse_flow_tree_preorder(root: &mut Flow,
         // Force reflow children during this traversal. This is needed when we failed
         // the float speculation of a block formatting context and need to fix it.
         if relayout_mode == RelayoutMode::Force {
-            flow::mut_base(flow).restyle_damage.insert(REFLOW_OUT_OF_FLOW | REFLOW);
+            flow::mut_base(flow)
+                .restyle_damage
+                .insert(REFLOW_OUT_OF_FLOW | REFLOW);
         }
 
         if assign_inline_sizes.should_process(flow) {
@@ -65,15 +65,21 @@ pub fn traverse_flow_tree_preorder(root: &mut Flow,
     }
 
     if opts::get().bubble_inline_sizes_separately {
-        let bubble_inline_sizes = BubbleISizes { layout_context: &layout_context };
+        let bubble_inline_sizes = BubbleISizes {
+            layout_context: &layout_context,
+        };
         {
             let root: &mut Flow = root;
             root.traverse_postorder(&bubble_inline_sizes);
         }
     }
 
-    let assign_inline_sizes = AssignISizes { layout_context: &layout_context };
+    let assign_inline_sizes = AssignISizes {
-    let assign_block_sizes  = AssignBSizes { layout_context: &layout_context };
+        layout_context: &layout_context,
+    };
+    let assign_block_sizes = AssignBSizes {
+        layout_context: &layout_context,
+    };
 
     doit(root, assign_inline_sizes, assign_block_sizes, relayout_mode);
 }
@@ -84,13 +90,14 @@ pub fn build_display_list_for_subtree<'a>(flow_root: &mut Flow,
     let mut state = DisplayListBuildState::new(layout_context);
     flow_root.collect_stacking_contexts(&mut state);
 
-    let mut build_display_list = BuildDisplayList { state: state };
+    let mut build_display_list = BuildDisplayList {
+        state: state,
+    };
     build_display_list.traverse(flow_root);
     build_display_list.state
 }
 
-pub fn iterate_through_flow_tree_fragment_border_boxes(root: &mut Flow,
+pub fn iterate_through_flow_tree_fragment_border_boxes(root: &mut Flow, iterator: &mut FragmentBorderBoxIterator) {
-                                                       iterator: &mut FragmentBorderBoxIterator) {
     fn doit(flow: &mut Flow,
             level: i32,
             iterator: &mut FragmentBorderBoxIterator,
@@ -98,15 +105,22 @@ pub fn iterate_through_flow_tree_fragment_border_boxes(root: &mut Flow,
         flow.iterate_through_fragment_border_boxes(iterator, level, stacking_context_position);
 
         for kid in flow::mut_base(flow).child_iter_mut() {
-            let stacking_context_position =
+            let mut stacking_context_position = *stacking_context_position;
-                if kid.is_block_flow() && kid.as_block().fragment.establishes_stacking_context() {
+            if kid.is_block_flow() && kid.as_block().fragment.establishes_stacking_context() {
-                    let margin = Point2D::new(kid.as_block().fragment.margin.inline_start, Au(0));
+                stacking_context_position = Point2D::new(kid.as_block().fragment.margin.inline_start, Au(0)) +
-                    *stacking_context_position + flow::base(kid).stacking_relative_position + margin
+                                            flow::base(kid).stacking_relative_position +
-                } else {
+                                            stacking_context_position;
-                    *stacking_context_position
+                let relative_position = kid.as_block()
-                };
+                    .stacking_relative_position(CoordinateSystem::Own);
-
+                if let Some(matrix) = kid.as_block()
-            // FIXME(#2795): Get the real container size.
+                       .fragment
+                       .transform_matrix(&relative_position) {
+                    let transform_matrix = matrix.transform_point(&Point2D::zero());
+                    stacking_context_position = stacking_context_position +
+                                                Point2D::new(Au::from_f32_px(transform_matrix.x),
+                                                             Au::from_f32_px(transform_matrix.y))
+                }
+            }
             doit(kid, level + 1, iterator, &stacking_context_position);
         }
     }
@@ -116,7 +130,7 @@ pub fn iterate_through_flow_tree_fragment_border_boxes(root: &mut Flow,
 
 pub fn store_overflow(layout_context: &LayoutContext, flow: &mut Flow) {
     if !flow::base(flow).restyle_damage.contains(STORE_OVERFLOW) {
-        return
+        return;
     }
 
     for mut kid in flow::mut_base(flow).child_iter_mut() {
@@ -125,7 +139,9 @@ pub fn store_overflow(layout_context: &LayoutContext, flow: &mut Flow) {
 
     flow.store_overflow(layout_context);
 
-    flow::mut_base(flow).restyle_damage.remove(STORE_OVERFLOW);
+    flow::mut_base(flow)
+        .restyle_damage
+        .remove(STORE_OVERFLOW);
 }
 
 /// Guesses how much inline size will be taken up by floats on the left and right sides of the
@@ -133,7 +149,7 @@ pub fn store_overflow(layout_context: &LayoutContext, flow: &mut Flow) {
 /// contexts. The speculation typically succeeds, but if it doesn't we have to lay it out again.
 pub fn guess_float_placement(flow: &mut Flow) {
     if !flow::base(flow).restyle_damage.intersects(REFLOW) {
-        return
+        return;
     }
 
     let mut floats_in = SpeculatedFloatPlacement::compute_floats_in_for_first_child(flow);

tests/wpt/metadata/MANIFEST.json

@@ -321172,6 +321172,12 @@
      {}
     ]
    ],
+   "cssom/GetBoundingRect.html": [
+    [
+     "/cssom/GetBoundingRect.html",
+     {}
+    ]
+   ],
    "cssom/MediaList.html": [
     [
      "/cssom/MediaList.html",
@@ -553765,6 +553771,10 @@
    "f0d47464da9d30e70733f09af78f3e9f982c4406",
    "testharness"
   ],
+  "cssom/GetBoundingRect.html": [
+   "7e5a8b25753ac970c2d192376c9dd93943b3dbb5",
+   "testharness"
+  ],
   "cssom/MediaList.html": [
    "21d9e43514fb3a7fbf8933429242dc544224ef24",
    "testharness"

tests/wpt/web-platform-tests/cssom/GetBoundingRect.html

@@ -0,0 +1,30 @@
+<!doctype html>
+<html>
+  <head>
+    <meta charset="utf-8">
+    <title>getBoundingClientRect</title>
+    <script src="/resources/testharness.js"></script>
+    <script src="/resources/testharnessreport.js"></script>
+
+    <style>
+      #foo {
+        margin: 0px 0px 0px 5px;
+        transform: translate(10px, 200px);
+        position: fixed;
+        left: 5px;
+        background-color: red;
+      }
+    </style>
+  </head>
+  <body>
+    <div id="foo">
+      FOO
+    </div>
+    <script>
+        test(function () {
+            var foo = document.getElementById("foo").getBoundingClientRect();
+            assert_equals(foo.left, 20);
+        });
+    </script>
+  </body>
+</html>