Bug 937053 - tap() method should calculate elementInView from the coordinates of the tap. r=mdas
☠☠ backed out by 313e5dcdfcb8 ☠ ☠
authorDave Hunt <dhunt@mozilla.com>
Thu, 16 Oct 2014 10:32:00 +0200
changeset 237813 9e996cc695eb5d92e4a7014b19c8e71293ce4b77
parent 237812 dccabcc68ff3d792273de0e508d966104f25bbd9
child 237814 776061abb9be3feedade5a31c84a99f869bbab45
push id660
push userraliiev@mozilla.com
push dateWed, 18 Feb 2015 20:30:48 +0000
treeherdermozilla-release@49e493494178 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmdas
bugs937053
milestone36.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 937053 - tap() method should calculate elementInView from the coordinates of the tap. r=mdas
testing/marionette/client/marionette/tests/unit/test_click_scrolling.py
testing/marionette/client/marionette/www/element_outside_viewport.html
testing/marionette/marionette-listener.js
--- a/testing/marionette/client/marionette/tests/unit/test_click_scrolling.py
+++ b/testing/marionette/client/marionette/tests/unit/test_click_scrolling.py
@@ -72,15 +72,40 @@ class TestClickScrolling(MarionetteTestC
 
     def test_should_be_able_to_click_radio_button_scrolled_into_view(self):
         test_html = self.marionette.absolute_url("scroll4.html")
         self.marionette.navigate(test_html)
 
         # If we dont throw we are good
         self.marionette.find_element(By.ID, "radio").click()
 
+    def test_should_scroll_elements_if_click_point_is_out_of_view_but_element_is_in_view(self):
+        test_html = self.marionette.absolute_url("element_outside_viewport.html")
+
+        for s in ["top", "bottom"]:
+            self.marionette.navigate(test_html)
+            scroll_y = self.marionette.execute_script("return window.scrollY;")
+            self.marionette.find_element(By.ID, "%s-70" % s).click()
+            self.assertNotEqual(scroll_y, self.marionette.execute_script("return window.scrollY;"))
+
+        for s in ["left", "right"]:
+            self.marionette.navigate(test_html)
+            scroll_x = self.marionette.execute_script("return window.scrollX;")
+            self.marionette.find_element(By.ID, "%s-70" % s).click()
+            self.assertNotEqual(scroll_x, self.marionette.execute_script("return window.scrollX;"))
+
+    def test_should_not_scroll_elements_if_click_point_is_in_view(self):
+        test_html = self.marionette.absolute_url("element_outside_viewport.html")
+
+        for s in ["top", "right", "bottom", "left"]:
+            for p in ["50", "30"]:
+                self.marionette.navigate(test_html)
+                scroll = self.marionette.execute_script("return [window.scrollX, window.scrollY];")
+                self.marionette.find_element(By.ID, "%s-%s" % (s, p)).click()
+                self.assertEqual(scroll, self.marionette.execute_script("return [window.scrollX, window.scrollY];"))
+
     @skip("Bug 1003687")
     def test_should_scroll_overflow_elements_if_click_point_is_out_of_view_but_element_is_in_view(self):
         test_html = self.marionette.absolute_url("scroll5.html")
         self.marionette.navigate(test_html)
 
         self.marionette.find_element(By.ID, "inner").click()
         self.assertEqual("clicked", self.marionette.find_element(By.ID, "clicked").text)
new file mode 100644
--- /dev/null
+++ b/testing/marionette/client/marionette/www/element_outside_viewport.html
@@ -0,0 +1,41 @@
+<!DOCTYPE html>
+<style>
+  div {
+    position: absolute;
+    width: 100px;
+    height: 100px;
+  }
+  .top { background-color: red; }
+  #top-70 { left: 80px; top: 0; }
+  #top-50 { left: 190px; top: 20px; }
+  #top-30 { left: 300px; top: 40px; }
+
+  .right { background-color: black; }
+  #right-70 { top: 80px; right: -140px;}
+  #right-50 { top: 190px; right: -120px;}
+  #right-30 { top: 300px; right: -100px;}
+
+  .bottom { background-color: blue; }
+  #bottom-70 { right: -50px; bottom: -140px; }
+  #bottom-50 { right: 60px; bottom: -120px; }
+  #bottom-30 { right: 170px; bottom: -100px; }
+
+  .left { background-color: green; }
+  #left-70 { bottom: -50px; left: 0; }
+  #left-50 { bottom: 60px; left: 20px; }
+  #left-30 { bottom: 170px; left: 40px; }
+</style>
+<body onload="window.scrollTo(70, 70);">
+  <div id="top-70" class="top"></div>
+  <div id="top-50" class="top"></div>
+  <div id="top-30" class="top"></div>
+  <div id="right-70" class="right"></div>
+  <div id="right-50" class="right"></div>
+  <div id="right-30" class="right"></div>
+  <div id="bottom-70" class="bottom"></div>
+  <div id="bottom-50" class="bottom"></div>
+  <div id="bottom-30" class="bottom"></div>
+  <div id="left-70" class="left"></div>
+  <div id="left-50" class="left"></div>
+  <div id="left-30" class="left"></div>
+</body>
--- a/testing/marionette/marionette-listener.js
+++ b/testing/marionette/marionette-listener.js
@@ -774,43 +774,48 @@ function coordinates(target, x, y) {
   }
   let coords = {};
   coords.x = box.left + x;
   coords.y = box.top + y;
   return coords;
 }
 
 /**
- * This function returns if the element is in viewport
+ * This function returns true if the given coordinates are in the viewport.
+ * @param 'x', and 'y' are the coordinates relative to the target.
+ *        If they are not specified, then the center of the target is used.
  */
