Skip to content

Commit

Permalink
deregister draggable marker mouseup and touchend (#7442)
Browse files Browse the repository at this point in the history
We have run into scenarios where we remove a draggable marker before `mouseup` is ever fired and, because mouseup/touchend never fired once, `_onUp` is called but the marker has already been torn down, leading to errors.

Ensuring these listeners are deregistered when tearing down the marker will ensure that doesn't happen.
  • Loading branch information
vbud authored and mollymerp committed Oct 19, 2018
1 parent 68ae1ed commit 1d734ca
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 0 deletions.
2 changes: 2 additions & 0 deletions src/ui/marker.js
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,8 @@ export default class Marker extends Evented {
this._map.off('moveend', this._update);
this._map.off('mousedown', this._addDragHandler);
this._map.off('touchstart', this._addDragHandler);
this._map.off('mouseup', this._onUp);
this._map.off('touchend', this._onUp);
delete this._map;
}
DOM.remove(this._element);
Expand Down
14 changes: 14 additions & 0 deletions test/unit/ui/marker.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -451,3 +451,17 @@ test('Marker with draggable:false does not move to new position in response to a
map.remove();
t.end();
});

test('Marker with draggable:true does not error if removed on mousedown', (t) => {
const map = createMap(t);
const marker = new Marker({draggable: true})
.setLngLat([0, 0])
.addTo(map);
const el = marker.getElement();
simulate.mousedown(el);
simulate.mousemove(el, {clientX: 10, clientY: 10});

marker.remove();
t.ok(map.fire('mouseup'));
t.end();
});

0 comments on commit 1d734ca

Please sign in to comment.