Skip to content

Commit

Permalink
fix(player): video-js embed missing video-js class (#5194)
Browse files Browse the repository at this point in the history
When having a video-js embed with a class attribute, as part of the
changes to remove old IE support (#5041), we overwrote our addition of
the video-js class when it was missing. Instead, we want to make sure
that we don't override the class names again since they are already set
up correctly.

Fixes videojs/http-streaming#100
  • Loading branch information
gkatsev committed May 23, 2018
1 parent fcd7d0d commit 954f3d9
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 1 deletion.
7 changes: 6 additions & 1 deletion src/js/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,12 @@ class Player extends Component {
tag.removeAttribute('height');

Object.getOwnPropertyNames(attrs).forEach(function(attr) {
el.setAttribute(attr, attrs[attr]);
// don't copy over the class attribute to the player element when we're in a div embed
// the class is already set up properly in the divEmbed case
// and we want to make sure that the `video-js` class doesn't get lost
if (!(divEmbed && attr === 'class')) {
el.setAttribute(attr, attrs[attr]);
}

if (divEmbed) {
tag.setAttribute(attr, attrs[attr]);
Expand Down
20 changes: 20 additions & 0 deletions test/unit/video.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,26 @@ QUnit.test('should return a video player instance', function(assert) {
assert.ok(player2.id() === 'test_vid_id2', 'created player from element');
});

QUnit.test('should add video-js class to video-js embed if missing', function(assert) {
const fixture = document.getElementById('qunit-fixture');

fixture.innerHTML += '<video-js id="test_vid_id"></video-js>' +
'<video-js id="test_vid_id2" class="foo"></video-js>';

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

assert.ok(player, 'created player from tag');
assert.ok(player.id() === 'test_vid_id');
assert.ok(player.hasClass('video-js'), 'we have the video-js class');

const tag2 = document.getElementById('test_vid_id2');
const player2 = videojs(tag2, { techOrder: ['techFaker'] });

assert.ok(player2.id() === 'test_vid_id2', 'created player from element');
assert.ok(player2.hasClass('video-js'), 'we have the video-js class');
assert.ok(player2.hasClass('foo'), 'we have the foo class');
});

QUnit.test('should log about already initalized players if options already passed',
function(assert) {
const origWarnLog = log.warn;
Expand Down

0 comments on commit 954f3d9

Please sign in to comment.