-function elementInViewport(el) {
-  let rect = el.getBoundingClientRect();
+function elementInViewport(el, x, y) {
+  let c = coordinates(el, x, y);
   let viewPort = {top: curFrame.pageYOffset,
                   left: curFrame.pageXOffset,
                   bottom: (curFrame.pageYOffset + curFrame.innerHeight),
                   right:(curFrame.pageXOffset + curFrame.innerWidth)};
-  return (viewPort.left <= rect.right + curFrame.pageXOffset &&
-          rect.left + curFrame.pageXOffset <= viewPort.right &&
-          viewPort.top <= rect.bottom + curFrame.pageYOffset &&
-          rect.top + curFrame.pageYOffset <= viewPort.bottom);
+  return (viewPort.left <= c.x + curFrame.pageXOffset &&
+          c.x + curFrame.pageXOffset <= viewPort.right &&
+          viewPort.top <= c.y + curFrame.pageYOffset &&
+          c.y + curFrame.pageYOffset <= viewPort.bottom);
 }
 
 /**
- * This function throws the visibility of the element error
+ * This function throws the visibility of the element error if the element is
+ * not displayed or the given coordinates are not within the viewport.
+ * @param 'x', and 'y' are the coordinates relative to the target.
+ *        If they are not specified, then the center of the target is used.
  */
-function checkVisible(el) {
+function checkVisible(el, x, y) {
   //check if the element is visible
   let visible = utils.isElementDisplayed(el);
   if (!visible) {
     return false;
   }
   if (el.tagName.toLowerCase() === 'body') {
     return true;
   }
-  if (!elementInViewport(el)) {
+  if (!elementInViewport(el, x, y)) {
     //check if scroll function exist. If so, call it.
     if (el.scrollIntoView) {
       el.scrollIntoView(false);
       if (!elementInViewport(el)) {
         return false;
       }
     }
     else {
@@ -929,24 +934,24 @@ function generateEvents(type, x, y, touc
 /**
  * Function that perform a single tap
  */
 function singleTap(msg) {
   let command_id = msg.json.command_id;
   try {
     let el = elementManager.getKnownElement(msg.json.id, curFrame);
     // after this block, the element will be scrolled into view
-    if (!checkVisible(el)) {
+    if (!checkVisible(el, msg.json.corx, msg.json.cory)) {
        sendError("Element is not currently visible and may not be manipulated", 11, null, command_id);
        return;
     }
     if (!curFrame.document.createTouch) {
       mouseEventsOnly = true;
     }
-    let c = coordinates(el, msg.json.corx, msg.json.cory);
+    c = coordinates(el, msg.json.corx, msg.json.cory);
     generateEvents('tap', c.x, c.y, null, el);
     sendOk(msg.json.command_id);
   }
   catch (e) {
     sendError(e.message, e.code, e.stack, msg.json.command_id);
   }
 }