Made tech related functions clearly private in the player

This is important for enforcing the model that techs should
work the same for everything.

closes #2590
fixes #2060

- Made techGet and techCall private functions
- Made loadTech, techName, and unloadTech private
- Cleaned up all other private method naming in the player
- Removed some unneeded comments

CHANGELOG.md

@@ -132,6 +132,7 @@ CHANGELOG
 * @heff added a fancy loading spinner ([view](https://github.com/videojs/video.js/pull/2582))
 * @gkatsev added a mouse-hover time display to the progress bar ([view](https://github.com/videojs/video.js/pull/2569))
 * @heff added an attributes argument to createEl() ([view](https://github.com/videojs/video.js/pull/2589))
+* @heff made tech related functions private in the player ([view](https://github.com/videojs/video.js/pull/2590))
 
 --------------------
 

src/js/control-bar/mute-toggle.js

@@ -21,14 +21,14 @@ class MuteToggle extends Button {
     this.on(player, 'volumechange', this.update);
 
     // hide mute toggle if the current tech doesn't support volume control
-    if (player.tech && player.tech['featuresVolumeControl'] === false) {
+    if (player.tech_ && player.tech_['featuresVolumeControl'] === false) {
       this.addClass('vjs-hidden');
     }
 
     this.on(player, 'loadstart', function() {
       this.update(); // We need to update the button to account for a default muted state.
 
-      if (player.tech['featuresVolumeControl'] === false) {
+      if (player.tech_['featuresVolumeControl'] === false) {
         this.addClass('vjs-hidden');
       } else {
         this.removeClass('vjs-hidden');

src/js/control-bar/playback-rate-menu/playback-rate-menu-button.js

@@ -125,8 +125,8 @@ class PlaybackRateMenuButton extends MenuButton {
    * @method playbackRateSupported
    */
   playbackRateSupported() {
-    return this.player().tech
-      && this.player().tech['featuresPlaybackRate']
+    return this.player().tech_
+      && this.player().tech_['featuresPlaybackRate']
       && this.playbackRates()
       && this.playbackRates().length > 0
     ;

src/js/control-bar/text-track-controls/captions-button.js

@@ -41,7 +41,7 @@ class CaptionsButton extends TextTrackButton {
     super.update();
 
     // if native, then threshold is 1 because no settings button
-    if (this.player().tech && this.player().tech['featuresNativeTextTracks']) {
+    if (this.player().tech_ && this.player().tech_['featuresNativeTextTracks']) {
       threshold = 1;
     }
 
@@ -61,7 +61,7 @@ class CaptionsButton extends TextTrackButton {
   createItems() {
     let items = [];
 
-    if (!(this.player().tech && this.player().tech['featuresNativeTextTracks'])) {
+    if (!(this.player().tech_ && this.player().tech_['featuresNativeTextTracks'])) {
       items.push(new CaptionSettingsMenuItem(this.player_, { 'kind': this.kind_ }));
     }
 

src/js/control-bar/volume-control/volume-control.js

@@ -20,11 +20,11 @@ class VolumeControl extends Component {
     super(player, options);
 
     // hide volume controls when they're not supported by the current tech
-    if (player.tech && player.tech['featuresVolumeControl'] === false) {
+    if (player.tech_ && player.tech_['featuresVolumeControl'] === false) {
       this.addClass('vjs-hidden');
     }
     this.on(player, 'loadstart', function(){
-      if (player.tech['featuresVolumeControl'] === false) {
+      if (player.tech_['featuresVolumeControl'] === false) {
         this.addClass('vjs-hidden');
       } else {
         this.removeClass('vjs-hidden');

src/js/control-bar/volume-menu-button.js

@@ -48,7 +48,7 @@ class VolumeMenuButton extends MenuButton {
 
     // hide mute toggle if the current tech doesn't support volume control
     function updateVisibility() {
-      if (player.tech && player.tech['featuresVolumeControl'] === false) {
+      if (player.tech_ && player.tech_['featuresVolumeControl'] === false) {
         this.addClass('vjs-hidden');
       } else {
         this.removeClass('vjs-hidden');

src/js/tech/loader.js

@@ -30,7 +30,7 @@ class MediaLoader extends Component {
 
         // Check if the browser supports this technology
         if (tech && tech.isSupported()) {
-          player.loadTech(techName);
+          player.loadTech_(techName);
           break;
         }
       }

src/js/tracks/text-track-display.js

@@ -46,7 +46,7 @@ class TextTrackDisplay extends Component {
     // Should probably be moved to an external track loader when we support
     // tracks that don't need a display.
     player.ready(Fn.bind(this, function() {
-      if (player.tech && player.tech['featuresNativeTextTracks']) {
+      if (player.tech_ && player.tech_['featuresNativeTextTracks']) {
         this.hide();
         return;
       }
@@ -62,12 +62,12 @@ class TextTrackDisplay extends Component {
   }
 
   /**
-   * Toggle display texttracks 
+   * Toggle display texttracks
    *
    * @method toggleDisplay
    */
   toggleDisplay() {
-    if (this.player_.tech && this.player_.tech['featuresNativeTextTracks']) {
+    if (this.player_.tech_ && this.player_.tech_['featuresNativeTextTracks']) {
       this.hide();
     } else {
       this.show();
@@ -87,7 +87,7 @@ class TextTrackDisplay extends Component {
   }
 
   /**
-   * Clear display texttracks 
+   * Clear display texttracks
    *
    * @method clearDisplay
    */
@@ -98,7 +98,7 @@ class TextTrackDisplay extends Component {
   }
 
   /**
-   * Update display texttracks 
+   * Update display texttracks
    *
    * @method updateDisplay
    */
@@ -120,7 +120,7 @@ class TextTrackDisplay extends Component {
   }
 
   /**
-   * Add texttrack to texttrack list 
+   * Add texttrack to texttrack list
    *
    * @param {TextTrackObject} track Texttrack object to be added to list
    * @method updateForTrack
@@ -226,7 +226,7 @@ function constructColor(color, opacity) {
  * @method tryUpdateStyle
  */
 function tryUpdateStyle(el, style, rule) {
-  // 
+  //
   try {
     el.style[style] = rule;
   } catch (e) {}

test/unit/controls.test.js

@@ -16,7 +16,7 @@ test('should hide volume control if it\'s not supported', function(){
     id: noop,
     on: noop,
     ready: noop,
-    tech: {
+    tech_: {
       'featuresVolumeControl': false
     },
     volume: function(){},
@@ -50,7 +50,7 @@ test('should test and toggle volume control on `loadstart`', function(){
     muted: function(){
       return false;
     },
-    tech: {
+    tech_: {
       'featuresVolumeControl': true
     },
     reportUserActivity: function(){}
@@ -62,7 +62,7 @@ test('should test and toggle volume control on `loadstart`', function(){
   equal(volumeControl.hasClass('vjs-hidden'), false, 'volumeControl is hidden initially');
   equal(muteToggle.hasClass('vjs-hidden'), false, 'muteToggle is hidden initially');
 
-  player.tech['featuresVolumeControl'] = false;
+  player.tech_['featuresVolumeControl'] = false;
   for (i = 0; i < listeners.length; i++) {
     listeners[i]();
   }
@@ -70,7 +70,7 @@ test('should test and toggle volume control on `loadstart`', function(){
   equal(volumeControl.hasClass('vjs-hidden'), true, 'volumeControl does not hide itself');
   equal(muteToggle.hasClass('vjs-hidden'), true, 'muteToggle does not hide itself');
 
-  player.tech['featuresVolumeControl'] = true;
+  player.tech_['featuresVolumeControl'] = true;
   for (i = 0; i < listeners.length; i++) {
     listeners[i]();
   }

test/unit/player.test.js

@@ -253,7 +253,7 @@ test('should hide the poster when play is called', function() {
   player.play();
   equal(player.hasStarted(), true, 'the show poster flag is false after play');
 
-  player.tech.trigger('loadstart');
+  player.tech_.trigger('loadstart');
   equal(player.hasStarted(),
         false,
         'the resource selection algorithm sets the show poster flag to true');
@@ -419,7 +419,7 @@ test('make sure that controls listeners do not get added too many times', functi
   var player = TestHelpers.makePlayer({});
   var listeners = 0;
 
-  player.addTechControlsListeners = function() {
+  player.addTechControlsListeners_ = function() {
     listeners++;
   };
 
@@ -430,13 +430,13 @@ test('make sure that controls listeners do not get added too many times', functi
 
   player.controls(true);
 
-  equal(listeners, 0, 'addTechControlsListeners should not have gotten called yet');
+  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');
+  equal(listeners, 1, 'addTechControlsListeners_ should have gotten called once');
 
   player.dispose();
 });
@@ -488,9 +488,9 @@ test('should not add multiple first play events despite subsequent loads', funct
   });
 
   // Checking to make sure onLoadStart removes first play listener before adding a new one.
-  player.tech.trigger('loadstart');
-  player.tech.trigger('loadstart');
-  player.tech.trigger('play');
+  player.tech_.trigger('loadstart');
+  player.tech_.trigger('loadstart');
+  player.tech_.trigger('play');
 });
 
 test('should fire firstplay after resetting the player', function() {
@@ -502,23 +502,23 @@ test('should fire firstplay after resetting the player', function() {
   });
 
   // init firstplay listeners
-  player.tech.trigger('loadstart');
-  player.tech.trigger('play');
+  player.tech_.trigger('loadstart');
+  player.tech_.trigger('play');
   ok(fpFired, 'First firstplay fired');
 
   // reset the player
-  player.tech.trigger('loadstart');
+  player.tech_.trigger('loadstart');
   fpFired = false;
-  player.tech.trigger('play');
+  player.tech_.trigger('play');
   ok(fpFired, 'Second firstplay fired');
 
   // the play event can fire before the loadstart event.
   // in that case we still want the firstplay even to fire.
-  player.tech.paused = function(){ return false; };
+  player.tech_.paused = function(){ return false; };
   fpFired = false;
   // reset the player
-  player.tech.trigger('loadstart');
-  // player.tech.trigger('play');
+  player.tech_.trigger('loadstart');
+  // player.tech_.trigger('play');
   ok(fpFired, 'Third firstplay fired');
 });
 
@@ -527,14 +527,14 @@ test('should remove vjs-has-started class', function(){
 
   var player = TestHelpers.makePlayer({});
 
-  player.tech.trigger('loadstart');
-  player.tech.trigger('play');
+  player.tech_.trigger('loadstart');
+  player.tech_.trigger('play');
   ok(player.el().className.indexOf('vjs-has-started') !== -1, 'vjs-has-started class added');
 
-  player.tech.trigger('loadstart');
+  player.tech_.trigger('loadstart');
   ok(player.el().className.indexOf('vjs-has-started') === -1, 'vjs-has-started class removed');
 
-  player.tech.trigger('play');
+  player.tech_.trigger('play');
   ok(player.el().className.indexOf('vjs-has-started') !== -1, 'vjs-has-started class added again');
 });
 
@@ -543,18 +543,18 @@ test('should add and remove vjs-ended class', function() {
 
   var player = TestHelpers.makePlayer({});
 
-  player.tech.trigger('loadstart');
-  player.tech.trigger('play');
-  player.tech.trigger('ended');
+  player.tech_.trigger('loadstart');
+  player.tech_.trigger('play');
+  player.tech_.trigger('ended');
   ok(player.el().className.indexOf('vjs-ended') !== -1, 'vjs-ended class added');
 
-  player.tech.trigger('play');
+  player.tech_.trigger('play');
   ok(player.el().className.indexOf('vjs-ended') === -1, 'vjs-ended class removed');
 
-  player.tech.trigger('ended');
+  player.tech_.trigger('ended');
   ok(player.el().className.indexOf('vjs-ended') !== -1, 'vjs-ended class re-added');
 
-  player.tech.trigger('loadstart');
+  player.tech_.trigger('loadstart');
   ok(player.el().className.indexOf('vjs-ended') === -1, 'vjs-ended class removed');
 });
 
@@ -721,7 +721,7 @@ test('pause is called when player ended event is fired and player is not paused'
   player.pause = function() {
     pauses++;
   };
-  player.tech.trigger('ended');
+  player.tech_.trigger('ended');
   equal(pauses, 1, 'pause was called');
 });
 
@@ -735,7 +735,7 @@ test('pause is not called if the player is paused and ended is fired', function(
   player.pause = function() {
     pauses++;
   };
-  player.tech.trigger('ended');
+  player.tech_.trigger('ended');
   equal(pauses, 0, 'pause was not called when ended fired');
 });
 

test/unit/tracks/text-track.test.js

@@ -224,7 +224,7 @@ test('fires cuechange when cues become active and inactive', function() {
       changes = 0,
       cuechangeHandler,
       tt = new TextTrack({
-        tech: player.tech,
+        tech: player.tech_,
         mode: 'showing'
       });
 
@@ -241,19 +241,19 @@ test('fires cuechange when cues become active and inactive', function() {
   tt.oncuechange = cuechangeHandler;
   tt.addEventListener('cuechange', cuechangeHandler);
 
-  player.tech.currentTime = function() {
+  player.tech_.currentTime = function() {
     return 2;
   };
 
-  player.tech.trigger('timeupdate');
+  player.tech_.trigger('timeupdate');
 
   equal(changes, 2, 'a cuechange event trigger addEventListener and oncuechange');
 
-  player.tech.currentTime = function() {
+  player.tech_.currentTime = function() {
     return 7;
   };
 
-  player.tech.trigger('timeupdate');
+  player.tech_.trigger('timeupdate');
 
   equal(changes, 4, 'a cuechange event trigger addEventListener and oncuechange');
 

test/unit/tracks/tracks.test.js

@@ -44,10 +44,10 @@ test('Player track methods call the tech', function() {
 
   player = TestHelpers.makePlayer();
 
-  player.tech.textTracks = function() {
+  player.tech_.textTracks = function() {
     calls++;
   };
-  player.tech.addTextTrack = function() {
+  player.tech_.addTextTrack = function() {
     calls++;
   };
 
@@ -289,10 +289,10 @@ test('html5 tech supports native text tracks if the video supports it, unless it
 test('when switching techs, we should not get a new text track', function() {
   let player = TestHelpers.makePlayer();
 
-  player.loadTech('TechFaker');
+  player.loadTech_('TechFaker');
   let firstTracks = player.textTracks();
 
-  player.loadTech('TechFaker');
+  player.loadTech_('TechFaker');
   let secondTracks = player.textTracks();
 
   ok(firstTracks === secondTracks, 'the tracks are equal');