feat: split overrideNative method into separate methods (#5107)

Split overrideNative method into one for Audio and Video. Fixes issues with differing track support in IE11.
videojs · Apr 20, 2018 · 083a86c · 083a86c
1 parent 97db94e
commit 083a86c
Show file tree

Hide file tree

Showing 3 changed files with 117 additions and 94 deletions.
diff --git a/src/js/tech/html5.js b/src/js/tech/html5.js
@@ -201,112 +201,136 @@ class Html5 extends Tech {
   }
 
   /**
-   * Attempt to force override of native audio/video tracks.
+   * Attempt to force override of tracks for the given type
    *
+   * @param {String} type - Track type to override, possible values include 'Audio',
+   * 'Video', and 'Text'.
    * @param {Boolean} override - If set to true native audio/video will be overridden,
    * otherwise native audio/video will potentially be used.
+   * @private
    */
-  overrideNativeTracks(override) {
+  overrideNative_(type, override) {
     // If there is no behavioral change don't add/remove listeners
-    if (override !== (this.featuresNativeAudioTracks && this.featuresNativeVideoTracks)) {
+    if (override !== this[`featuresNative${type}Tracks`]) {
       return;
     }
 
-    if (this.audioTracksListeners_) {
-      Object.keys(this.audioTracksListeners_).forEach((eventName) => {
-        const elTracks = this.el().audioTracks;
-
-        elTracks.removeEventListener(eventName, this.audioTracksListeners_[eventName]);
-      });
-    }
+    const lowerCaseType = type.toLowerCase();
 
-    if (this.videoTracksListeners_) {
-      Object.keys(this.videoTracksListeners_).forEach((eventName) => {
-        const elTracks = this.el().videoTracks;
+    if (this[`${lowerCaseType}TracksListeners_`]) {
+      Object.keys(this[`${lowerCaseType}TracksListeners_`]).forEach((eventName) => {
+        const elTracks = this.el()[`${lowerCaseType}Tracks`];
 
-        elTracks.removeEventListener(eventName, this.videoTracksListeners_[eventName]);
+        elTracks.removeEventListener(eventName, this[`${lowerCaseType}TracksListeners_`][eventName]);
       });
     }
 
-    this.featuresNativeVideoTracks = !override;
-    this.featuresNativeAudioTracks = !override;
+    this[`featuresNative${type}Tracks`] = !override;
+    this[`${lowerCaseType}TracksListeners_`] = null;
 
-    this.audioTracksListeners_ = null;
-    this.videoTracksListeners_ = null;
+    this.proxyNativeTracksForType_(lowerCaseType);
+  }
 
-    this.proxyNativeTracks_();
+  /**
+   * Attempt to force override of native audio tracks.
+   *
+   * @param {Boolean} override - If set to true native audio will be overridden,
+   * otherwise native audio will potentially be used.
+   */
+  overrideNativeAudioTracks(override) {
+    this.overrideNative_('Audio', override);
   }
 
   /**
-   * Proxy all native track list events to our track lists if the browser we are playing
-   * in supports that type of track list.
+   * Attempt to force override of native video tracks.
    *
+   * @param {Boolean} override - If set to true native video will be overridden,
+   * otherwise native video will potentially be used.
+   */
+  overrideNativeVideoTracks(override) {
+    this.overrideNative_('Video', override);
+  }
+
+ /**
+   * Proxy native track list events for the given type to our track
+   * lists if the browser we are playing in supports that type of track list.
+   *
+   * @param {string} name - Track type; values include 'audio', 'video', and 'text'
    * @private
    */
-  proxyNativeTracks_() {
-    TRACK_TYPES.names.forEach((name) => {
-      const props = TRACK_TYPES[name];
-      const elTracks = this.el()[props.getterName];
-      const techTracks = this[props.getterName]();
-
-      if (!this[`featuresNative${props.capitalName}Tracks`] ||
-          !elTracks ||
-          !elTracks.addEventListener) {
-        return;
+  proxyNativeTracksForType_(name) {
+    const props = TRACK_TYPES[name];
+    const elTracks = this.el()[props.getterName];
+    const techTracks = this[props.getterName]();
+
+    if (!this[`featuresNative${props.capitalName}Tracks`] ||
+        !elTracks ||
+        !elTracks.addEventListener) {
+      return;
+    }
+    const listeners = {
+      change(e) {
+        techTracks.trigger({
+          type: 'change',
+          target: techTracks,
+          currentTarget: techTracks,
+          srcElement: techTracks
+        });
+      },
+      addtrack(e) {
+        techTracks.addTrack(e.track);
+      },
+      removetrack(e) {
+        techTracks.removeTrack(e.track);
       }
-      const listeners = {
-        change(e) {
-          techTracks.trigger({
-            type: 'change',
-            target: techTracks,
-            currentTarget: techTracks,
-            srcElement: techTracks
-          });
-        },
-        addtrack(e) {
-          techTracks.addTrack(e.track);
-        },
-        removetrack(e) {
-          techTracks.removeTrack(e.track);
-        }
-      };
-      const removeOldTracks = function() {
-        const removeTracks = [];
-
-        for (let i = 0; i < techTracks.length; i++) {
-          let found = false;
+    };
+    const removeOldTracks = function() {
+      const removeTracks = [];
 
-          for (let j = 0; j < elTracks.length; j++) {
-            if (elTracks[j] === techTracks[i]) {
-              found = true;
-              break;
-            }
-          }
+      for (let i = 0; i < techTracks.length; i++) {
+        let found = false;
 
-          if (!found) {
-            removeTracks.push(techTracks[i]);
+        for (let j = 0; j < elTracks.length; j++) {
+          if (elTracks[j] === techTracks[i]) {
+            found = true;
+            break;
           }
         }
 
-        while (removeTracks.length) {
-          techTracks.removeTrack(removeTracks.shift());
+        if (!found) {
+          removeTracks.push(techTracks[i]);
         }
-      };
+      }
 
-      this[props.getterName + 'Listeners_'] = listeners;
+      while (removeTracks.length) {
+        techTracks.removeTrack(removeTracks.shift());
+      }
+    };
 
-      Object.keys(listeners).forEach((eventName) => {
-        const listener = listeners[eventName];
+    this[props.getterName + 'Listeners_'] = listeners;
 
-        elTracks.addEventListener(eventName, listener);
-        this.on('dispose', (e) => elTracks.removeEventListener(eventName, listener));
-      });
+    Object.keys(listeners).forEach((eventName) => {
+      const listener = listeners[eventName];
 
-      // Remove (native) tracks that are not used anymore
-      this.on('loadstart', removeOldTracks);
-      this.on('dispose', (e) => this.off('loadstart', removeOldTracks));
+      elTracks.addEventListener(eventName, listener);
+      this.on('dispose', (e) => elTracks.removeEventListener(eventName, listener));
     });
 
+    // Remove (native) tracks that are not used anymore
+    this.on('loadstart', removeOldTracks);
+    this.on('dispose', (e) => this.off('loadstart', removeOldTracks));
+  }
+
+  /**
+   * Proxy all native track list events to our track lists if the browser we are playing
+   * in supports that type of track list.
+   *
+   * @private
+   */
+  proxyNativeTracks_() {
+    TRACK_TYPES.names.forEach((name) => {
+      this.proxyNativeTracksForType_(name);
+    });
   }
 
   /**

diff --git a/src/js/tech/tech.js b/src/js/tech/tech.js
@@ -786,14 +786,24 @@ class Tech extends Component {
   setPlaysinline() {}
 
   /**
-   * Attempt to force override of native audio.video tracks.
+   * Attempt to force override of native audio tracks.
    *
-   * @param {Boolean} override - If set to true native audio/video will be overridden,
-   * otherwise native audio/video will potentially be used.
+   * @param {Boolean} override - If set to true native audio will be overridden,
+   * otherwise native audio will potentially be used.
    *
    * @abstract
    */
-  overrideNativeTracks() {}
+  overrideNativeAudioTracks() {}
+
+  /**
+   * Attempt to force override of native video tracks.
+   *
+   * @param {Boolean} override - If set to true native video will be overridden,
+   * otherwise native video will potentially be used.
+   *
+   * @abstract
+   */
+  overrideNativeVideoTracks() {}
 
   /*
    * Check if the tech can support the given mime-type.

diff --git a/test/unit/tech/html5.test.js b/test/unit/tech/html5.test.js
@@ -550,15 +550,10 @@ if (Html5.supportsNativeAudioTracks()) {
         rems.push({ type, fn });
       }
     };
-    const vt = {
-      length: 0,
-      addEventListener: (type, fn) => null,
-      removeEventListener: (type, fn) => null
-    };
+
     const el = document.createElement('div');
 
     el.audioTracks = at;
-    el.videoTracks = vt;
 
     const htmlTech = new Html5({el});
 
@@ -567,21 +562,21 @@ if (Html5.supportsNativeAudioTracks()) {
     assert.equal(rems.length, 0,
       'no listeners should be removed');
 
-    htmlTech.overrideNativeTracks(true);
+    htmlTech.overrideNativeAudioTracks(true);
 
     assert.equal(adds.length, 3,
       'should not have added additional listeners');
     assert.equal(rems.length, 3,
       'should have removed previous three listeners');
 
-    htmlTech.overrideNativeTracks(true);
+    htmlTech.overrideNativeAudioTracks(true);
 
     assert.equal(adds.length, 3,
       'no state change so do not add listeners');
     assert.equal(rems.length, 3,
       'no state change so do not remove listeners');
 
-    htmlTech.overrideNativeTracks(false);
+    htmlTech.overrideNativeAudioTracks(false);
 
     assert.equal(adds.length, 6,
       'should add listeners because native tracks should be proxied');
@@ -673,14 +668,8 @@ if (Html5.supportsNativeVideoTracks()) {
         rems.push({ type, fn });
       }
     };
-    const at = {
-      length: 0,
-      addEventListener: (type, fn) => null,
-      removeEventListener: (type, fn) => null
-    };
     const el = document.createElement('div');
 
-    el.audioTracks = at;
     el.videoTracks = vt;
 
     const htmlTech = new Html5({el});
@@ -690,21 +679,21 @@ if (Html5.supportsNativeVideoTracks()) {
     assert.equal(rems.length, 0,
       'no listeners should be removed');
 
-    htmlTech.overrideNativeTracks(true);
+    htmlTech.overrideNativeVideoTracks(true);
 
     assert.equal(adds.length, 3,
       'should not have added additional listeners');
     assert.equal(rems.length, 3,
       'should have removed previous three listeners');
 
-    htmlTech.overrideNativeTracks(true);
+    htmlTech.overrideNativeVideoTracks(true);
 
     assert.equal(adds.length, 3,
       'no state change so do not add listeners');
     assert.equal(rems.length, 3,
       'no state change so do not remove listeners');
 
-    htmlTech.overrideNativeTracks(false);
+    htmlTech.overrideNativeVideoTracks(false);
 
     assert.equal(adds.length, 6,
       'should add listeners because native tracks should be proxied');