Merge pull request #13771 from ahocevar/better-target-change-fix

Better fix for changing pointer ids on event target change

src/ol/MapBrowserEvent.js

@@ -15,9 +15,17 @@ class MapBrowserEvent extends MapEvent {
    * @param {import("./PluggableMap.js").default} map Map.
    * @param {EVENT} originalEvent Original event.
    * @param {boolean} [opt_dragging] Is the map currently being dragged?
-   * @param {?import("./PluggableMap.js").FrameState} [opt_frameState] Frame state.
+   * @param {import("./PluggableMap.js").FrameState} [opt_frameState] Frame state.
+   * @param {Array<PointerEvent>} [opt_activePointers] Active pointers.
    */
-  constructor(type, map, originalEvent, opt_dragging, opt_frameState) {
+  constructor(
+    type,
+    map,
+    originalEvent,
+    opt_dragging,
+    opt_frameState,
+    opt_activePointers
+  ) {
     super(type, map, opt_frameState);
 
     /**
@@ -48,6 +56,11 @@ class MapBrowserEvent extends MapEvent {
      * @api
      */
     this.dragging = opt_dragging !== undefined ? opt_dragging : false;
+
+    /**
+     * @type {Array<PointerEvent>|undefined}
+     */
+    this.activePointers = opt_activePointers;
   }
 
   /**

src/ol/MapBrowserEventHandler.js

@@ -9,6 +9,7 @@ import PointerEventType from './pointer/EventType.js';
 import Target from './events/Target.js';
 import {PASSIVE_EVENT_LISTENERS} from './has.js';
 import {VOID} from './functions.js';
+import {getValues} from './obj.js';
 import {listen, unlistenByKey} from './events.js';
 
 class MapBrowserEventHandler extends Target {
@@ -67,13 +68,13 @@ class MapBrowserEventHandler extends Target {
     const element = this.map_.getViewport();
 
     /**
-     * @type {number}
+     * @type {Array<PointerEvent>}
      * @private
      */
-    this.activePointers_ = 0;
+    this.activePointers_ = [];
 
     /**
-     * @type {!Object<number, boolean>}
+     * @type {!Object<number, Event>}
      * @private
      */
     this.trackedTouches_ = {};
@@ -104,7 +105,7 @@ class MapBrowserEventHandler extends Target {
     this.relayedListenerKey_ = listen(
       element,
       PointerEventType.POINTERMOVE,
-      this.relayEvent_,
+      this.relayMoveEvent_,
       this
     );
 
@@ -169,16 +170,30 @@ class MapBrowserEventHandler extends Target {
    */
   updateActivePointers_(pointerEvent) {
     const event = pointerEvent;
+    const id = event.pointerId;
 
     if (
       event.type == MapBrowserEventType.POINTERUP ||
       event.type == MapBrowserEventType.POINTERCANCEL
     ) {
-      delete this.trackedTouches_[event.pointerId];
+      delete this.trackedTouches_[id];
-    } else if (event.type == MapBrowserEventType.POINTERDOWN) {
+      for (const pointerId in this.trackedTouches_) {
-      this.trackedTouches_[event.pointerId] = true;
+        if (this.trackedTouches_[pointerId].target !== event.target) {
+          // Some platforms assign a new pointerId when the target changes.
+          // If this happens, delete one tracked pointer. If there is more
+          // than one tracked pointer for the old target, it will be cleared
+          // by subsequent POINTERUP events from other pointers.
+          delete this.trackedTouches_[pointerId];
+          break;
         }
-    this.activePointers_ = Object.keys(this.trackedTouches_).length;
+      }
+    } else if (
+      event.type == MapBrowserEventType.POINTERDOWN ||
+      event.type == MapBrowserEventType.POINTERMOVE
+    ) {
+      this.trackedTouches_[id] = event;
+    }
+    this.activePointers_ = getValues(this.trackedTouches_);
   }
 
   /**
@@ -191,7 +206,10 @@ class MapBrowserEventHandler extends Target {
     const newEvent = new MapBrowserEvent(
       MapBrowserEventType.POINTERUP,
       this.map_,
-      pointerEvent
+      pointerEvent,
+      undefined,
+      undefined,
+      this.activePointers_
     );
     this.dispatchEvent(newEvent);
 
@@ -210,7 +228,7 @@ class MapBrowserEventHandler extends Target {
       this.emulateClick_(this.down_);
     }
 
-    if (this.activePointers_ === 0) {
+    if (this.activePointers_.length === 0) {
       this.dragListenerKeys_.forEach(unlistenByKey);
       this.dragListenerKeys_.length = 0;
       this.dragging_ = false;
@@ -234,12 +252,15 @@ class MapBrowserEventHandler extends Target {
    * @private
    */
   handlePointerDown_(pointerEvent) {
-    this.emulateClicks_ = this.activePointers_ === 0;
+    this.emulateClicks_ = this.activePointers_.length === 0;
     this.updateActivePointers_(pointerEvent);
     const newEvent = new MapBrowserEvent(
       MapBrowserEventType.POINTERDOWN,
       this.map_,
-      pointerEvent
+      pointerEvent,
+      undefined,
+      undefined,
+      this.activePointers_
     );
     this.dispatchEvent(newEvent);
 
@@ -303,29 +324,36 @@ class MapBrowserEventHandler extends Target {
     // To avoid a 'false' touchmove event to be dispatched, we test if the pointer
     // moved a significant distance.
     if (this.isMoving_(pointerEvent)) {
+      this.updateActivePointers_(pointerEvent);
       this.dragging_ = true;
       const newEvent = new MapBrowserEvent(
         MapBrowserEventType.POINTERDRAG,
         this.map_,
         pointerEvent,
-        this.dragging_
+        this.dragging_,
+        undefined,
+        this.activePointers_
       );
       this.dispatchEvent(newEvent);
     }
   }
 
   /**
-   * Wrap and relay a pointer event.  Note that this requires that the type
+   * Wrap and relay a pointermove event.
-   * string for the MapBrowserEvent matches the PointerEvent type.
    * @param {PointerEvent} pointerEvent Pointer
    * event.
    * @private
    */
-  relayEvent_(pointerEvent) {
+  relayMoveEvent_(pointerEvent) {
     this.originalPointerMoveEvent_ = pointerEvent;
     const dragging = !!(this.down_ && this.isMoving_(pointerEvent));
     this.dispatchEvent(
-      new MapBrowserEvent(pointerEvent.type, this.map_, pointerEvent, dragging)
+      new MapBrowserEvent(
+        MapBrowserEventType.POINTERMOVE,
+        this.map_,
+        pointerEvent,
+        dragging
+      )
     );
   }
 

src/ol/interaction/Pointer.js

@@ -3,7 +3,6 @@
  */
 import Interaction from './Interaction.js';
 import MapBrowserEventType from '../MapBrowserEventType.js';
-import {getValues} from '../obj.js';
 
 /**
  * @typedef {Object} Options
@@ -80,12 +79,6 @@ class PointerInteraction extends Interaction {
      */
     this.handlingDownUpSequence = false;
 
-    /**
-     * @type {!Object<string, PointerEvent>}
-     * @private
-     */
-    this.trackedPointers_ = {};
-
     /**
      * @type {Array<PointerEvent>}
      * @protected
@@ -189,29 +182,8 @@ class PointerInteraction extends Interaction {
    * @private
    */
   updateTrackedPointers_(mapBrowserEvent) {
-    if (isPointerDraggingEvent(mapBrowserEvent)) {
+    if (mapBrowserEvent.activePointers) {
-      const event = mapBrowserEvent.originalEvent;
+      this.targetPointers = mapBrowserEvent.activePointers;
-
-      const id = event.pointerId.toString();
-      if (mapBrowserEvent.type == MapBrowserEventType.POINTERUP) {
-        delete this.trackedPointers_[id];
-        for (const pointerId in this.trackedPointers_) {
-          if (this.trackedPointers_[pointerId].target !== event.target) {
-            // Some platforms assign a new pointerId when the target changes.
-            // If this happens, delete one tracked pointer. If there is more
-            // than one tracked pointer for the old target, it will be cleared
-            // by subsequent POINTERUP events from other pointers.
-            delete this.trackedPointers_[pointerId];
-            break;
-          }
-        }
-      } else if (mapBrowserEvent.type == MapBrowserEventType.POINTERDOWN) {
-        this.trackedPointers_[id] = event;
-      } else if (id in this.trackedPointers_) {
-        // update only when there was a pointerdown event for this pointer
-        this.trackedPointers_[id] = event;
-      }
-      this.targetPointers = getValues(this.trackedPointers_);
     }
   }
 }
@@ -231,18 +203,4 @@ export function centroid(pointerEvents) {
   return [clientX / length, clientY / length];
 }
 
-/**
- * @param {import("../MapBrowserEvent.js").default} mapBrowserEvent Event.
- * @return {boolean} Whether the event is a pointerdown, pointerdrag
- *     or pointerup event.
- */
-function isPointerDraggingEvent(mapBrowserEvent) {
-  const type = mapBrowserEvent.type;
-  return (
-    type === MapBrowserEventType.POINTERDOWN ||
-    type === MapBrowserEventType.POINTERDRAG ||
-    type === MapBrowserEventType.POINTERUP
-  );
-}
-
 export default PointerInteraction;

test/browser/spec/ol/MapBrowserEventHandler.test.js

@@ -281,4 +281,48 @@ describe('ol/MapBrowserEventHandler', function () {
       expect(dblclickSpy.called).to.not.be.ok();
     });
   });
+
+  describe('Event target change', function () {
+    let target, handler, element, down1, down2, up1, up2;
+    beforeEach(function () {
+      target = document.createElement('div');
+      handler = new MapBrowserEventHandler(
+        new Map({
+          target: target,
+        })
+      );
+
+      element = handler.element_;
+      down1 = new Event('pointerdown', {target: element});
+      down1.clientX = 0;
+      down1.clientY = 0;
+      down1.button = 0;
+      down1.pointerId = 1;
+      down2 = new Event('pointerdown', {target: element});
+      down2.clientX = 0;
+      down2.clientY = 0;
+      down2.button = 0;
+      down2.pointerId = 2;
+      up1 = new Event('pointerup', {target: element.firstChild});
+      up1.clientX = 0;
+      up1.clientY = 0;
+      up1.button = 0;
+      up1.pointerId = 3;
+      up2 = new Event('pointerup', {target: element.firstChild});
+      up2.clientX = 0;
+      up2.clientY = 0;
+      up2.button = 0;
+      up2.pointerId = 4;
+    });
+
+    it('keeps activePointers up to date when event target changes', function () {
+      element.dispatchEvent(down1);
+      element.dispatchEvent(down2);
+      expect(handler.activePointers_[0].pointerId).to.be(1);
+      expect(handler.activePointers_[1].pointerId).to.be(2);
+      document.dispatchEvent(up1);
+      document.dispatchEvent(up2);
+      expect(handler.activePointers_).to.have.length(0);
+    });
+  });
 });

test/browser/spec/ol/interaction/pointer.test.js

@@ -1,7 +1,10 @@
-import Event from '../../../../../src/ol/events/Event.js';
+import Layer from '../../../../../src/ol/layer/Layer.js';
 import Map from '../../../../../src/ol/Map.js';
 import MapBrowserEvent from '../../../../../src/ol/MapBrowserEvent.js';
+import MapBrowserEventHandler from '../../../../../src/ol/MapBrowserEventHandler.js';
+import OlEvent from '../../../../../src/ol/events/Event.js';
 import PointerInteraction from '../../../../../src/ol/interaction/Pointer.js';
+import View from '../../../../../src/ol/View.js';
 
 describe('ol.interaction.Pointer', function () {
   describe('#handleEvent', function () {
@@ -10,7 +13,7 @@ describe('ol.interaction.Pointer', function () {
 
     beforeEach(function () {
       const type = 'pointerdown';
-      const pointerEvent = new Event();
+      const pointerEvent = new OlEvent();
       pointerEvent.type = type;
       pointerEvent.pointerId = 0;
       pointerEvent.preventDefault = function () {
@@ -130,4 +133,66 @@ describe('ol.interaction.Pointer', function () {
       expect(handleUpCalled).to.be(true);
     });
   });
+
+  describe("With a map's MapBrowserEventHandler", function () {
+    let target, interaction, element, down1, down2, up1, up2;
+    beforeEach(function () {
+      target = document.createElement('div');
+      target.style.width = '100px';
+      target.style.height = '100px';
+      document.body.appendChild(target);
+      interaction = new PointerInteraction({});
+      const handler = new MapBrowserEventHandler(
+        new Map({
+          target: target,
+          layers: [
+            new Layer({
+              render: () => {},
+            }),
+          ],
+          interactions: [interaction],
+          view: new View({
+            center: [0, 0],
+            zoom: 0,
+          }),
+        })
+      );
+      handler.map_.renderSync();
+      element = handler.element_;
+      down1 = new Event('pointerdown', {target: element});
+      down1.clientX = 0;
+      down1.clientY = 0;
+      down1.button = 0;
+      down1.pointerId = 1;
+      down2 = new Event('pointerdown', {target: element});
+      down2.clientX = 0;
+      down2.clientY = 0;
+      down2.button = 0;
+      down2.pointerId = 2;
+      up1 = new Event('pointerup', {target: element.firstChild});
+      up1.clientX = 0;
+      up1.clientY = 0;
+      up1.button = 0;
+      up1.pointerId = 1;
+      up2 = new Event('pointerup', {target: element.firstChild});
+      up2.clientX = 0;
+      up2.clientY = 0;
+      up2.button = 0;
+      up2.pointerId = 2;
+    });
+
+    afterEach(function () {
+      document.body.removeChild(target);
+    });
+
+    it('tracks pointers correctly', function () {
+      element.dispatchEvent(down1);
+      element.dispatchEvent(down2);
+      expect(interaction.targetPointers[0].pointerId).to.be(1);
+      expect(interaction.targetPointers[1].pointerId).to.be(2);
+      document.dispatchEvent(up1);
+      document.dispatchEvent(up2);
+      expect(interaction.targetPointers).to.have.length(0);
+    });
+  });
 });