Fix text track language selection logic

https://bugs.webkit.org/show_bug.cgi?id=144467

Reviewed by Brent Fulgham.

Source/WebCore:

No new tests, media/track/track-language-preference.html was updated.

* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::configureTextTrackGroup): Correct a minor style typo.
(WebCore::HTMLMediaElement::configureTextTrackDisplay): Update logging.

* page/CaptionUserPreferences.cpp:
(WebCore::CaptionUserPreferences::textTrackSelectionScore): Minor cleanup.
(WebCore::CaptionUserPreferences::textTrackLanguageSelectionScore): Give exact matches a
higher score.

* page/CaptionUserPreferencesMediaAF.cpp:
(WebCore::CaptionUserPreferencesMediaAF::textTrackSelectionScore): Update for
indexOfBestMatchingLanguageInList change.

* platform/Language.cpp:
(WebCore::indexOfBestMatchingLanguageInList): Add parameter for exact match. Convert the
passed language to lower case as we do with the preferred languages.
* platform/Language.h:

LayoutTests:

* media/track/track-language-preference-expected.txt:
* media/track/track-language-preference.html: Updated, add new tests.
* platform/mac/TestExpectations:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@183676 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index e685096..cd2a232 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,14 @@
+2015-05-01  Eric Carlson  <eric.carlson@apple.com>
+
+        Fix text track language selection logic
+        https://bugs.webkit.org/show_bug.cgi?id=144467
+
+        Reviewed by Brent Fulgham.
+
+        * media/track/track-language-preference-expected.txt:
+        * media/track/track-language-preference.html: Updated, add new tests.
+        * platform/mac/TestExpectations:
+
 2015-05-01  Martin Robinson  <mrobinson@igalia.com>
 
         Unreviewed gardening. Rebaseline some tests for WebKitGTK+.

diff --git a/LayoutTests/media/track/track-language-preference-expected.txt b/LayoutTests/media/track/track-language-preference-expected.txt
index 03708fe..58affc3 100644 (file)
--- a/LayoutTests/media/track/track-language-preference-expected.txt
+++ b/LayoutTests/media/track/track-language-preference-expected.txt
@@ -3,7 +3,8 @@ Tests that the user's preferred languages are honored.
 
 **Set track preferences and user preferred languages
 RUN(internals.settings.setShouldDisplayTrackKind('Captions', true))
-RUN(internals.setUserPreferredLanguages(['jp', 'es-ES', 'en', 'fr']))
+RUN(internals.setUserPreferredLanguages(['jp', 'es-ES', 'en', 'fr', 'pt-PT']))
+EVENT(canplaythrough)
 
 Test: a track language matches one of the user's preferred languages exactly.
 - creating tracks for: [fr,en,jp].
@@ -29,6 +30,11 @@ EVENT(load)
 EXPECTED (track.readyState == '2') OK
 EXPECTED (track.srclang == 'fa') OK
 
-EXPECTED (video.textTracks.length == '3') OK
+Test: a track language without locale matches one of the user's preferred languages exactly when another language matches locale only.
+- creating tracks for: [pt-BR,nl-NL,pt-PT].
+EVENT(load)
+EXPECTED (track.readyState == '2') OK
+EXPECTED (track.srclang == 'pt-PT') OK
+
 END OF TEST
 

diff --git a/LayoutTests/media/track/track-language-preference.html b/LayoutTests/media/track/track-language-preference.html
index 0fa8b05..f8a66aa 100644 (file)
--- a/LayoutTests/media/track/track-language-preference.html
+++ b/LayoutTests/media/track/track-language-preference.html
@@ -11,7 +11,6 @@
         <script>
 
             var timer = null;
-            var expectedLanguage;
             var testList = 
             {
                 current : -1,
@@ -37,6 +36,11 @@
                         languages : ['fa', 'ru', 'no'], 
                         expectedLanguage : "fa", 
                     },
+                    {
+                        description: "a track language without locale matches one of the user's preferred languages exactly when another language matches locale only", 
+                        languages : ['pt-BR', 'nl-NL', 'pt-PT'], 
+                        expectedLanguage : "pt-PT", 
+                    },
                 ]
             };
 
@@ -45,8 +49,6 @@
                 consoleWrite("");
                 testList.current++;
                 if (testList.current >= testList.tests.length) {
-                    var tracks = document.querySelectorAll('track');
-                    testExpected("video.textTracks.length", tracks.length);
                     endTest();
                     return;
                 }
