feat: Change addRemoteTextTrack's manualCleanup option default value to false (#7588)

src/js/player.js

@@ -4521,23 +4521,20 @@ class Player extends Component {
 
   /**
    * Create a remote {@link TextTrack} and an {@link HTMLTrackElement}.
-   * When manualCleanup is set to false, the track will be automatically removed
-   * on source changes.
    *
    * @param {Object} options
    *        Options to pass to {@link HTMLTrackElement} during creation. See
    *        {@link HTMLTrackElement} for object properties that you should use.
    *
-   * @param {boolean} [manualCleanup=true] if set to false, the TextTrack will be
-   *                                       removed on a source change
+   * @param {boolean} [manualCleanup=false] if set to true, the TextTrack will not be removed
+   *                                        from the TextTrackList and HtmlTrackElementList
+   *                                        after a source change
    *
    * @return {HtmlTrackElement}
    *         the HTMLTrackElement that was created and added
    *         to the HtmlTrackElementList and the remote
    *         TextTrackList
    *
-   * @deprecated The default value of the "manualCleanup" parameter will default
-   *             to "false" in upcoming versions of Video.js
    */
   addRemoteTextTrack(options, manualCleanup) {
     if (this.tech_) {

src/js/tech/html5.js

@@ -911,12 +911,12 @@ class Html5 extends Tech {
    *
    * @param {Object} options The object should contain values for
    * kind, language, label, and src (location of the WebVTT file)
-   * @param {boolean} [manualCleanup=true] if set to false, the TextTrack will be
-   * automatically removed from the video element whenever the source changes
+   * @param {boolean} [manualCleanup=false] if set to true, the TextTrack
+   * will not be removed from the TextTrackList and HtmlTrackElementList
+   * after a source change
    * @return {HTMLTrackElement} An Html Track Element.
    * This can be an emulated {@link HTMLTrackElement} or a native one.
-   * @deprecated The default value of the "manualCleanup" parameter will default
-   * to "false" in upcoming versions of Video.js
+   *
    */
   addRemoteTextTrack(options, manualCleanup) {
     const htmlTrackElement = super.addRemoteTextTrack(options, manualCleanup);

src/js/tech/tech.js

@@ -758,7 +758,7 @@ class Tech extends Component {
    * @param {Object} options
    *        See {@link Tech#createRemoteTextTrack} for more detailed properties.
    *
-   * @param {boolean} [manualCleanup=true]
+   * @param {boolean} [manualCleanup=false]
    *        - When false: the TextTrack will be automatically removed from the video
    *          element whenever the source changes
    *        - When True: The TextTrack will have to be cleaned up manually
@@ -766,24 +766,19 @@ class Tech extends Component {
    * @return {HTMLTrackElement}
    *         An Html Track Element.
    *
-   * @deprecated The default functionality for this function will be equivalent
-   *             to "manualCleanup=false" in the future. The manualCleanup parameter will
-   *             also be removed.
    */
   addRemoteTextTrack(options = {}, manualCleanup) {
     const htmlTrackElement = this.createRemoteTextTrack(options);
 
-    if (manualCleanup !== true && manualCleanup !== false) {
-      // deprecation warning
-      log.warn('Calling addRemoteTextTrack without explicitly setting the "manualCleanup" parameter to `true` is deprecated and default to `false` in future version of video.js');
-      manualCleanup = true;
+    if (typeof manualCleanup !== 'boolean') {
+      manualCleanup = false;
     }
 
     // store HTMLTrackElement and TextTrack to remote list
     this.remoteTextTrackEls().addTrackElement_(htmlTrackElement);
     this.remoteTextTracks().addTrack(htmlTrackElement.track);
 
-    if (manualCleanup !== true) {
+    if (manualCleanup === false) {
       // create the TextTrackList if it doesn't exist
       this.ready(() => this.autoRemoteTextTracks_.addTrack(htmlTrackElement.track));
     }

test/unit/player.test.js

@@ -2061,7 +2061,6 @@ QUnit.test('player#reset removes the poster', function(assert) {
 });
 
 QUnit.test('player#reset removes remote text tracks', function(assert) {
-  sinon.stub(log, 'warn');
   const player = TestHelpers.makePlayer();
 
   this.clock.tick(1);
@@ -2076,8 +2075,6 @@ QUnit.test('player#reset removes remote text tracks', function(assert) {
   assert.strictEqual(player.remoteTextTracks().length, 1, 'there is one RTT');
   player.reset();
   assert.strictEqual(player.remoteTextTracks().length, 0, 'there are zero RTTs');
-  assert.strictEqual(log.warn.callCount, 1, 'one warning about for manualCleanup');
-  log.warn.restore();
 });
 
 QUnit.test('player#reset progress bar', function(assert) {

test/unit/tech/tech.test.js

@@ -219,14 +219,8 @@ QUnit.test('dispose() should clear all tracks that are added after creation', fu
   assert.equal(tech.textTracks().length, 0, 'should have zero video tracks after dispose');
 });
 
-QUnit.test('switching sources should clear all remote tracks that are added with manualCleanup = false', function(assert) {
-
+QUnit.test('switching sources should clear all remote tracks that are added with the default manualCleanup = false', function(assert) {
   const oldLogWarn = log.warn;
-  let warning;
-
-  log.warn = function(wrning) {
-    warning = wrning;
-  };
 
   // Define a new tech class
   const MyTech = extend(Tech);
@@ -254,18 +248,10 @@ QUnit.test('switching sources should clear all remote tracks that are added with
   // 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',
-    'we log a warning when `addRemoteTextTrack` is called without a manualCleanup argument'
-  );
-
+  // should not be automatically cleaned up when source changes
+  tech.addRemoteTextTrack({}, true);
   // should be automatically cleaned up when source changes
-  tech.addRemoteTextTrack({}, false);
+  tech.addRemoteTextTrack({});
   this.clock.tick(1);
 
   assert.equal(tech.textTracks().length, 2, 'should have two text tracks at the start');