Skip to content

Commit

Permalink
fix: only allow left click dragging on progress bar and volume control (
Browse files Browse the repository at this point in the history
#4613)

Previously, any mouse button interaction with the bars would cause a reaction but that's unexpected for right and middle mouse clicks. This PR makes it so right and middle mouse clicks are ignored for dragging and clicking.
  • Loading branch information
kocoten1992 authored and gkatsev committed Nov 16, 2017
1 parent dc588dd commit 79b4355
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 0 deletions.
8 changes: 8 additions & 0 deletions src/js/control-bar/progress-control/seek-bar.js
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,10 @@ class SeekBar extends Slider {
* @listens mousedown
*/
handleMouseDown(event) {
if (!Dom.isSingleLeftClick(event)) {
return;
}

this.player_.scrubbing(true);

this.videoWasPlaying = !this.player_.paused();
Expand All @@ -191,6 +195,10 @@ class SeekBar extends Slider {
* @listens mousemove
*/
handleMouseMove(event) {
if (!Dom.isSingleLeftClick(event)) {
return;
}

let newTime = this.calculateDistance(event) * this.player_.duration();

// Don't let video end while scrubbing.
Expand Down
21 changes: 21 additions & 0 deletions src/js/control-bar/volume-control/volume-bar.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*/
import Slider from '../../slider/slider.js';
import Component from '../../component.js';
import * as Dom from '../../utils/dom.js';

// Required children
import './volume-level.js';
Expand Down Expand Up @@ -45,6 +46,22 @@ class VolumeBar extends Slider {
});
}

/**
* Handle mouse down on volume bar
*
* @param {EventTarget~Event} event
* The `mousedown` event that caused this to run.
*
* @listens mousedown
*/
handleMouseDown(event) {
if (!Dom.isSingleLeftClick(event)) {
return;
}

super.handleMouseDown(event);
}

/**
* Handle movement events on the {@link VolumeMenuButton}.
*
Expand All @@ -54,6 +71,10 @@ class VolumeBar extends Slider {
* @listens mousemove
*/
handleMouseMove(event) {
if (!Dom.isSingleLeftClick(event)) {
return;
}

this.checkMuted();
this.player_.volume(this.calculateDistance(event));
}
Expand Down
50 changes: 50 additions & 0 deletions src/js/utils/dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -744,6 +744,56 @@ export function insertContent(el, content) {
return appendContent(emptyEl(el), content);
}

/**
* Check if event was a single left click
*
* @param {EventTarget~Event} event
* Event object
*
* @return {boolean}
* - True if a left click
* - False if not a left click
*/
export function isSingleLeftClick(event) {
// Note: if you create something draggable, be sure to
// call it on both `mousedown` and `mousemove` event,
// otherwise `mousedown` should be enough for a button

if (event.button === undefined && event.buttons === undefined) {
// Why do we need `butttons` ?
// Because, middle mouse sometimes have this:
// e.button === 0 and e.buttons === 4
// Furthermore, we want to prevent combination click, something like
// HOLD middlemouse then left click, that would be
// e.button === 0, e.buttons === 5
// just `button` is not gonna work

// Alright, then what this block does ?
// this is for chrome `simulate mobile devices`
// I want to support this as well

return true;
}

if (event.button === 0 && event.buttons === undefined) {
// Touch screen, sometimes on some specific device, `buttons`
// doesn't have anything (safari on ios, blackberry...)

return true;
}

if (event.button !== 0 || event.buttons !== 1) {
// This is the reason we have those if else block above
// if any special case we can catch and let it slide
// we do it above, when get to here, this definitely
// is-not-left-click

return false;
}

return true;
}

/**
* Finds a single DOM element matching `selector` within the optional
* `context` of another DOM element (defaulting to `document`).
Expand Down

0 comments on commit 79b4355

Please sign in to comment.