From 82c8b805488a520f26447917da736e069181727a Mon Sep 17 00:00:00 2001
From: Gary Katsevman <git@gkatsev.com>
Date: Wed, 28 Jun 2017 16:38:29 +1000
Subject: [PATCH] fix: auto-removal remote text tracks being removed when not
 supposed to (#4450)

We added a feature so that remote text tracks can auto-removed when a source changes. However, in 6.x we changed the source behavior to be asynchronous meaning that some text tracks were accidentally being removed when they weren't supposed to be.
For example:
```js
var player = videojs('my-player');
player.src({src: 'video.mp4', type: 'video/mp4'});
 // set second arg to false so that they get auto-removed on source change
player.addRemoteTextTrack({kind: 'captions', src: 'text.vtt', srclang: 'en'}, false);
```
Now when the player loads, this captions track is actually missing because it was removed.

Instead of adding auto-removal tracks immediately to the list, wait until we've selected a source before adding them in.


Fixes #4403 and #4315.
---
 src/js/tech/tech.js                  |  2 +-
 test/unit/tech/tech.test.js          |  5 +++
 test/unit/tracks/text-tracks.test.js | 52 ++++++++++++++++++++++++++++
 3 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/src/js/tech/tech.js b/src/js/tech/tech.js
index c0501af6bb..c13067c32a 100644
--- a/src/js/tech/tech.js
+++ b/src/js/tech/tech.js
@@ -700,7 +700,7 @@ class Tech extends Component {
 
     if (manualCleanup !== true) {
       // create the TextTrackList if it doesn't exist
-      this.autoRemoteTextTracks_.addTrack(htmlTrackElement.track);
+      this.ready(() => this.autoRemoteTextTracks_.addTrack(htmlTrackElement.track));
     }
 
     return htmlTrackElement;
diff --git a/test/unit/tech/tech.test.js b/test/unit/tech/tech.test.js
index fa07559832..c219834d40 100644
--- a/test/unit/tech/tech.test.js
+++ b/test/unit/tech/tech.test.js
@@ -214,11 +214,14 @@ QUnit.test('switching sources should clear all remote tracks that are added with
 
   const tech = new MyTech();
 
+  tech.triggerReady();
+
   // set the initial source
   tech.setSource({src: 'foo.mp4', type: 'mp4'});
 
   // default value for manualCleanup is true
   tech.addRemoteTextTrack({});
+  this.clock.tick(1);
 
   assert.equal(warning,
                'Calling addRemoteTextTrack without explicitly setting the "manualCleanup" parameter to `true` is deprecated and default to `false` in future version of video.js',
@@ -226,6 +229,7 @@ QUnit.test('switching sources should clear all remote tracks that are added with
 
   // should be automatically cleaned up when source changes
   tech.addRemoteTextTrack({}, false);
+  this.clock.tick(1);
 
   assert.equal(tech.textTracks().length, 2, 'should have two text tracks at the start');
   assert.equal(tech.remoteTextTrackEls().length,
@@ -238,6 +242,7 @@ QUnit.test('switching sources should clear all remote tracks that are added with
 
   // change source to force cleanup of auto remote text tracks
   tech.setSource({src: 'bar.mp4', type: 'mp4'});
+  this.clock.tick(1);
 
   assert.equal(tech.textTracks().length,
                1,
diff --git a/test/unit/tracks/text-tracks.test.js b/test/unit/tracks/text-tracks.test.js
index 2c980ead2a..00199a9239 100644
--- a/test/unit/tracks/text-tracks.test.js
+++ b/test/unit/tracks/text-tracks.test.js
@@ -528,3 +528,55 @@ QUnit.test('removeRemoteTextTrack should be able to take both a track and the re
               'the track element was removed correctly');
   player.dispose();
 });
+
+if (Html5.isSupported()) {
+  QUnit.test('auto remove tracks should not clean up tracks added while source is being added', function(assert) {
+    const player = TestHelpers.makePlayer({
+      techOrder: ['html5'],
+      html5: {
+        nativeTextTracks: false
+      }
+    });
+
+    const track = {
+      kind: 'kind',
+      src: 'src',
+      language: 'language',
+      label: 'label',
+      default: 'default'
+    };
+
+    player.src({src: 'example.mp4', type: 'video/mp4'});
+    player.addRemoteTextTrack(track, false);
+
+    this.clock.tick(1);
+    assert.equal(player.textTracks().length, 1, 'we have one text track');
+
+    player.dispose();
+  });
+
+  QUnit.test('auto remove tracks added right before a source change will be cleaned up', function(assert) {
+    const player = TestHelpers.makePlayer({
+      techOrder: ['html5'],
+      html5: {
+        nativeTextTracks: false
+      }
+    });
+
+    const track = {
+      kind: 'kind',
+      src: 'src',
+      language: 'language',
+      label: 'label',
+      default: 'default'
+    };
+
+    player.addRemoteTextTrack(track, false);
+    player.src({src: 'example.mp4', type: 'video/mp4'});
+
+    this.clock.tick(1);
+    assert.equal(player.textTracks().length, 0, 'we do not have any tracks left');
+
+    player.dispose();
+  });
+}