Skip to content

Commit

Permalink
@gkatsev improved tech controls listener handling.. closes #2511
Browse files Browse the repository at this point in the history
  • Loading branch information
gkatsev committed Aug 25, 2015
1 parent e10a2f4 commit de39cfc
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ CHANGELOG
* @forbesjo switched automated testing to BrowserStack ([view](https://github.com/videojs/video.js/pull/2492))
* @gkatsev fixed nativeControlsForTouch handling. Defaults to native controls on iphone and native android browsers. ([view](https://github.com/videojs/video.js/pull/2499))
* @heff fixed cross-platform track tests by switching to a fake tech ([view](https://github.com/videojs/video.js/pull/2496))
* @gkatsev improved tech controls listener handling. ([view](https://github.com/videojs/video.js/pull/2511))

--------------------

Expand Down
5 changes: 3 additions & 2 deletions src/js/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,9 @@ class Player extends Component {
* @method addTechControlsListeners
*/
addTechControlsListeners() {
// Make sure to remove all the previous listeners in case we are called multiple times.
this.removeTechControlsListeners();

// Some browsers (Chrome & IE) don't trigger a click on a flash swf, but do
// trigger mousedown/up.
// http://stackoverflow.com/questions/1444562/javascript-onclick-event-over-flash-object
Expand Down Expand Up @@ -1939,7 +1942,6 @@ class Player extends Component {
if (this.usingNativeControls_ !== bool) {
this.usingNativeControls_ = bool;
if (bool) {
this.removeTechControlsListeners();
this.addClass('vjs-using-native-controls');

/**
Expand All @@ -1952,7 +1954,6 @@ class Player extends Component {
*/
this.trigger('usingnativecontrols');
} else {
this.addTechControlsListeners();
this.removeClass('vjs-using-native-controls');

/**
Expand Down
26 changes: 26 additions & 0 deletions test/unit/player.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,32 @@ test('should allow for tracking when native controls are used', function(){
player.dispose();
});

test('make sure that controls listeners do not get added too many times', function(){
var player = TestHelpers.makePlayer({});
var listeners = 0;

player.addTechControlsListeners = function() {
listeners++;
};

// Make sure native controls is false before starting test
player.usingNativeControls(false);

player.usingNativeControls(true);

player.controls(true);

equal(listeners, 0, 'addTechControlsListeners should not have gotten called yet');

player.usingNativeControls(false);
player.controls(false);

player.controls(true);
equal(listeners, 1, 'addTechControlsListeners should have gotten called once');

player.dispose();
});

// test('should use custom message when encountering an unsupported video type',
// function() {
// videojs.options['notSupportedMessage'] = 'Video no go <a href="">link</a>';
Expand Down

0 comments on commit de39cfc

Please sign in to comment.