Update: Improve subtitle selection algorithm (#129)

When selecting subtitles from scratch (toggle-on button) we were
previously selecting the first subtitle in the list. Instead, in this
commit we will go by the user's locale - if the user's language isn't
available, we fall back to English, and if English isn't available, we
fall back to the first language in the subtitles menu. This is similar
to what YouTube does.

src/lib/viewers/media/DashViewer.js

@@ -313,7 +313,7 @@ class DashViewer extends VideoBaseViewer {
     loadSubtitles() {
         this.textTracks = this.player.getTextTracks().sort((track1, track2) => track1.id - track2.id);
         if (this.textTracks.length > 0) {
-            this.mediaControls.initSubtitles(this.textTracks.map((track) => getLanguageName(track.language) || track.language));
+            this.mediaControls.initSubtitles(this.textTracks.map((track) => getLanguageName(track.language) || track.language), getLanguageName(this.options.location.locale.substring(0, 2)));
         }
     }
     /**

src/lib/viewers/media/MediaControls.js

@@ -805,11 +805,12 @@ class MediaControls extends EventEmitter {
      * Takes a list of subtitle names and populates the settings menu
      *
      * @param {Array} subtitles - A list of subtitle names as strings
+     * @param {string} language - Language of the user, as derived from their locale (e.g. en-US -> "English")
      * @return {void}
      */
-    initSubtitles(subtitles) {
+    initSubtitles(subtitles, language) {
         this.settings.addListener('subtitles', this.handleSubtitle);
-        this.settings.loadSubtitles(subtitles);
+        this.settings.loadSubtitles(subtitles, language);
     }
 }
 

src/lib/viewers/media/Settings.js

@@ -126,9 +126,10 @@ class Settings extends EventEmitter {
 
         addActivationListener(this.settingsEl, this.menuEventHandler);
         this.visible = false;
-        this.hasSubtitles = false;
         this.containerEl.classList.add(CLASS_SETTINGS_SUBTITLES_UNAVAILABLE);
-        this.toggleToSubtitle = 0; // An index into the subtitles track list. Initialize with the first subtitle in list
+        this.subtitles = [];
+        this.language = undefined;
+        this.toggleToSubtitle = undefined; // An index into the subtitles list. Initialize with sentinel value
         this.init();
     }
 
@@ -510,6 +511,18 @@ class Settings extends EventEmitter {
         return selected.getAttribute('data-value') !== '-1';
     }
 
+    /**
+     * Returns whether subtitles are available or not
+     *
+     * @return {boolean}
+     */
+    hasSubtitles() {
+        if (this.settingsEl.querySelector(`[data-type="subtitles"][data-value="0"]${SELECTOR_SETTINGS_SUB_ITEM}`)) {
+            return true;
+        }
+        return false;
+    }
+
     /**
      * Toggles subtitles on/off
      *
@@ -518,27 +531,40 @@ class Settings extends EventEmitter {
     toggleSubtitles() {
         if (this.areSubtitlesOn()) {
             this.chooseOption('subtitles', '-1');
-        } else if (this.hasSubtitles) {
+        } else if (this.toggleToSubtitle !== undefined) {
             this.chooseOption('subtitles', this.toggleToSubtitle.toString());
+        } else { // Do intelligent selection: Prefer user's language, fallback to English, then first subtitle in list
+            // Use the previewer's locale to determine preferred language
+            let idx = this.subtitles.findIndex((subtitle) => subtitle === this.language);
+            if (idx === -1) {
+                // Fall back to English if user's language doesn't exist
+                idx = this.subtitles.findIndex((subtitle) => subtitle === 'English');
+                if (idx === -1) {
+                    idx = 0; // Fall back to first subtitle in list
+                }
+            }
+            this.chooseOption('subtitles', idx.toString());
         }
     }
 
     /**
      * Takes a list of subtitle names and populates the settings menu
      *
      * @param {Array} subtitles - A list of subtitle names as strings
+     * @param {string} language - The language of the user. Used in determining preferred subtitle when toggling
      * @return {void}
      */
-    loadSubtitles(subtitles) {
+    loadSubtitles(subtitles, language) {
         const subtitlesSubMenu = this.settingsEl.querySelector('.bp-media-settings-menu-subtitles');
-        subtitles.forEach((subtitle, idx) => {
+        this.subtitles = subtitles;
+        this.language = language;
+        this.subtitles.forEach((subtitle, idx) => {
             insertTemplate(subtitlesSubMenu, SUBTITLES_SUBITEM_TEMPLATE.replace(/{{dataValue}}/g, idx));
             const languageNode = subtitlesSubMenu.lastChild.querySelector('.bp-media-settings-value');
             languageNode.textContent = subtitle;
         });
 
         this.containerEl.classList.remove(CLASS_SETTINGS_SUBTITLES_UNAVAILABLE);
-        this.hasSubtitles = true;
         const subsCache = cache.get('media-subtitles');
         if (subsCache !== null && subsCache !== '-1') { // Last video watched with subtitles, so turn them on here too
             this.toggleSubtitles();

src/lib/viewers/media/__tests__/DashViewer-test.js

@@ -34,6 +34,7 @@ describe('lib/viewers/media/DashViewer', () => {
                 }
             },
             container: containerEl,
+            location: { locale: 'en-US' },
             representation: {
                 content: {
                     url_template: 'url'
@@ -464,7 +465,7 @@ describe('lib/viewers/media/DashViewer', () => {
                 chinese
             ];
             stubs.mockPlayer.expects('getTextTracks').returns(subs);
-            stubs.mockControls.expects('initSubtitles').withArgs(['Korean', 'Russian', 'English', 'Spanish', 'Chinese']);
+            stubs.mockControls.expects('initSubtitles').withArgs(['Korean', 'Russian', 'English', 'Spanish', 'Chinese'], 'English');
 
             dash.loadSubtitles();
 
@@ -483,7 +484,7 @@ describe('lib/viewers/media/DashViewer', () => {
                 chinese
             ];
             stubs.mockPlayer.expects('getTextTracks').returns(subs);
-            stubs.mockControls.expects('initSubtitles').withArgs(['Russian', 'Spanish', 'Korean', 'Chinese']);
+            stubs.mockControls.expects('initSubtitles').withArgs(['Russian', 'Spanish', 'Korean', 'Chinese'], 'English');
 
             dash.loadSubtitles();
 
@@ -506,7 +507,7 @@ describe('lib/viewers/media/DashViewer', () => {
                 zero
             ];
             stubs.mockPlayer.expects('getTextTracks').returns(subs);
-            stubs.mockControls.expects('initSubtitles').withArgs(['Russian', 'foo', 'und', '', 'doesntmatter', '0']);
+            stubs.mockControls.expects('initSubtitles').withArgs(['Russian', 'foo', 'und', '', 'doesntmatter', '0'], 'English');
 
             dash.loadSubtitles();
 

src/lib/viewers/media/__tests__/Settings-test.js

@@ -33,7 +33,7 @@ describe('lib/viewers/media/Settings', () => {
 
         it('should initialize as invisible and without subtitles', () => {
             expect(settings.visible).to.be.false;
-            expect(settings.hasSubtitles).to.be.false;
+            expect(settings.hasSubtitles()).to.be.false;
             expect(settings.areSubtitlesOn()).to.be.false;
             expect(settings.containerEl).to.have.class('bp-media-settings-subtitles-unavailable');
         });
@@ -602,6 +602,7 @@ describe('lib/viewers/media/Settings', () => {
     describe('toggleSubtitles()', () => {
         it('Should turn off subtitles if they were previously on', () => {
             sandbox.stub(settings, 'chooseOption');
+            sandbox.stub(settings, 'hasSubtitles').returns(true);
             sandbox.stub(settings, 'areSubtitlesOn').returns(true);
 
             settings.toggleSubtitles();
@@ -610,15 +611,51 @@ describe('lib/viewers/media/Settings', () => {
         });
 
         it('Should turn on subtitles if they were previously off', () => {
-            settings.hasSubtitles = true;
             sandbox.stub(settings, 'chooseOption');
+            sandbox.stub(settings, 'hasSubtitles').returns(true);
             sandbox.stub(settings, 'areSubtitlesOn').returns(false);
             settings.toggleToSubtitle = '2';
 
             settings.toggleSubtitles();
 
             expect(settings.chooseOption).to.be.calledWith('subtitles', '2');
         });
+
+        it('Should prefer subtitle matching previewer language/locale', () => {
+            sandbox.stub(settings, 'chooseOption');
+            sandbox.stub(settings, 'hasSubtitles').returns(true);
+            sandbox.stub(settings, 'areSubtitlesOn').returns(false);
+            settings.subtitles = ['English', 'Spanish', 'Russian', 'French'];
+            settings.language = 'Spanish';
+
+            settings.toggleSubtitles();
+
+            expect(settings.chooseOption).to.be.calledWith('subtitles', '1');
+        });
+
+        it('Should prefer English subtitle if previewer language not in list', () => {
+            sandbox.stub(settings, 'chooseOption');
+            sandbox.stub(settings, 'hasSubtitles').returns(true);
+            sandbox.stub(settings, 'areSubtitlesOn').returns(false);
+            settings.subtitles = ['Spanish', 'Russian', 'English', 'French'];
+            settings.language = 'Mongolian';
+
+            settings.toggleSubtitles();
+
+            expect(settings.chooseOption).to.be.calledWith('subtitles', '2');
+        });
+
+        it('Should prefer first subtitle in list if previewer language not in list and English absent', () => {
+            sandbox.stub(settings, 'chooseOption');
+            sandbox.stub(settings, 'hasSubtitles').returns(true);
+            sandbox.stub(settings, 'areSubtitlesOn').returns(false);
+            settings.subtitles = ['Spanish', 'Russian', 'French'];
+            settings.language = 'Mongolian';
+
+            settings.toggleSubtitles();
+
+            expect(settings.chooseOption).to.be.calledWith('subtitles', '0');
+        });
     });
 
     describe('loadSubtitles()', () => {
@@ -628,7 +665,7 @@ describe('lib/viewers/media/Settings', () => {
             settings.loadSubtitles(['English', 'Russian', 'Spanish']);
 
             expect(subsMenu.children.length).to.equal(5); // Three languages, 'Off', and back to main menu
-            expect(settings.hasSubtitles).to.be.true;
+            expect(settings.hasSubtitles()).to.be.true;
             expect(settings.containerEl).to.not.have.class('bp-media-settings-subtitles-unavailable');
         });
 
@@ -672,6 +709,18 @@ describe('lib/viewers/media/Settings', () => {
         });
     });
 
+    describe('hasSubtitles()', () => {
+        it('Should be false before loading subtitles', () => {
+            expect(settings.hasSubtitles()).to.be.false;
+        });
+
+        it('Should be true after loading subtitles', () => {
+            settings.loadSubtitles(['English']);
+
+            expect(settings.hasSubtitles()).to.be.true;
+        });
+    });
+
     describe('blurHandler()', () => {
         it('should hide if click is outside of settings element', () => {
             sandbox.stub(settings, 'hide');