@@ -59,12 +61,10 @@
             {
                 consoleWrite("EVENT(load)");
                 
-                // Don't log the event name because the order of the two events in not predictable.
                 track = event.target;
                 testExpected("track.readyState", HTMLTrackElement.LOADED);
                 testExpected("track.srclang", testList.tests[testList.current].expectedLanguage);
-
-                timer = setTimeout(runNextTest, 200);
+                timer = setTimeout(runNextTest, 100);
             }
 
             function setPreferences()
@@ -76,7 +76,7 @@
 
                 consoleWrite("<i>**Set track preferences and user preferred languages<" + "/i>");
                 run("internals.settings.setShouldDisplayTrackKind('Captions', true)");
-                run("internals.setUserPreferredLanguages(['jp', 'es-ES', 'en', 'fr'])");
+                run("internals.setUserPreferredLanguages(['jp', 'es-ES', 'en', 'fr', 'pt-PT'])");
             }
             
             function createTrackElement(language, src)
@@ -102,18 +102,18 @@
 
             function setup()
             {
-                findMediaElement();
-
+                setCaptionDisplayMode('AlwaysOn');
                 setPreferences("Subtitles", true);
 
-                runNextTest();
+                findMediaElement();
+                video.src = findMediaFile('video', '../content/test');
+                waitForEvent('canplaythrough', runNextTest);
             }
 
         </script>
     </head>
     <body onload="setup()">
         <p>Tests that the user's preferred languages are honored.</p>
-        <video>
-        </video>
+        <video></video>
     </body>
 </html>

diff --git a/LayoutTests/platform/mac/TestExpectations b/LayoutTests/platform/mac/TestExpectations
index 4fb2923..ddfa547 100644 (file)
--- a/LayoutTests/platform/mac/TestExpectations
+++ b/LayoutTests/platform/mac/TestExpectations
@@ -988,7 +988,6 @@ webkit.org/b/142152 media/track/track-in-band-cues-added-once.html [ Pass Failur
 webkit.org/b/28221 media/audio-delete-while-step-button-clicked.html [ Failure ]
 
 # These tests always time out.
-webkit.org/b/112492 media/track/track-language-preference.html [ Skip ]
 webkit.org/b/112492 media/track/track-prefer-captions.html [ Skip ]
 
 webkit.org/b/136708 http/tests/media/video-play-stall-seek.html

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index edaf0dc..6dc161e 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,30 @@
+2015-05-01  Eric Carlson  <eric.carlson@apple.com>
+
+        Fix text track language selection logic
+        https://bugs.webkit.org/show_bug.cgi?id=144467
+
+        Reviewed by Brent Fulgham.
+
+        No new tests, media/track/track-language-preference.html was updated.
+
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::configureTextTrackGroup): Correct a minor style typo.
+        (WebCore::HTMLMediaElement::configureTextTrackDisplay): Update logging.
+
+        * page/CaptionUserPreferences.cpp:
+        (WebCore::CaptionUserPreferences::textTrackSelectionScore): Minor cleanup.
+        (WebCore::CaptionUserPreferences::textTrackLanguageSelectionScore): Give exact matches a
+        higher score.
+
+        * page/CaptionUserPreferencesMediaAF.cpp:
+        (WebCore::CaptionUserPreferencesMediaAF::textTrackSelectionScore): Update for 
+        indexOfBestMatchingLanguageInList change.
+
+        * platform/Language.cpp:
+        (WebCore::indexOfBestMatchingLanguageInList): Add parameter for exact match. Convert the
+        passed language to lower case as we do with the preferred languages.
+        * platform/Language.h:
+
 2015-05-01  Martin Robinson  <mrobinson@igalia.com>
 
         [Freetype] Add support for the font-synthesis property

diff --git a/Source/WebCore/html/HTMLMediaElement.cpp b/Source/WebCore/html/HTMLMediaElement.cpp
index 3b9ae54..982f7b2 100644 (file)
--- a/Source/WebCore/html/HTMLMediaElement.cpp
+++ b/Source/WebCore/html/HTMLMediaElement.cpp
@@ -3563,7 +3563,7 @@ void HTMLMediaElement::configureTextTrackGroup(const TrackGroup& group)
     LOG(Media, "HTMLMediaElement::configureTextTrackGroup(%p)", this);
 
     Page* page = document().page();
