Skip to content

Commit

Permalink
Don't always add controls to the video element
Browse files Browse the repository at this point in the history
Fix for #1561. If the HTML tech is being constructed without a video element to work off of, make sure that the controls attribute is only added under the same circumstances it would be at player init. Before this fix, if you loaded the Flash tech and then switched to the HTML tech, you would see the native controls underneath the video.js controls.

Fix controls attribute test on iOS
iOS uses native controls by default and so was failing the assertions that native controls weren't used. Force custom controls for this test case to make it work like everywhere else.

Update nativeControlsForTouch default
The default value changed so fix the predicate that tested for whether it was in use.

closes #1811, closes #1564, closes #1561
  • Loading branch information
dmlap authored and heff committed Jan 22, 2015
1 parent b51d830 commit 4bde5c8
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 5 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ CHANGELOG
## HEAD (Unreleased)
* @heff exported missing source handler functions ([view](https://github.com/videojs/video.js/pull/1787))
* @heff fixed type support checking for an empty src string ([view](https://github.com/videojs/video.js/pull/1797))
@carpasse fixed a bug in updating child indexes after removing components ([view](https://github.com/videojs/video.js/pull/1814))
* @carpasse fixed a bug in updating child indexes after removing components ([view](https://github.com/videojs/video.js/pull/1814))
* @dmlap fixed a bug where native controls would show after switching techs ([view](https://github.com/videojs/video.js/pull/1811))

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

Expand Down
10 changes: 9 additions & 1 deletion src/js/media/html5.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ vjs.Html5.prototype.createEl = function(){
var player = this.player_,
// If possible, reuse original tag for HTML5 playback technology element
el = player.tag,
attributes,
newEl,
clone;

Expand All @@ -74,8 +75,15 @@ vjs.Html5.prototype.createEl = function(){
player.tag = null;
} else {
el = vjs.createEl('video');

// determine if native controls should be used
attributes = videojs.util.mergeOptions({}, player.tagAttributes);
if (!vjs.TOUCH_ENABLED || player.options()['nativeControlsForTouch'] !== true) {
delete attributes.controls;
}

vjs.setElementAttributes(el,
vjs.obj.merge(player.tagAttributes || {}, {
vjs.obj.merge(attributes, {
id:player.id() + '_html5_api',
'class':'vjs-tech'
})
Expand Down
15 changes: 14 additions & 1 deletion test/unit/media.html5.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ module('HTML5', {
id: function(){ return 'id'; },
el: function(){ return el; },
options_: {},
options: function(){ return {}; },
options: function(){ return this.options_; },
bufferedPercent: function() { return 0; },
controls: function(){ return false; },
usingNativeControls: function(){ return false; },
Expand Down Expand Up @@ -71,6 +71,19 @@ test('test playbackRate', function() {
strictEqual(tech.playbackRate(), 0.75);
});

test('should remove the controls attribute when recreating the element', function() {
var el;
player.tagAttributes = {
controls: true
};
// force custom controls so the test environment is equivalent on iOS
player.options_['nativeControlsForTouch'] = false;
el = tech.createEl();

ok(!el.controls, 'controls attribute is absent');
ok(player.tagAttributes.controls, 'tag attribute is still present');
});

test('patchCanPlayType patches canplaytype with our function, conditionally', function() {
// the patch runs automatically so we need to first unpatch
vjs.Html5.unpatchCanPlayType();
Expand Down
4 changes: 2 additions & 2 deletions test/unit/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,7 @@ test('should restore attributes from the original video tag when creating a new
// simulate attributes stored from the original tag
player.tagAttributes = {
'preload': 'auto',
'controls': true,
'autoplay': true,
'webkit-playsinline': true
};

Expand All @@ -545,7 +545,7 @@ test('should restore attributes from the original video tag when creating a new
el = vjs.Html5.prototype.createEl.call(html5Mock);

equal(el.getAttribute('preload'), 'none', 'attribute was successful overridden by an option');
equal(el.getAttribute('controls'), '', 'controls attribute was set properly');
equal(el.getAttribute('autoplay'), '', 'autoplay attribute was set properly');
equal(el.getAttribute('webkit-playsinline'), '', 'webkit-playsinline attribute was set properly');
});

Expand Down

0 comments on commit 4bde5c8

Please sign in to comment.