Auto merge of #15047 - emilio:click-regression, r=notriddle,emilio

gfx: Don't mutate the same point multiple times while translating it …

…to other stacking context space.

I don't know how neither review or a test caught this.

r? @pcwalton  or @notriddle

Fixes #15015

<!-- 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/15047)
<!-- Reviewable:end -->

components/gfx/display_list/mod.rs

@@ -55,11 +55,10 @@ impl DisplayList {
                       scroll_offsets: &ScrollOffsetMap)
                       -> Option<usize> {
         let mut result = Vec::new();
-        let mut translated_point = client_point.clone();
         let mut traversal = DisplayListTraversal::new(self);
         self.text_index_contents(node,
                                  &mut traversal,
-                                 &mut translated_point,
+                                 client_point,
                                  client_point,
                                  scroll_offsets,
                                  &mut result);
@@ -69,30 +68,32 @@ impl DisplayList {
     pub fn text_index_contents<'a>(&self,
                                    node: OpaqueNode,
                                    traversal: &mut DisplayListTraversal<'a>,
-                                   translated_point: &mut Point2D<Au>,
+                                   translated_point: &Point2D<Au>,
                                    client_point: &Point2D<Au>,
                                    scroll_offsets: &ScrollOffsetMap,
                                    result: &mut Vec<usize>) {
         while let Some(item) = traversal.next() {
             match item {
                 &DisplayItem::PushStackingContext(ref stacking_context_item) => {
+                    let mut point = *translated_point;
                     DisplayList::translate_point(&stacking_context_item.stacking_context,
-                                                 translated_point,
+                                                 &mut point,
                                                  client_point);
                     self.text_index_contents(node,
                                              traversal,
-                                             translated_point,
+                                             &point,
                                              client_point,
                                              scroll_offsets,
                                              result);
                 }
                 &DisplayItem::PushScrollRoot(ref item) => {
+                    let mut point = *translated_point;
                     DisplayList::scroll_root(&item.scroll_root,
-                                             translated_point,
+                                             &mut point,
                                              scroll_offsets);
                     self.text_index_contents(node,
                                              traversal,
-                                             translated_point,
+                                             &point,
                                              client_point,
                                              scroll_offsets,
                                              result);
@@ -115,7 +116,7 @@ impl DisplayList {
     // Return all nodes containing the point of interest, bottommost first, and
     // respecting the `pointer-events` CSS property.
     pub fn hit_test(&self,
-                    translated_point: &mut Point2D<Au>,
+                    translated_point: &Point2D<Au>,
                     client_point: &Point2D<Au>,
                     scroll_offsets: &ScrollOffsetMap)
                     -> Vec<DisplayItemMetadata> {
@@ -131,28 +132,30 @@ impl DisplayList {
 
     pub fn hit_test_contents<'a>(&self,
                                  traversal: &mut DisplayListTraversal<'a>,
-                                 translated_point: &mut Point2D<Au>,
+                                 translated_point: &Point2D<Au>,
                                  client_point: &Point2D<Au>,
                                  scroll_offsets: &ScrollOffsetMap,
                                  result: &mut Vec<DisplayItemMetadata>) {
         while let Some(item) = traversal.next() {
             match item {
                 &DisplayItem::PushStackingContext(ref stacking_context_item) => {
+                    let mut point = *translated_point;
                     DisplayList::translate_point(&stacking_context_item.stacking_context,
-                                                 translated_point,
+                                                 &mut point,
                                                  client_point);
                     self.hit_test_contents(traversal,
-                                           translated_point,
+                                           &point,
                                            client_point,
                                            scroll_offsets,
                                            result);
                 }
                 &DisplayItem::PushScrollRoot(ref item) => {
+                    let mut point = *translated_point;
                     DisplayList::scroll_root(&item.scroll_root,
-                                             translated_point,
+                                             &mut point,
                                              scroll_offsets);
                     self.hit_test_contents(traversal,
-                                           translated_point,
+                                           &point,
                                            client_point,
                                            scroll_offsets,
                                            result);

components/layout/query.rs

@@ -141,7 +141,7 @@ impl LayoutRPC for LayoutRPCImpl {
     fn nodes_from_point(&self,
                         page_point: Point2D<f32>,
                         client_point: Point2D<f32>) -> Vec<UntrustedNodeAddress> {
-        let mut page_point = Point2D::new(Au::from_f32_px(page_point.x),
+        let page_point = Point2D::new(Au::from_f32_px(page_point.x),
                                       Au::from_f32_px(page_point.y));
         let client_point = Point2D::new(Au::from_f32_px(client_point.x),
                                         Au::from_f32_px(client_point.y));
@@ -152,7 +152,7 @@ impl LayoutRPC for LayoutRPCImpl {
             let result = match rw_data.display_list {
                 None => panic!("Tried to hit test without a DisplayList"),
                 Some(ref display_list) => {
-                    display_list.hit_test(&mut page_point,
+                    display_list.hit_test(&page_point,
                                           &client_point,
                                           &rw_data.stacking_context_scroll_offsets)
                 }

tests/wpt/mozilla/meta/MANIFEST.json

@@ -8432,6 +8432,12 @@
             "url": "/_mozilla/mozilla/globals/entry.worker.html"
           }
         ],
+        "mozilla/hit_test_multiple_sc.html": [
+          {
+            "path": "mozilla/hit_test_multiple_sc.html",
+            "url": "/_mozilla/mozilla/hit_test_multiple_sc.html"
+          }
+        ],
         "mozilla/hit_test_nested_sc.html": [
           {
             "path": "mozilla/hit_test_nested_sc.html",

tests/wpt/mozilla/tests/mozilla/hit_test_multiple_sc.html

@@ -0,0 +1,27 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>Hit testing works for following stacking contexts</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<style>
+  .stacking-context {
+    z-index: 100;
+    opacity: 0.99;
+    width: 100px;
+    height: 100px;
+    background: green;
+  }
+</style>
+<div class="stacking-context"></div>
+<div class="stacking-context" id="second-sc"></div>
+<script>
+async_test(function(t) {
+  window.onload = t.step_func(function() {
+    var points = document.elementsFromPoint(50, 150);
+    assert_true(points.length > 0, "Should report at least one element");
+    assert_true(points[0].id === "second-sc",
+                "Should be the second stacking context");
+    t.done();
+  })
+});
+</script>