Skip to content

Commit

Permalink
remove handler event listeners when map is removed (#9508) (#9517)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ansis
ansis authored Apr 8, 2020
1 parent 97037a4 commit acb48ab
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 48 deletions.
8 changes: 4 additions & 4 deletions src/ui/control/navigation_control.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,10 +173,10 @@ class MouseRotateWrapper {

move(e: MouseEvent, point: Point) {
const map = this.map;
const r = this.mouseRotate.windowMousemove(e, point);
const r = this.mouseRotate.mousemoveWindow(e, point);
if (r && r.bearingDelta) map.setBearing(map.getBearing() + r.bearingDelta);
if (this.mousePitch) {
const p = this.mousePitch.windowMousemove(e, point);
const p = this.mousePitch.mousemoveWindow(e, point);
if (p && p.pitchDelta) map.setPitch(map.getPitch() + p.pitchDelta);
}
}
Expand Down Expand Up @@ -208,8 +208,8 @@ class MouseRotateWrapper {
}

mouseup(e: MouseEvent) {
this.mouseRotate.windowMouseup(e);
if (this.mousePitch) this.mousePitch.windowMouseup(e);
this.mouseRotate.mouseupWindow(e);
if (this.mousePitch) this.mousePitch.mouseupWindow(e);
this.offTemp();
}

Expand Down
4 changes: 2 additions & 2 deletions src/ui/handler/box_zoom.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ class BoxZoomHandler {
this._active = true;
}

windowMousemove(e: MouseEvent, point: Point) {
mousemoveWindow(e: MouseEvent, point: Point) {
if (!this._active) return;

const pos = point;
Expand Down Expand Up @@ -111,7 +111,7 @@ class BoxZoomHandler {
this._box.style.height = `${maxY - minY}px`;
}

windowMouseup(e: MouseEvent, point: Point) {
mouseupWindow(e: MouseEvent, point: Point) {
if (!this._active) return;

if (e.button !== 0) return;
Expand Down
4 changes: 2 additions & 2 deletions src/ui/handler/mouse.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class MouseHandler {
this._eventButton = eventButton;
}

windowMousemove(e: MouseEvent, point: Point) {
mousemoveWindow(e: MouseEvent, point: Point) {
const lastPoint = this._lastPoint;
if (!lastPoint) return;
e.preventDefault();
Expand All @@ -58,7 +58,7 @@ class MouseHandler {
return this._move(lastPoint, point);
}

windowMouseup(e: MouseEvent) {
mouseupWindow(e: MouseEvent) {
const eventButton = DOM.mouseButton(e);
if (eventButton !== this._eventButton) return;
if (this._moved) DOM.suppressClick();
Expand Down
101 changes: 61 additions & 40 deletions src/ui/handler_manager.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import TapDragZoomHandler from './handler/tap_drag_zoom';
import DragPanHandler from './handler/shim/drag_pan';
import DragRotateHandler from './handler/shim/drag_rotate';
import TouchZoomRotateHandler from './handler/shim/touch_zoom_rotate';
import {extend} from '../util/util';
import {bindAll, extend} from '../util/util';
import window from '../util/window';
import Point from '@mapbox/point-geometry';
import assert from 'assert';
Expand Down Expand Up @@ -105,6 +105,7 @@ class HandlerManager {
_changes: Array<[HandlerResult, Object, any]>;
_previousActiveHandlers: { [string]: Handler };
_bearingChanged: boolean;
_listeners: Array<[HTMLElement, string, void | {passive?: boolean, capture?: boolean}]>;

constructor(map: Map, options: { interactive: boolean, pitchWithRotate: boolean, clickTolerance: number, bearingSnap: number}) {
this._map = map;
Expand All @@ -122,45 +123,56 @@ class HandlerManager {

this._addDefaultHandlers(options);

// Bind touchstart and touchmove with passive: false because, even though
// they only fire a map events and therefore could theoretically be
// passive, binding with passive: true causes iOS not to respect
// e.preventDefault() in _other_ handlers, even if they are non-passive
// (see https://bugs.webkit.org/show_bug.cgi?id=184251)
this._addListener(this._el, 'touchstart', {passive: false});
this._addListener(this._el, 'touchmove', {passive: false});
this._addListener(this._el, 'touchend');
this._addListener(this._el, 'touchcancel');

this._addListener(this._el, 'mousedown');
this._addListener(this._el, 'mousemove');
this._addListener(this._el, 'mouseup');

// Bind window-level event listeners for move and up/end events. In the absence of
// the pointer capture API, which is not supported by all necessary platforms,
// window-level event listeners give us the best shot at capturing events that
// fall outside the map canvas element. Use `{capture: true}` for the move event
// to prevent map move events from being fired during a drag.
this._addListener(window.document, 'mousemove', {capture: true}, 'windowMousemove');
this._addListener(window.document, 'mouseup', undefined, 'windowMouseup');

this._addListener(this._el, 'mouseover');
this._addListener(this._el, 'mouseout');
this._addListener(this._el, 'dblclick');
this._addListener(this._el, 'click');

this._addListener(this._el, 'keydown', {capture: false});
this._addListener(this._el, 'keyup');

this._addListener(this._el, 'wheel', {passive: false});
this._addListener(this._el, 'contextmenu');

DOM.addEventListener(window, 'blur', () => this.stop());
bindAll(['handleEvent', 'handleWindowEvent'], this);

const el = this._el;

this._listeners = [
// Bind touchstart and touchmove with passive: false because, even though
// they only fire a map events and therefore could theoretically be
// passive, binding with passive: true causes iOS not to respect
// e.preventDefault() in _other_ handlers, even if they are non-passive
// (see https://bugs.webkit.org/show_bug.cgi?id=184251)
[el, 'touchstart', {passive: false}],
[el, 'touchmove', {passive: false}],
[el, 'touchend', undefined],
[el, 'touchcancel', undefined],

[el, 'mousedown', undefined],
[el, 'mousemove', undefined],
[el, 'mouseup', undefined],

// Bind window-level event listeners for move and up/end events. In the absence of
// the pointer capture API, which is not supported by all necessary platforms,
// window-level event listeners give us the best shot at capturing events that
// fall outside the map canvas element. Use `{capture: true}` for the move event
// to prevent map move events from being fired during a drag.
[window.document, 'mousemove', {capture: true}],
[window.document, 'mouseup', undefined],

[el, 'mouseover', undefined],
[el, 'mouseout', undefined],
[el, 'dblclick', undefined],
[el, 'click', undefined],

[el, 'keydown', {capture: false}],
[el, 'keyup', undefined],

[el, 'wheel', {passive: false}],
[el, 'contextmenu', undefined],

[window, 'blur', undefined]
];

for (const [target, type, listenerOptions] of this._listeners) {
DOM.addEventListener(target, type, target === window.document ? this.handleWindowEvent : this.handleEvent, listenerOptions);
}
}

_addListener(element: Element, eventType: string, options: Object, name_?: string) {
const name = name_ || eventType;
DOM.addEventListener(element, eventType, e => this._processInputEvent(e, name), options);
destroy() {
for (const [target, type, listenerOptions] of this._listeners) {
DOM.removeEventListener(target, type, target === window.document ? this.handleWindowEvent : this.handleEvent, listenerOptions);
}
}

_addDefaultHandlers(options: { interactive: boolean, pitchWithRotate: boolean, clickTolerance: number }) {
Expand Down Expand Up @@ -257,7 +269,16 @@ class HandlerManager {
return false;
}

_processInputEvent(e: InputEvent | RenderFrameEvent, eventName?: string) {
handleWindowEvent(e: InputEvent) {
this.handleEvent(e, `${e.type}Window`);
}

handleEvent(e: InputEvent | RenderFrameEvent, eventName?: string) {

if (e.type === 'blur') {
this.stop();
return;
}

this._updatingCamera = true;
assert(e.timeStamp !== undefined);
Expand Down Expand Up @@ -475,7 +496,7 @@ class HandlerManager {
if (this._frameId === undefined) {
this._frameId = this._map._requestRenderFrame(timeStamp => {
delete this._frameId;
this._processInputEvent(new RenderFrameEvent('renderFrame', {timeStamp}));
this.handleEvent(new RenderFrameEvent('renderFrame', {timeStamp}));
this._applyChanges();
});
}
Expand Down
2 changes: 2 additions & 0 deletions src/ui/map.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2319,6 +2319,8 @@ class Map extends Camera {
}
this._renderTaskQueue.clear();
this.painter.destroy();
this.handlers.destroy();
delete this.handlers;
this.setStyle(null);
if (typeof window !== 'undefined') {
window.removeEventListener('resize', this._onWindowResize, false);
Expand Down

0 comments on commit acb48ab

Please sign in to comment.