Changeset 132929 in webkit

Timestamp:

Oct 30, 2012 11:44:42 AM (11 years ago)

Author:

commit-queue@webkit.org

Message:

[touchadjustment] touch-links-longpress tests passes incorrectly
​https://bugs.webkit.org/show_bug.cgi?id=96810

Patch by Rick Byers <​rbyers@chromium.org> on 2012-10-30
Reviewed by Antonio Gomes.

Tools:

Add support for suppling width/height information for longpress
gesture (as for other gestures like tapDown) for the purposes
of touch adjustment.

• DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:

(WebTestRunner):
(WebTestRunner::EventSender::gestureEvent):

LayoutTests:

Fix touch-links-longpress test to be consistent with touch-links-active.
Now it's actually touching where we intended - at the center of the
touch region identified instead of the top/left corner. Also pass
the width/height of the touch to EventSender in order to enable touch
adjustment.

Also adds checks to verify that our touches are actually landing on
or outside the element we're targetting as desired.

• touchadjustment/resources/touchadjustment.js:

(findAbsoluteBounds): Update to support both inline and block nodes (clientHeight is 0 for inline nodes)

• touchadjustment/touch-links-active.html: Use updated findAbsoluteBounds
• touchadjustment/touch-links-longpress.html:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/qt/TestExpectations (modified) (1 diff)
• LayoutTests/touchadjustment/resources/touchadjustment.js (modified) (1 diff)
• LayoutTests/touchadjustment/touch-links-active.html (modified) (2 diffs)
• LayoutTests/touchadjustment/touch-links-longpress.html (modified) (3 diffs)
• Tools/ChangeLog (modified) (1 diff)
• Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2012-10-30  Rick Byers  <rbyers@chromium.org>
+
+        [touchadjustment] touch-links-longpress tests passes incorrectly
+        https://bugs.webkit.org/show_bug.cgi?id=96810
+
+        Reviewed by Antonio Gomes.
+
+        Fix touch-links-longpress test to be consistent with touch-links-active.
+        Now it's actually touching where we intended - at the center of the
+        touch region identified instead of the top/left corner.  Also pass
+        the width/height of the touch to EventSender in order to enable touch
+        adjustment.
+
+        Also adds checks to verify that our touches are actually landing on
+        or outside the element we're targetting as desired.
+
+        * touchadjustment/resources/touchadjustment.js:
+        (findAbsoluteBounds): Update to support both inline and block nodes (clientHeight is 0 for inline nodes)
+        * touchadjustment/touch-links-active.html: Use updated findAbsoluteBounds
+        * touchadjustment/touch-links-longpress.html:
+
 2012-10-30  Dan Bernstein  <mitz@apple.com>
 

trunk/LayoutTests/platform/qt/TestExpectations

@@ -449 +449 @@
 touchadjustment/touch-links-active.html
 
+# Don't have width/height on GestureLongPress necessary to enable adjustment
+touchadjustment/touch-links-longpress.html
+
 # ENABLE(HIDDEN_PAGE_DOM_TIMER_THROTTLING) is disabled
 fast/dom/timer-throttling-hidden-page.html

trunk/LayoutTests/touchadjustment/resources/touchadjustment.js

@@ -1 @@
-/* function for finding the absolute bounds of a node */
+/* function for finding the absolute bounds of a node (both inline and block) */
 function findAbsoluteBounds(node)
 {
-    var bounds = {left: 0, top: 0};
-    bounds.width = node.clientWidth;
-    bounds.height = node.clientHeight;
-    do {
-        bounds.left += node.offsetLeft;
-        bounds.top += node.offsetTop;
-    } while (node = node.offsetParent);
-    return bounds;
+    var bounds = node.getBoundingClientRect();
+    return {
+        left: bounds.left,
+        top: bounds.top,
+        width: bounds.right - bounds.left,
+        height: bounds.bottom - bounds.top
+    };
 }
 

trunk/LayoutTests/touchadjustment/touch-links-active.html

@@ -64 +64 @@
     }
 
-    // FIXME: Why doesn't the approach in findAbsoluteBounds work correctly here?
-    // I'll investigate/fix along with http://wkb.ug/96810
-    function getBounds(node)
-    {
-      var bounds = node.getBoundingClientRect();
-      return {
-          left: bounds.left,
-          top: bounds.top,
-          width: bounds.right - bounds.left,
-          height: bounds.bottom - bounds.top
-      };    
-    }
-    
     function testDirectTouch(element)
     {
         // Touch directly in the center of the element.
-        var touchpoint = offsetTouchPoint(getBounds(element), 'center', 0, 20, 30);
+        var touchpoint = offsetTouchPoint(findAbsoluteBounds(element), 'center', 0, 20, 30);
         if (document.elementFromPoint(touchpoint.x, touchpoint.y) != element)
             testFailed('Direct touch ended up on some other element');
@@ -100 +87 @@
     {
         // Touch just right of the element.
-        var touchpoint = offsetTouchPoint(getBounds(element), 'right', 10, 30, 20);
+        var touchpoint = offsetTouchPoint(findAbsoluteBounds(element), 'right', 10, 30, 20);
         if (isDescendantOf(element, document.elementFromPoint(touchpoint.x, touchpoint.y)))
             testFailed('Indirect touch ended up still on top of the element');

trunk/LayoutTests/touchadjustment/touch-links-longpress.html

@@ -37 +37 @@
     {
         if (eventSender.gestureLongPress)
-            eventSender.gestureLongPress(touchpoint.left, touchpoint.top);
+            eventSender.gestureLongPress(touchpoint.x, touchpoint.y, touchpoint.width, touchpoint.height);
         else
             debug("gestureLongPress not implemented by this platform.");
@@ -46 +46 @@
         // Touch directly in the center of the element.
         var touchpoint = offsetTouchPoint(findAbsoluteBounds(element), 'center', 0, 20, 30);
+        if (document.elementFromPoint(touchpoint.x, touchpoint.y) != element)
+            testFailed('Direct touch ended up on some other element');
         testLongPress(touchpoint);
     }
@@ -53 +55 @@
         // Touch just right of the element.
         var touchpoint = offsetTouchPoint(findAbsoluteBounds(element), 'right', 10, 30, 20);
+        if (isDescendantOf(element, document.elementFromPoint(touchpoint.x, touchpoint.y)))
+            testFailed('Indirect touch ended up still on top of the element');
         testLongPress(touchpoint);
+    }
+
+    function isDescendantOf(parent, child)
+    {
+        var n = child;
+        while(n) {
+            if(n==parent)
+                return true;
+            n = n.parentNode;
+        }
+        return false;
     }
 

trunk/Tools/ChangeLog

@@ +1 @@
+2012-10-30  Rick Byers  <rbyers@chromium.org>
+
+        [touchadjustment] touch-links-longpress tests passes incorrectly
+        https://bugs.webkit.org/show_bug.cgi?id=96810
+
+        Reviewed by Antonio Gomes.
+
+        Add support for suppling width/height information for longpress
+        gesture (as for other gestures like tapDown) for the purposes
+        of touch adjustment.
+
+        * DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:
+        (WebTestRunner):
+        (WebTestRunner::EventSender::gestureEvent):
+
 2012-10-29  Anders Carlsson  <andersca@apple.com>
 

trunk/Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp

@@ -1201 +1201 @@
         event.x = point.x;
         event.y = point.y;
+        if (arguments.size() >= 4) {
+            event.data.tapDown.width = static_cast<float>(arguments[2].toDouble());
+            event.data.tapDown.height = static_cast<float>(arguments[3].toDouble());
+        }
         break;
     case WebInputEvent::GestureTwoFingerTap: