Skip to content

Commit

Permalink
fix DragRotate when mouseup occurs outside window or iframe (#9512)
Browse files Browse the repository at this point in the history 
fix #4622
  • Loading branch information
@ansis
ansis authored Apr 13, 2020
1 parent d9696a0 commit 7312e58
Show file tree
Hide file tree
Showing 4 changed files with 107 additions and 18 deletions.
23 changes: 23 additions & 0 deletions src/ui/handler/mouse.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,17 @@ import type Point from '@mapbox/point-geometry';
const LEFT_BUTTON = 0;
const RIGHT_BUTTON = 2;

// the values for each button in MouseEvent.buttons
const BUTTONS_FLAGS = {
[LEFT_BUTTON]: 1,
[RIGHT_BUTTON]: 2
};

function buttonStillPressed(e: MouseEvent, button: number) {
const flag = BUTTONS_FLAGS[button];
return e.buttons === undefined || (e.buttons & flag) !== flag;
}

class MouseHandler {

_enabled: boolean;
Expand Down Expand Up @@ -50,6 +61,17 @@ class MouseHandler {
if (!lastPoint) return;
e.preventDefault();

if (buttonStillPressed(e, this._eventButton)) {
// Some browsers don't fire a `mouseup` when the mouseup occurs outside
// the window or iframe:
// https://github.com/mapbox/mapbox-gl-js/issues/4622
//
// If the button is no longer pressed during this `mousemove` it may have
// been released outside of the window or iframe.
this.reset();
return;
}

if (!this._moved && point.dist(lastPoint) < this._clickTolerance) return;
this._moved = true;
this._lastPoint = point;
Expand All @@ -59,6 +81,7 @@ class MouseHandler {
}

mouseupWindow(e: MouseEvent) {
if (!this._lastPoint) return;
const eventButton = DOM.mouseButton(e);
if (eventButton !== this._eventButton) return;
if (this._moved) DOM.suppressClick();
Expand Down
35 changes: 19 additions & 16 deletions test/unit/ui/handler/drag_pan.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ function createMap(t, clickTolerance, dragPan) {
});
}

// MouseEvent.buttons = 1 // left button
const buttons = 1;

test('DragPanHandler fires dragstart, drag, and dragend events at appropriate times in response to a mouse-triggered drag', (t) => {
const map = createMap(t);

Expand All @@ -30,7 +33,7 @@ test('DragPanHandler fires dragstart, drag, and dragend events at appropriate ti
t.equal(drag.callCount, 0);
t.equal(dragend.callCount, 0);

simulate.mousemove(map.getCanvas(), {clientX: 10, clientY: 10});
simulate.mousemove(map.getCanvas(), {buttons, clientX: 10, clientY: 10});
map._renderTaskQueue.run();
t.equal(dragstart.callCount, 1);
t.equal(drag.callCount, 1);
Expand Down Expand Up @@ -63,7 +66,7 @@ test('DragPanHandler captures mousemove events during a mouse-triggered drag (re
t.equal(drag.callCount, 0);
t.equal(dragend.callCount, 0);

simulate.mousemove(window.document.body, {clientX: 10, clientY: 10});
simulate.mousemove(window.document.body, {buttons, clientX: 10, clientY: 10});
map._renderTaskQueue.run();
t.equal(dragstart.callCount, 1);
t.equal(drag.callCount, 1);
Expand Down Expand Up @@ -121,7 +124,7 @@ test('DragPanHandler prevents mousemove events from firing during a drag (#1555)
simulate.mousedown(map.getCanvasContainer());
map._renderTaskQueue.run();

simulate.mousemove(map.getCanvasContainer(), {clientX: 10, clientY: 10});
simulate.mousemove(map.getCanvasContainer(), {buttons, clientX: 10, clientY: 10});
map._renderTaskQueue.run();

simulate.mouseup(map.getCanvasContainer());
Expand All @@ -142,7 +145,7 @@ test('DragPanHandler ends a mouse-triggered drag if the window blurs', (t) => {
simulate.mousedown(map.getCanvas());
map._renderTaskQueue.run();

simulate.mousemove(map.getCanvas(), {clientX: 10, clientY: 10});
simulate.mousemove(map.getCanvas(), {buttons, clientX: 10, clientY: 10});
map._renderTaskQueue.run();

simulate.blur(window);
Expand Down Expand Up @@ -176,14 +179,14 @@ test('DragPanHandler requests a new render frame after each mousemove event', (t
const requestFrame = t.spy(map, '_requestRenderFrame');

simulate.mousedown(map.getCanvas());
simulate.mousemove(map.getCanvas(), {clientX: 10, clientY: 10});
simulate.mousemove(map.getCanvas(), {buttons, clientX: 10, clientY: 10});
t.ok(requestFrame.callCount > 0);

map._renderTaskQueue.run();

// https://github.com/mapbox/mapbox-gl-js/issues/6063
requestFrame.resetHistory();
simulate.mousemove(map.getCanvas(), {clientX: 20, clientY: 20});
simulate.mousemove(map.getCanvas(), {buttons, clientX: 20, clientY: 20});
t.equal(requestFrame.callCount, 1);

map.remove();
Expand All @@ -208,7 +211,7 @@ test('DragPanHandler can interleave with another handler', (t) => {
t.equal(drag.callCount, 0);
t.equal(dragend.callCount, 0);

simulate.mousemove(map.getCanvas(), {clientX: 10, clientY: 10});
simulate.mousemove(map.getCanvas(), {buttons, clientX: 10, clientY: 10});
map._renderTaskQueue.run();
t.equal(dragstart.callCount, 1);
t.equal(drag.callCount, 1);
Expand All @@ -221,7 +224,7 @@ test('DragPanHandler can interleave with another handler', (t) => {
t.equal(drag.callCount, 1);
t.equal(dragend.callCount, 0);

simulate.mousemove(map.getCanvas(), {clientX: 20, clientY: 20});
simulate.mousemove(map.getCanvas(), {buttons, clientX: 20, clientY: 20});
map._renderTaskQueue.run();
t.equal(dragstart.callCount, 1);
t.equal(drag.callCount, 2);
Expand Down Expand Up @@ -250,13 +253,13 @@ test('DragPanHandler can interleave with another handler', (t) => {
map.on('drag', drag);
map.on('dragend', dragend);

simulate.mousedown(map.getCanvas(), {[`${modifier}Key`]: true});
simulate.mousedown(map.getCanvas(), {buttons, [`${modifier}Key`]: true});
map._renderTaskQueue.run();
t.equal(dragstart.callCount, 0);
t.equal(drag.callCount, 0);
t.equal(dragend.callCount, 0);

simulate.mousemove(map.getCanvas(), {[`${modifier}Key`]: true, clientX: 10, clientY: 10});
simulate.mousemove(map.getCanvas(), {buttons, [`${modifier}Key`]: true, clientX: 10, clientY: 10});
map._renderTaskQueue.run();
t.equal(dragstart.callCount, 0);
t.equal(drag.callCount, 0);
Expand Down Expand Up @@ -296,7 +299,7 @@ test('DragPanHandler can interleave with another handler', (t) => {
t.equal(drag.callCount, 0);
t.equal(dragend.callCount, 0);

simulate.mousemove(map.getCanvas(), {clientX: 10, clientY: 10});
simulate.mousemove(map.getCanvas(), {buttons, clientX: 10, clientY: 10});
map._renderTaskQueue.run();
t.equal(dragstart.callCount, 0);
t.equal(drag.callCount, 0);
Expand Down Expand Up @@ -359,25 +362,25 @@ test('DragPanHandler does not end a drag on right button mouseup', (t) => {
t.equal(drag.callCount, 0);
t.equal(dragend.callCount, 0);

simulate.mousemove(map.getCanvas(), {clientX: 10, clientY: 10});
simulate.mousemove(map.getCanvas(), {buttons, clientX: 10, clientY: 10});
map._renderTaskQueue.run();
t.equal(dragstart.callCount, 1);
t.equal(drag.callCount, 1);
t.equal(dragend.callCount, 0);

simulate.mousedown(map.getCanvas(), {buttons: 2, button: 2});
simulate.mousedown(map.getCanvas(), {buttons: buttons + 2, button: 2});
map._renderTaskQueue.run();
t.equal(dragstart.callCount, 1);
t.equal(drag.callCount, 1);
t.equal(dragend.callCount, 0);

simulate.mouseup(map.getCanvas(), {buttons: 0, button: 2});
simulate.mouseup(map.getCanvas(), {buttons, button: 2});
map._renderTaskQueue.run();
t.equal(dragstart.callCount, 1);
t.equal(drag.callCount, 1);
t.equal(dragend.callCount, 0);

simulate.mousemove(map.getCanvas(), {clientX: 20, clientY: 20});
simulate.mousemove(map.getCanvas(), {buttons, clientX: 20, clientY: 20});
map._renderTaskQueue.run();
t.equal(dragstart.callCount, 1);
t.equal(drag.callCount, 2);
Expand Down Expand Up @@ -409,7 +412,7 @@ test('DragPanHandler does not begin a drag if preventDefault is called on the mo
simulate.mousedown(map.getCanvas());
map._renderTaskQueue.run();

simulate.mousemove(map.getCanvas(), {clientX: 10, clientY: 10});
simulate.mousemove(map.getCanvas(), {buttons, clientX: 10, clientY: 10});
map._renderTaskQueue.run();

simulate.mouseup(map.getCanvas());
Expand Down
60 changes: 60 additions & 0 deletions test/unit/ui/handler/mouse_rotate.test.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import {test} from '../../../util/test';
import {extend} from '../../../../src/util/util';
import window from '../../../../src/util/window';
import Map from '../../../../src/ui/map';
import DOM from '../../../../src/util/dom';
import simulate from '../../../util/simulate_interaction';
import browser from '../../../../src/util/browser';

function createMap(t, options) {
t.stub(Map.prototype, '_detectMissingCSS');
return new Map(extend({container: DOM.create('div', '', window.document.body)}, options));
}

test('MouseRotateHandler#isActive', (t) => {
const map = createMap(t);
const mouseRotate = map.handlers._handlersById.mouseRotate;

// Prevent inertial rotation.
t.stub(browser, 'now').returns(0);
t.equal(mouseRotate.isActive(), false);

simulate.mousedown(map.getCanvas(), {buttons: 2, button: 2});
map._renderTaskQueue.run();
t.equal(mouseRotate.isActive(), false);

simulate.mousemove(map.getCanvas(), {buttons: 2, clientX: 10, clientY: 10});
map._renderTaskQueue.run();
t.equal(mouseRotate.isActive(), true);

simulate.mouseup(map.getCanvas(), {buttons: 0, button: 2});
map._renderTaskQueue.run();
t.equal(mouseRotate.isActive(), false);

map.remove();
t.end();
});

test('MouseRotateHandler#isActive #4622 regression test', (t) => {
const map = createMap(t);
const mouseRotate = map.handlers._handlersById.mouseRotate;

// Prevent inertial rotation.
simulate.mousedown(map.getCanvas(), {buttons: 2, button: 2});
map._renderTaskQueue.run();
t.equal(mouseRotate.isActive(), false);

simulate.mousemove(map.getCanvas(), {buttons: 2, clientX: 10, clientY: 10});
map._renderTaskQueue.run();
t.equal(mouseRotate.isActive(), true);

// Some browsers don't fire mouseup when it happens outside the window.
// Make the handler in active when it encounters a mousemove without the button pressed.

simulate.mousemove(map.getCanvas(), {buttons: 0, clientX: 10, clientY: 10});
map._renderTaskQueue.run();
t.equal(mouseRotate.isActive(), false);

map.remove();
t.end();
});
7 changes: 5 additions & 2 deletions test/unit/ui/map/isMoving.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ function createMap(t) {
return new Map({container: DOM.create('div', '', window.document.body)});
}

// MouseEvent.buttons
const buttons = 1;

test('Map#isMoving returns false by default', (t) => {
const map = createMap(t);
t.equal(map.isMoving(), false);
Expand Down Expand Up @@ -49,7 +52,7 @@ test('Map#isMoving returns true when drag panning', (t) => {
simulate.mousedown(map.getCanvas());
map._renderTaskQueue.run();

simulate.mousemove(map.getCanvas(), {clientX: 10, clientY: 10});
simulate.mousemove(map.getCanvas(), {buttons, clientX: 10, clientY: 10});
map._renderTaskQueue.run();

simulate.mouseup(map.getCanvas());
Expand Down Expand Up @@ -139,7 +142,7 @@ test('Map#isMoving returns true when drag panning and scroll zooming interleave'
simulate.mousedown(map.getCanvas());
map._renderTaskQueue.run();

simulate.mousemove(map.getCanvas(), {clientX: 10, clientY: 10});
simulate.mousemove(map.getCanvas(), {buttons, clientX: 10, clientY: 10});
map._renderTaskQueue.run();

const browserNow = t.stub(browser, 'now');
Expand Down

0 comments on commit 7312e58

Please sign in to comment.