Skip to content

Commit

Permalink
fix: extra warn logs on already initialized player references (#3888)
Browse files Browse the repository at this point in the history
  • Loading branch information
gesinger authored and gkatsev committed Dec 23, 2016
1 parent 4fd9022 commit b7c384e
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 2 deletions.
4 changes: 2 additions & 2 deletions src/js/video.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,6 @@ if (typeof HTMLVideoElement === 'undefined' &&
function videojs(id, options, ready) {
let tag;

options = options || {};

// Allow for element or ID to be passed in
// String ID
if (typeof id === 'string') {
Expand Down Expand Up @@ -111,6 +109,8 @@ function videojs(id, options, ready) {
return tag.player || Player.players[tag.playerId];
}

options = options || {};

videojs.hooks('beforesetup').forEach(function(hookFunction) {
const opts = hookFunction(tag, mergeOptions(options));

Expand Down
39 changes: 39 additions & 0 deletions test/unit/video.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import videojs from '../../src/js/video.js';
import TestHelpers from './test-helpers.js';
import * as Dom from '../../src/js/utils/dom.js';
import log from '../../src/js/utils/log.js';
import document from 'global/document';

QUnit.module('video.js');
Expand Down Expand Up @@ -45,6 +46,44 @@ QUnit.test('should return a video player instance', function(assert) {
player2.dispose();
});

QUnit.test('should log about already initalized players if options already passed',
function(assert) {
const origWarnLog = log.warn;
const fixture = document.getElementById('qunit-fixture');
const warnLogs = [];

log.warn = (args) => {
warnLogs.push(args);
};

fixture.innerHTML += '<video id="test_vid_id"></video>';

const player = videojs('test_vid_id', { techOrder: ['techFaker'] });

assert.ok(player, 'created player from tag');
assert.equal(player.id(), 'test_vid_id', 'player has the right ID');
assert.equal(warnLogs.length, 0, 'no warn logs');

const playerAgain = videojs('test_vid_id');

assert.equal(player, playerAgain, 'did not create a second player from same tag');
assert.equal(warnLogs.length, 0, 'no warn logs');

const playerAgainWithOptions = videojs('test_vid_id', { techOrder: ['techFaker'] });

assert.equal(player,
playerAgainWithOptions,
'did not create a second player from same tag');
assert.equal(warnLogs.length, 1, 'logged a warning');
assert.equal(warnLogs[0],
'Player "test_vid_id" is already initialised. Options will not be applied.',
'logged the right message');

log.warn = origWarnLog;

player.dispose();
});

QUnit.test('should return a video player instance from el html5 tech', function(assert) {
const fixture = document.getElementById('qunit-fixture');

Expand Down

0 comments on commit b7c384e

Please sign in to comment.