-    CaptionUserPreferences* captionPreferences = page? page->group().captionPreferences() : 0;
+    CaptionUserPreferences* captionPreferences = page ? page->group().captionPreferences() : 0;
     CaptionUserPreferences::CaptionDisplayMode displayMode = captionPreferences ? captionPreferences->captionDisplayMode() : CaptionUserPreferences::Automatic;
 
     // First, find the track in the group that should be enabled (if any).
@@ -5357,10 +5357,11 @@ void HTMLMediaElement::configureTextTrackDisplay(TextTrackVisibilityCheckType ch
     if (m_processingPreferenceChange)
         return;
 
-    LOG(Media, "HTMLMediaElement::configureTextTrackDisplay(%p)", this);
+    LOG(Media, "HTMLMediaElement::configureTextTrackDisplay(%p) - checkType = %s", this, checkType == CheckTextTrackVisibility ? "check-visibility" : "assume-visibility-changed");
 
     bool haveVisibleTextTrack = false;
     for (unsigned i = 0; i < m_textTracks->length(); ++i) {
+        LOG(Media, "     track[%i]->mode = %s", i, String(m_textTracks->item(i)->mode()).utf8().data());
         if (m_textTracks->item(i)->mode() == TextTrack::showingKeyword()) {
             haveVisibleTextTrack = true;
             break;

diff --git a/Source/WebCore/page/CaptionUserPreferences.cpp b/Source/WebCore/page/CaptionUserPreferences.cpp
index 0ed2ca4..d7280f8 100644 (file)
--- a/Source/WebCore/page/CaptionUserPreferences.cpp
+++ b/Source/WebCore/page/CaptionUserPreferences.cpp
@@ -234,20 +234,13 @@ Vector<RefPtr<AudioTrack>> CaptionUserPreferences::sortedTrackListForMenu(AudioT
 
 int CaptionUserPreferences::textTrackSelectionScore(TextTrack* track, HTMLMediaElement*) const
 {
-    int trackScore = 0;
-
     if (track->kind() != TextTrack::captionsKeyword() && track->kind() != TextTrack::subtitlesKeyword())
-        return trackScore;
+        return 0;
     
     if (!userPrefersSubtitles() && !userPrefersCaptions())
-        return trackScore;
-    
-    if (track->kind() == TextTrack::subtitlesKeyword() && userPrefersSubtitles())
-        trackScore = 1;
-    else if (track->kind() == TextTrack::captionsKeyword() && userPrefersCaptions())
-        trackScore = 1;
+        return 0;
     
-    return trackScore + textTrackLanguageSelectionScore(track, preferredLanguages());
+    return textTrackLanguageSelectionScore(track, preferredLanguages()) + 1;
 }
 
 int CaptionUserPreferences::textTrackLanguageSelectionScore(TextTrack* track, const Vector<String>& preferredLanguages) const
@@ -255,13 +248,15 @@ int CaptionUserPreferences::textTrackLanguageSelectionScore(TextTrack* track, co
     if (track->language().isEmpty())
         return 0;
 
-    size_t languageMatchIndex = indexOfBestMatchingLanguageInList(track->language(), preferredLanguages);
+    bool exactMatch;
+    size_t languageMatchIndex = indexOfBestMatchingLanguageInList(track->language(), preferredLanguages, exactMatch);
     if (languageMatchIndex >= preferredLanguages.size())
         return 0;
 
     // Matching a track language is more important than matching track type, so this multiplier must be
     // greater than the maximum value returned by textTrackSelectionScore.
-    return (preferredLanguages.size() - languageMatchIndex) * 10;
+    int bonus = exactMatch ? 1 : 0;
+    return (preferredLanguages.size() + bonus - languageMatchIndex) * 10;
 }
 
 void CaptionUserPreferences::setCaptionsStyleSheetOverride(const String& override)

diff --git a/Source/WebCore/page/CaptionUserPreferencesMediaAF.cpp b/Source/WebCore/page/CaptionUserPreferencesMediaAF.cpp
index 5b1535d..30aa5e5 100644 (file)
--- a/Source/WebCore/page/CaptionUserPreferencesMediaAF.cpp
+++ b/Source/WebCore/page/CaptionUserPreferencesMediaAF.cpp
@@ -663,9 +663,10 @@ int CaptionUserPreferencesMediaAF::textTrackSelectionScore(TextTrack* track, HTM
         if (audioTrackLanguage.isEmpty())
             return 0;
 
+        bool exactMatch;
         if (trackHasOnlyForcedSubtitles) {
             languageList.append(audioTrackLanguage);
-            size_t offset = indexOfBestMatchingLanguageInList(textTrackLanguage, languageList);
+            size_t offset = indexOfBestMatchingLanguageInList(textTrackLanguage, languageList, exactMatch);
 
             // Only consider a forced-only track if it IS in the same language as the primary audio track.
             if (offset)
@@ -674,12 +675,12 @@ int CaptionUserPreferencesMediaAF::textTrackSelectionScore(TextTrack* track, HTM
             languageList.append(defaultLanguage());
 
             // Only enable a text track if the current audio track is NOT in the user's preferred language ...
-            size_t offset = indexOfBestMatchingLanguageInList(audioTrackLanguage, languageList);
+            size_t offset = indexOfBestMatchingLanguageInList(audioTrackLanguage, languageList, exactMatch);
             if (!offset)
                 return 0;
 
             // and the text track matches the user's preferred language.
-            offset = indexOfBestMatchingLanguageInList(textTrackLanguage, languageList);
+            offset = indexOfBestMatchingLanguageInList(textTrackLanguage, languageList, exactMatch);
             if (offset)
                 return 0;
         }

diff --git a/Source/WebCore/platform/Language.cpp b/Source/WebCore/platform/Language.cpp
index c2892e4..53785e1 100644 (file)
--- a/Source/WebCore/platform/Language.cpp
+++ b/Source/WebCore/platform/Language.cpp
@@ -105,22 +105,25 @@ static String canonicalLanguageIdentifier(const String& languageCode)
     return lowercaseLanguageCode;
 }
 
-size_t indexOfBestMatchingLanguageInList(const String& language, const Vector<String>& languageList)
+size_t indexOfBestMatchingLanguageInList(const String& language, const Vector<String>& languageList, bool& exactMatch)
 {
+    String lowercaseLanguage = language.lower();
     String languageWithoutLocaleMatch;
     String languageMatchButNotLocale;
     size_t languageWithoutLocaleMatchIndex = 0;
     size_t languageMatchButNotLocaleMatchIndex = 0;
-    bool canMatchLanguageOnly = (language.length() == 2 || (language.length() >= 3 && language[2] == '-'));
+    bool canMatchLanguageOnly = (lowercaseLanguage.length() == 2 || (lowercaseLanguage.length() >= 3 && lowercaseLanguage[2] == '-'));
 
     for (size_t i = 0; i < languageList.size(); ++i) {
         String canonicalizedLanguageFromList = canonicalLanguageIdentifier(languageList[i]);
 
-        if (language == canonicalizedLanguageFromList)
+        if (lowercaseLanguage == canonicalizedLanguageFromList) {
+            exactMatch = true;
             return i;
+        }
 
         if (canMatchLanguageOnly && canonicalizedLanguageFromList.length() >= 2) {
-            if (language[0] == canonicalizedLanguageFromList[0] && language[1] == canonicalizedLanguageFromList[1]) {
+            if (lowercaseLanguage[0] == canonicalizedLanguageFromList[0] && lowercaseLanguage[1] == canonicalizedLanguageFromList[1]) {
                 if (!languageWithoutLocaleMatch.length() && canonicalizedLanguageFromList.length() == 2) {
                     languageWithoutLocaleMatch = languageList[i];
                     languageWithoutLocaleMatchIndex = i;
@@ -133,6 +136,8 @@ size_t indexOfBestMatchingLanguageInList(const String& language, const Vector<St
         }
     }
 
+    exactMatch = false;
+
     // If we have both a language-only match and a languge-but-not-locale match, return the
     // languge-only match as is considered a "better" match. For example, if the list
     // provided has both "en-GB" and "en" and the user prefers "en-US" we will return "en".

diff --git a/Source/WebCore/platform/Language.h b/Source/WebCore/platform/Language.h
index 3185f86..4edbe7e 100644 (file)
--- a/Source/WebCore/platform/Language.h
+++ b/Source/WebCore/platform/Language.h
@@ -35,7 +35,7 @@ WEBCORE_EXPORT String defaultLanguage();
 WEBCORE_EXPORT Vector<String> userPreferredLanguages();
 Vector<String> userPreferredLanguagesOverride();
 WEBCORE_EXPORT void overrideUserPreferredLanguages(const Vector<String>&);
-size_t indexOfBestMatchingLanguageInList(const String& language, const Vector<String>& languageList);
+size_t indexOfBestMatchingLanguageInList(const String& language, const Vector<String>& languageList, bool& exactMatch);
 
 // The observer function will be called when system language changes.
 typedef void (*LanguageChangeObserverFunction)(void* context);