Merge pull request video-dev#1527 from SVT/hidden-subtitles

Properly handle hidden subtitles, and use the disabled property where approrpriate

doc/API.md

@@ -1093,7 +1093,7 @@ get/set : subtitle track id (returned by). Returns -1 if no track is visible. Se
 
 (default: `false`)
 
-get/set : boolean to enable/disable subtitle display by hls.js
+get/set : if set to true the active subtitle track mode will be set to `showing` and the browser will display the active subtitles. If set to false, the mode will be set to `hidden`. 
 
 ## Live stream API
 

src/controller/subtitle-track-controller.js

@@ -40,8 +40,12 @@ class SubtitleTrackController extends EventHandler {
     let trackId = -1;
     let tracks = filterSubtitleTracks(this.media.textTracks);
     for (let id = 0; id < tracks.length; id++) {
-      if (tracks[id].mode === 'showing') {
+      if (tracks[id].mode === 'hidden') {
+        // Do not break in case there is a following track with showing.
         trackId = id;
+      } else if (tracks[id].mode === 'showing') {
+        trackId = id;
+        break;
       }
     }
 
@@ -189,8 +193,8 @@ class SubtitleTrackController extends EventHandler {
     let textTracks = filterSubtitleTracks(this.media.textTracks);
 
     // hide currently enabled subtitle track
-    if (this.trackId !== -1 && this.subtitleDisplay) {
-      textTracks[this.trackId].mode = 'hidden';
+    if (this.trackId !== -1) {
+      textTracks[this.trackId].mode = 'disabled';
     }
 
     this.trackId = newId;
@@ -201,9 +205,9 @@ class SubtitleTrackController extends EventHandler {
       return;
     }
 
-    let subtitleTrack = this.tracks[newId];
-    if (this.subtitleDisplay) {
-      textTracks[newId].mode = 'showing';
+    const subtitleTrack = this.tracks[newId];
+    if(newId < textTracks.length) {
+      textTracks[newId].mode = this.subtitleDisplay ? 'showing' : 'hidden';
     }
 
     // check if we need to load playlist for this subtitle Track

src/controller/timeline-controller.js

@@ -208,7 +208,11 @@ class TimelineController extends EventHandler {
         if (!textTrack) {
             textTrack = this.createTextTrack('subtitles', track.name, track.lang);
         }
-        textTrack.mode = track.default ? 'showing' : 'hidden';
+        if (track.default) {
+          textTrack.mode = this.hls.subtitleDisplay ? 'showing' : 'hidden';
+        } else {
+          textTrack.mode = 'disabled';
+        }
         this.textTracks.push(textTrack);
       });
     }

tests/unit/controller/subtitle-track-controller.js

@@ -0,0 +1,85 @@
+const assert = require('assert');
+
+import SubtitleTrackController from '../../../src/controller/subtitle-track-controller';
+import Hls from '../../../src/hls';
+
+describe('SubtitleTrackController', () => {
+
+  let subtitleTrackController;
+  let videoElement;
+
+  beforeEach(() => {
+    const hls = new Hls();
+
+    videoElement = document.createElement('video');
+    subtitleTrackController = new SubtitleTrackController(hls);
+
+    subtitleTrackController.media = videoElement;
+    subtitleTrackController.tracks = [{ id: 0 }, { id: 1 }];
+
+    const textTrack1 = videoElement.addTextTrack('subtitles', 'English', 'en');
+    const textTrack2 = videoElement.addTextTrack('subtitles', 'Swedish', 'se');
+
+    textTrack1.mode = 'disabled';
+    textTrack2.mode = 'disabled';
+  });
+
+  describe('onTextTrackChanged', () => {
+    it('should set subtitleTrack to -1 if disabled', () => {
+      assert.strictEqual(subtitleTrackController.subtitleTrack, -1);
+
+      videoElement.textTracks[0].mode = 'disabled';
+      subtitleTrackController._onTextTracksChanged();
+
+      assert.strictEqual(subtitleTrackController.subtitleTrack, -1);
+    });
+
+    it('should set subtitleTrack to 0 if hidden', () => {
+      assert.strictEqual(subtitleTrackController.subtitleTrack, -1);
+
+      videoElement.textTracks[0].mode = 'hidden';
+      subtitleTrackController._onTextTracksChanged();
+
+      assert.strictEqual(subtitleTrackController.subtitleTrack, 0);
+    });
+
+    it('should set subtitleTrack to 0 if showing', () => {
+      assert.strictEqual(subtitleTrackController.subtitleTrack, -1);
+
+      videoElement.textTracks[0].mode = 'showing';
+      subtitleTrackController._onTextTracksChanged();
+
+      assert.strictEqual(subtitleTrackController.subtitleTrack, 0);
+    });
+  });
+
+  describe('setSubtitleTrackInternal', () => {
+    it('should set active text track mode to showing', () => {
+      videoElement.textTracks[0].mode = 'disabled';
+
+      subtitleTrackController.subtitleDisplay = true;
+      subtitleTrackController.subtitleTrack = 0;
+
+      assert.strictEqual(videoElement.textTracks[0].mode, 'showing');
+    });
+
+    it('should set active text track mode to hidden', () => {
+      videoElement.textTracks[0].mode = 'disabled';
+
+      subtitleTrackController.subtitleDisplay = false;
+      subtitleTrackController.subtitleTrack = 0;
+
+      assert.strictEqual(videoElement.textTracks[0].mode, 'hidden');
+    });
+
+    it('should disable previous track', () => {
+      // Change active track without triggering setSubtitleTrackInternal
+      subtitleTrackController.trackId = 0;
+
+      // Change active track and trigger setSubtitleTrackInternal
+      subtitleTrackController.subtitleTrack = 1;
+
+      assert.strictEqual(videoElement.textTracks[0].mode, 'disabled');
+    });
+  });
+});

tests/unit/controller/timeline-controller.js

@@ -0,0 +1,40 @@
+const assert = require('assert');
+
+import TimelineController from '../../../src/controller/timeline-controller';
+import Hls from '../../../src/hls';
+
+describe('TimelineController', () => {
+
+  let timelineController;
+  let hls;
+
+  beforeEach(() => {
+    hls = new Hls();
+    hls.config.enableWebVTT = true;
+    timelineController = new TimelineController(hls);
+    timelineController.media = document.createElement('video');
+
+  });
+
+  it('should set default track to showing when displaySubtitles is true', () => {
+    hls.subtitleTrackController = { subtitleDisplay: true };
+
+    timelineController.onManifestLoaded({
+      subtitles: [{ id: 0 }, { id: 1, default: true }]
+    });
+
+    assert.strictEqual(timelineController.textTracks[0].mode, 'disabled');
+    assert.strictEqual(timelineController.textTracks[1].mode, 'showing');
+  });
+
+  it('should set default track to hidden when displaySubtitles is false', () => {
+    hls.subtitleTrackController = { subtitleDisplay: false };
+
+    timelineController.onManifestLoaded({
+      subtitles: [{ id: 0 }, { id: 1, default: true }]
+    });
+
+    assert.strictEqual(timelineController.textTracks[0].mode, 'disabled');
+    assert.strictEqual(timelineController.textTracks[1].mode, 'hidden');
+  });
+});