Merge pull request #742 from finn-no/scroll-touch-fix

Fixed problems with touch events on a scrollable page

lib/OpenLayers/Events.js

@@ -903,7 +903,7 @@ OpenLayers.Events = OpenLayers.Class({
             var num = touches.length;
             var touch;
             for (var i=0; i<num; ++i) {
-                touch = touches[i];
+                touch = this.getTouchClientXY(touches[i]);
                 x += touch.clientX;
                 y += touch.clientY;
             }
@@ -916,6 +916,47 @@ OpenLayers.Events = OpenLayers.Class({
         this.triggerEvent(type, evt);
     },
     
+    /**
+     * Method: getTouchClientXY
+     * WebKit has a few bugs for clientX/clientY. This method detects them
+     * and calculate the correct values.
+     *
+     * Parameters:
+     * evt - {Touch} a Touch object from a TouchEvent
+     * 
+     * Returns:
+     * {Object} An object with only clientX and clientY properties with the
+     * calculated values.
+     */
+    getTouchClientXY: function (evt) {
+        // olMochWin is to override window, used for testing
+        var win = window.olMockWin || window,
+            winPageX = win.pageXOffset,
+            winPageY = win.pageYOffset,
+            x = evt.clientX,
+            y = evt.clientY;
+        
+        if (evt.pageY === 0 && Math.floor(y) > Math.floor(evt.pageY) ||
+            evt.pageX === 0 && Math.floor(x) > Math.floor(evt.pageX)) {
+            // iOS4 include scroll offset in clientX/Y
+            x = x - winPageX;
+            y = y - winPageY;
+        } else if (y < (evt.pageY - winPageY) || x < (evt.pageX - winPageX) ) {
+            // Some Android browsers have totally bogus values for clientX/Y
+            // when scrolling/zooming a page
+            x = evt.pageX - winPageX;
+            y = evt.pageY - winPageY;
+        }
+        
+        evt.olClientX = x;
+        evt.olClientY = y;
+        
+        return {
+            clientX: x,
+            clientY: y
+        };
+    },
+    
     /**
      * APIMethod: clearMouseCache
      * Clear cached data about the mouse position. This should be called any 
@@ -925,17 +966,7 @@ OpenLayers.Events = OpenLayers.Class({
     clearMouseCache: function() { 
         this.element.scrolls = null;
         this.element.lefttop = null;
-        // OpenLayers.Util.pagePosition needs to use
+        this.element.offsets = null;
-        // element.getBoundingClientRect to correctly calculate the offsets
-        // for the iPhone, but once the page is scrolled, getBoundingClientRect
-        // returns incorrect offsets. So our best bet is to not invalidate the
-        // offsets once we have them, and hope that the page was not scrolled
-        // when we did the initial calculation.
-        var body = document.body;
-        if (body && !((body.scrollTop != 0 || body.scrollLeft != 0) &&
-                                    navigator.userAgent.match(/iPhone/i))) {
-            this.element.offsets = null;
-        }
     },      
 
     /**
@@ -959,8 +990,8 @@ OpenLayers.Events = OpenLayers.Class({
         if (!this.element.scrolls) {
             var viewportElement = OpenLayers.Util.getViewportElement();
             this.element.scrolls = [
-                viewportElement.scrollLeft,
+                window.pageXOffset || viewportElement.scrollLeft,
-                viewportElement.scrollTop
+                window.pageYOffset || viewportElement.scrollTop
             ];
         }
 

lib/OpenLayers/Handler/Click.js

@@ -495,8 +495,8 @@ OpenLayers.Handler.Click = OpenLayers.Class(OpenLayers.Handler, {
             for (var i=0; i<len; i++) {
                 touch = evt.touches[i];
                 touches[i] = {
-                    clientX: touch.clientX,
+                    clientX: touch.olClientX,
-                    clientY: touch.clientY
+                    clientY: touch.olClientY
                 };
             }
         }

lib/OpenLayers/Handler/Pinch.js

@@ -97,6 +97,10 @@ OpenLayers.Handler.Pinch = OpenLayers.Class(OpenLayers.Handler, {
             };
             this.callback("start", [evt, this.start]);
             propagate = !this.stopDown;
+        } else if (this.started) {
+            // Some webkit versions send fake single-touch events during
+            // multitouch, which cause the drag handler to trigger
+            return false;
         } else {
             this.started = false;
             this.start = null;
@@ -125,6 +129,11 @@ OpenLayers.Handler.Pinch = OpenLayers.Class(OpenLayers.Handler, {
             this.last = current;
             // prevent document dragging
             OpenLayers.Event.stop(evt);
+            return false;
+        } else if (this.started) {
+            // Some webkit versions send fake single-touch events during
+            // multitouch, which cause the drag handler to trigger
+            return false;
         }
         return true;
     },
@@ -140,12 +149,13 @@ OpenLayers.Handler.Pinch = OpenLayers.Class(OpenLayers.Handler, {
      * {Boolean} Let the event propagate.
      */
     touchend: function(evt) {
-        if (this.started) {
+        if (this.started && !OpenLayers.Event.isMultiTouch(evt)) {
             this.started = false;
             this.pinching = false;
             this.callback("done", [evt, this.start, this.last]);
             this.start = null;
             this.last = null;
+            return false;
         }
         return true;
     },
@@ -199,8 +209,8 @@ OpenLayers.Handler.Pinch = OpenLayers.Class(OpenLayers.Handler, {
         var t0 = touches[0];
         var t1 = touches[1];
         return Math.sqrt(
-            Math.pow(t0.clientX - t1.clientX, 2) +
+            Math.pow(t0.olClientX - t1.olClientX, 2) +
-            Math.pow(t0.clientY - t1.clientY, 2)
+            Math.pow(t0.olClientY - t1.olClientY, 2)
         );
     },
 

lib/OpenLayers/Util.js

@@ -1195,8 +1195,8 @@ OpenLayers.Util.pagePosition =  function(forElement) {
 
     if (forElement.getBoundingClientRect) { // IE
         box = forElement.getBoundingClientRect();
-        var scrollTop = viewportElement.scrollTop;
+        var scrollTop = window.pageYOffset || viewportElement.scrollTop;
-        var scrollLeft = viewportElement.scrollLeft;
+        var scrollLeft = window.pageXOffset || viewportElement.scrollLeft;
         
         pos[0] = box.left + scrollLeft;
         pos[1] = box.top + scrollTop;

tests/Events.html

@@ -300,7 +300,7 @@
     }
 
     function test_Events_handleBrowserEvent(t) {
-        t.plan(2);
+        t.plan(8);
         var events = new OpenLayers.Events({}, null);
         events.on({'sometouchevent': function() {}});
 
@@ -312,6 +312,57 @@
         events.handleBrowserEvent(evt);
         t.eq(evt.clientX, 1.5, "evt.clientX value is correct");
         t.eq(evt.clientY, 1.5, "evt.clientY value is correct");
+        
+        // test bug where clientX/clientY includes scroll offset
+        window.olMockWin = {
+            pageXOffset: 10,
+            pageYOffset: 20
+        };
+        evt = {type: 'sometouchevent',
+               touches: [{
+                   clientX: 11,
+                   clientY: 21,
+                   pageX: 0,
+                   pageY: 0
+               }]
+              };
+        events.handleBrowserEvent(evt);
+        t.eq(evt.clientX, 1, "evt.clientX value is correct");
+        t.eq(evt.clientY, 1, "evt.clientY value is correct");
+        
+        
+        // test bug where clientX/clientY have negative values
+        evt = {
+            type: 'sometouchevent',
+            touches: [{
+                clientX: -412,
+                clientY: -1005,
+                pageX: 11,
+                pageY: 21
+            }]
+        };
+        events.handleBrowserEvent(evt);
+        t.eq(evt.clientX, 1, "evt.clientX value is correct");
+        t.eq(evt.clientY, 1, "evt.clientY value is correct");
+        
+        window.olMockWin = {
+            pageXOffset: 11,
+            pageYOffset: 299
+        };
+        evt = {
+            type: 'sometouchevent',
+                touches: [{
+                clientX: 223,
+                clientY: 119,
+                pageX: 242,
+                pageY: 623
+            }]
+        };
+        events.handleBrowserEvent(evt);
+        t.eq(evt.clientX, 231, "evt.clientX value is correct");
+        t.eq(evt.clientY, 324, "evt.clientY value is correct");
+        
+        window.olMockWin = undefined;
     }
     
     function test_Events_attachToElement(t) {

tests/Handler/Pinch.html

@@ -95,7 +95,7 @@
     }
 
     function test_callbacks(t) {
-        t.plan(23);
+        t.plan(32);
 
         var map = new OpenLayers.Map('map', {controls: []});
 
@@ -105,7 +105,7 @@
         // set fake values for touches
         var testEvents = {
             start: {
-                type: 'start',
+                type: 'touchstart',
                 touches: [{
                     clientX: 100,
                     clientY: 0
@@ -115,7 +115,7 @@
                 }]
             },
             move: {
-                type: 'move',
+                type: 'touchmove',
                 touches: [{
                     clientX: 100,
                     clientY: 0
@@ -125,7 +125,7 @@
                 }]
             },
             done: {
-                type: 'done',
+                type: 'touchend',
                 touches: []
             }
         };
@@ -133,7 +133,8 @@
         // set callback methods
         var customCb = OpenLayers.Function.False;
         var cb = function(evt) {
-            var tch = testEvents[evt.type].touches;
+            var callback = evt.type.replace("touch", "").replace("end", "done");;
+            var tch = testEvents[callback].touches;
             t.ok(evt.touches[0].clientX == tch[0].clientX &&
                 evt.touches[0].clientY == tch[0].clientY,
                 "touchstart sets first touch position correctly in evt");
@@ -147,7 +148,9 @@
         var callbacks = {
             start: cb,
             move: cb,
-            done: customCb
+            done: function () {
+                customCb.apply(this, arguments);
+            }
         };
 
         var handler = new OpenLayers.Handler.Pinch(control, callbacks);
@@ -160,6 +163,14 @@
         OpenLayers.Event.isMultiTouch = function() {
             return false;
         }
+        
+        // no callbacks with tests expected (pinch not started)
+        map.events.handleBrowserEvent(testEvents.start);
+        // test 1, 2, 3
+        t.ok(!handler.started, "1) touchstart (singletouch) sets started to false");
+        t.eq(handler.start, null, "1) touchstart (singletouch) sets start to null");
+        t.eq(handler.last, null, "1) touchstart (singletouch) sets last to null");
+        
         handler.started = true;
         handler.start = {
             distance: 100,
@@ -171,10 +182,13 @@
             delta: 10,
             scale: 1.5
         };
-        map.events.triggerEvent("touchstart", testEvents.start);
+        
-        t.ok(!handler.started, "1) touchstart (singletouch) sets started to false");
+        // no callbacks with tests expected (multitouch pinch started, so ignores singletouch)
-        t.eq(handler.start, null, "1) touchstart (singletouch) sets start to null");
+        map.events.handleBrowserEvent(testEvents.start);
-        t.eq(handler.last, null, "1) touchstart (singletouch) sets last to null");
+        // test 4, 5, 6
+        t.ok(handler.started, "1) touchstart (singletouch) after pinch started is ignored");
+        t.ok(!!handler.start, "1) touchstart (singletouch) after pinch started is ignored");
+        t.ok(!!handler.last, "1) touchstart (singletouch) after pinch started is ignored");
 
         OpenLayers.Event.stop = function(evt, allowDefault) {
             if(allowDefault) {
@@ -192,7 +206,9 @@
             t.eq(pinchdata.delta, 0, "2) calculated delta is correct");
             t.eq(pinchdata.scale, 1, "2) calculated scale is correct");
         }
-        map.events.triggerEvent("touchstart", testEvents.start);
+        // test 7, 8, 9, 10, 11, 12, 13
+        map.events.handleBrowserEvent(testEvents.start);
+        // test 14, 15
         t.ok(handler.started, "2) touchstart sets the started flag to true");
         t.ok(!handler.pinching, "2) touchstart sets the pinching flag to false");
 
@@ -201,10 +217,13 @@
             t.eq(pinchdata.delta, 20, "3) calculated delta is correct");
             t.eq(pinchdata.scale, 0.8, "3) calculated scale is correct");
         }
-        map.events.triggerEvent("touchmove", testEvents.move);
+        // test 16, 17, 18, 19, 20, 21, 22
+        map.events.handleBrowserEvent(testEvents.move);
+        // test 23, 24
         t.ok(handler.started, "3) started flag still set to true");
         t.ok(handler.pinching, "3) touchmove sets the pinching flag to true");
 
+        OpenLayers.Event.isMultiTouch = old_isMultiTouch;
         
         customCb = function(evt, first, last) {
             t.eq(first.distance, 100, "4) calculated distance is correct");
@@ -214,19 +233,21 @@
             t.eq(last.delta, 20, "4) calculated delta is correct");
             t.eq(last.scale, 0.8, "4) calculated scale is correct");
         }
-        map.events.triggerEvent("touchend", testEvents.done);
+        // test 25, 26, 27, 28, 29, 30
+        map.events.handleBrowserEvent(testEvents.done);
+        // test 31, 32
         t.ok(!handler.started, "4) started flag is set to false");
         t.ok(!handler.pinching, "4) touchdone sets the pinching flag to false");
 
         OpenLayers.Event.stop = old_stop;
-        OpenLayers.Event.isMultiTouch = old_isMultiTouch;
 
         // test move or done before start
         customCb = function(evt) {
             t.fail("should not pass here")
         }
-        map.events.triggerEvent("touchmove", testEvents.move);
+        // no callbacks with tests expected
-        map.events.triggerEvent("touchend", testEvents.end);
+        map.events.handleBrowserEvent(testEvents.move);
+        map.events.handleBrowserEvent(testEvents.done);
 
     }
 

tests/Util.html

@@ -1,6 +1,17 @@
 <html>
 <head>
   <meta http-equiv="Content-Type" content="text/html;charset=utf-8" />
+  <style>
+    body {
+        margin: 0;
+        padding: 0;
+    }
+    #map {
+        position: absolute;
+        top:  1234px;
+        left: 123px;
+    }
+  </style>
   <script>
     var OpenLayers = [
         "OpenLayers/BaseTypes/Class.js",
@@ -99,10 +110,14 @@
     }
     
     function test_Util_pagePosition(t) {
-        t.plan( 1 );
+        t.plan( 2 );
         var pp = OpenLayers.Util.pagePosition(window);
-        t.eq( pp.toString(), "0,0", "Page position doesn't bail if passed 'window'")
+        t.eq( pp.toString(), "0,0", "Page position doesn't bail if passed 'window'");
         
+        window.scrollTo(100, 1200);
+        var mapDiv = document.getElementById("map");
+        pp = OpenLayers.Util.pagePosition(mapDiv);
+        t.eq( pp.toString(), "123,1234", "Page position should work after page has been scrolled");
     }
 
     function test_Util_createDiv(t) {