From f99dc1e9ec3435fa372d60b11f5c8f161a7d4f7f Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Wed, 15 Jun 2022 16:29:07 +0200 Subject: [PATCH 1/3] Better fix for changing pointer ids on event target change --- src/ol/MapBrowserEventHandler.js | 18 +++++++++++++++--- src/ol/interaction/Pointer.js | 10 ---------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/ol/MapBrowserEventHandler.js b/src/ol/MapBrowserEventHandler.js index b17513cd53..e34350897f 100644 --- a/src/ol/MapBrowserEventHandler.js +++ b/src/ol/MapBrowserEventHandler.js @@ -73,7 +73,7 @@ class MapBrowserEventHandler extends Target { this.activePointers_ = 0; /** - * @type {!Object} + * @type {!Object} * @private */ this.trackedTouches_ = {}; @@ -169,14 +169,26 @@ 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) { - this.trackedTouches_[event.pointerId] = true; + this.trackedTouches_[id] = event; + } else if (event.type == MapBrowserEventType.POINTERMOVE) { + for (const pointerId in this.trackedTouches_) { + 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; } diff --git a/src/ol/interaction/Pointer.js b/src/ol/interaction/Pointer.js index 83ff187f35..51d268e144 100644 --- a/src/ol/interaction/Pointer.js +++ b/src/ol/interaction/Pointer.js @@ -195,16 +195,6 @@ class PointerInteraction extends Interaction { 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_) { From 134ec9c8d0f6d620540ec14247adf10434b04604 Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Thu, 16 Jun 2022 11:26:28 +0200 Subject: [PATCH 2/3] Remove duplicated logic from PointerInteraction --- src/ol/MapBrowserEvent.js | 17 +++++++++-- src/ol/MapBrowserEventHandler.js | 48 +++++++++++++++++++++----------- src/ol/interaction/Pointer.js | 36 ++---------------------- 3 files changed, 49 insertions(+), 52 deletions(-) diff --git a/src/ol/MapBrowserEvent.js b/src/ol/MapBrowserEvent.js index 95d2063b3c..dab8ab991d 100644 --- a/src/ol/MapBrowserEvent.js +++ b/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} [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|undefined} + */ + this.activePointers = opt_activePointers; } /** diff --git a/src/ol/MapBrowserEventHandler.js b/src/ol/MapBrowserEventHandler.js index e34350897f..aa52b2f2cf 100644 --- a/src/ol/MapBrowserEventHandler.js +++ b/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,10 +68,10 @@ class MapBrowserEventHandler extends Target { const element = this.map_.getViewport(); /** - * @type {number} + * @type {Array} * @private */ - this.activePointers_ = 0; + this.activePointers_ = []; /** * @type {!Object} @@ -104,7 +105,7 @@ class MapBrowserEventHandler extends Target { this.relayedListenerKey_ = listen( element, PointerEventType.POINTERMOVE, - this.relayEvent_, + this.relayMoveEvent_, this ); @@ -176,9 +177,6 @@ class MapBrowserEventHandler extends Target { event.type == MapBrowserEventType.POINTERCANCEL ) { delete this.trackedTouches_[id]; - } else if (event.type == MapBrowserEventType.POINTERDOWN) { - this.trackedTouches_[id] = event; - } else if (event.type == MapBrowserEventType.POINTERMOVE) { for (const pointerId in this.trackedTouches_) { if (this.trackedTouches_[pointerId].target !== event.target) { // Some platforms assign a new pointerId when the target changes. @@ -189,8 +187,13 @@ class MapBrowserEventHandler extends Target { break; } } + } else if ( + event.type == MapBrowserEventType.POINTERDOWN || + event.type == MapBrowserEventType.POINTERMOVE + ) { + this.trackedTouches_[id] = event; } - this.activePointers_ = Object.keys(this.trackedTouches_).length; + this.activePointers_ = getValues(this.trackedTouches_); } /** @@ -203,7 +206,10 @@ class MapBrowserEventHandler extends Target { const newEvent = new MapBrowserEvent( MapBrowserEventType.POINTERUP, this.map_, - pointerEvent + pointerEvent, + undefined, + undefined, + this.activePointers_ ); this.dispatchEvent(newEvent); @@ -222,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; @@ -246,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); @@ -315,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 - * string for the MapBrowserEvent matches the PointerEvent type. + * Wrap and relay a pointermove event. * @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 + ) ); } diff --git a/src/ol/interaction/Pointer.js b/src/ol/interaction/Pointer.js index 51d268e144..493d1b31b0 100644 --- a/src/ol/interaction/Pointer.js +++ b/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} - * @private - */ - this.trackedPointers_ = {}; - /** * @type {Array} * @protected @@ -189,19 +182,8 @@ class PointerInteraction extends Interaction { * @private */ updateTrackedPointers_(mapBrowserEvent) { - if (isPointerDraggingEvent(mapBrowserEvent)) { - const event = mapBrowserEvent.originalEvent; - - const id = event.pointerId.toString(); - if (mapBrowserEvent.type == MapBrowserEventType.POINTERUP) { - delete this.trackedPointers_[id]; - } 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_); + if (mapBrowserEvent.activePointers) { + this.targetPointers = mapBrowserEvent.activePointers; } } } @@ -221,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; From 6dcc583e06d789a28493ff90df9e88cf0dc1bc5d Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Thu, 16 Jun 2022 13:31:16 +0200 Subject: [PATCH 3/3] Add tests to ensure correct pointer tracking --- .../spec/ol/MapBrowserEventHandler.test.js | 44 ++++++++++++ .../spec/ol/interaction/pointer.test.js | 69 ++++++++++++++++++- 2 files changed, 111 insertions(+), 2 deletions(-) diff --git a/test/browser/spec/ol/MapBrowserEventHandler.test.js b/test/browser/spec/ol/MapBrowserEventHandler.test.js index c4c04b980e..2abedc04ad 100644 --- a/test/browser/spec/ol/MapBrowserEventHandler.test.js +++ b/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); + }); + }); }); diff --git a/test/browser/spec/ol/interaction/pointer.test.js b/test/browser/spec/ol/interaction/pointer.test.js index ee426592cd..72cc22e05c 100644 --- a/test/browser/spec/ol/interaction/pointer.test.js +++ b/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); + }); + }); });