Title: [152671] branches/safari-537-branch

Diff

Modified: branches/safari-537-branch/LayoutTests/ChangeLog (152670 => 152671)


--- branches/safari-537-branch/LayoutTests/ChangeLog	2013-07-15 23:31:17 UTC (rev 152670)
+++ branches/safari-537-branch/LayoutTests/ChangeLog	2013-07-15 23:34:23 UTC (rev 152671)
@@ -1,5 +1,25 @@
 2013-07-15  Lucas Forschler  <lforsch...@apple.com>
 
+        Merge r152547
+
+    2013-07-10  Eric Carlson  <eric.carl...@apple.com>
+
+            [Mac] every enabled text track should be listed in the track menu
+            https://bugs.webkit.org/show_bug.cgi?id=118477
+
+            Reviewed by Jer Noble.
+
+            * media/trackmenu-test.js: Add some new utility functions.
+
+            * media/video-controls-captions-trackmenu-includes-enabled-track-expected.txt: Added.
+            * media/video-controls-captions-trackmenu-includes-enabled-track.html: Added.
+            * platform/efl/TestExpectations:
+            * platform/gtk/TestExpectations:
+            * platform/qt/TestExpectations:
+            * platform/win/TestExpectations:
+
+2013-07-15  Lucas Forschler  <lforsch...@apple.com>
+
         Merge r152532
 
     2013-07-10  James Craig  <ja...@cookiecrook.com>

Modified: branches/safari-537-branch/LayoutTests/media/trackmenu-test.js (152670 => 152671)


--- branches/safari-537-branch/LayoutTests/media/trackmenu-test.js	2013-07-15 23:31:17 UTC (rev 152670)
+++ branches/safari-537-branch/LayoutTests/media/trackmenu-test.js	2013-07-15 23:34:23 UTC (rev 152671)
@@ -89,3 +89,29 @@
     eventSender.mouseUp();
 }
 
+function showTrackMenu()
+{
+    clickCCButton();
+}
+
+function hideTrackMenu()
+{
+    if (!window.eventSender)
+        return;
+
+    eventSender.mouseMoveTo(1, 1);
+    eventSender.mouseDown();
+    eventSender.mouseUp();
+}
+
+function listTrackMenu()
+{
+    var trackListItems = trackMenuList();
+    consoleWrite("Track menu:");
+    for (i = 0; i < trackListItems.length; i++) {
+        var logString = i + ": \"" + trackListItems[i].textContent + "\"";
+        if (trackListItems[i].className == "selected")
+            logString += ", checked";
+        consoleWrite(logString);
+    }
+}

Copied: branches/safari-537-branch/LayoutTests/media/video-controls-captions-trackmenu-includes-enabled-track-expected.txt (from rev 152547, trunk/LayoutTests/media/video-controls-captions-trackmenu-includes-enabled-track-expected.txt) (0 => 152671)


--- branches/safari-537-branch/LayoutTests/media/video-controls-captions-trackmenu-includes-enabled-track-expected.txt	                        (rev 0)
+++ branches/safari-537-branch/LayoutTests/media/video-controls-captions-trackmenu-includes-enabled-track-expected.txt	2013-07-15 23:34:23 UTC (rev 152671)
@@ -0,0 +1,30 @@
+Test that all enabled tracks are included in the track menu.
+
+EVENT(canplaythrough)
+
+*** Initially both tracks are disabled so neither should be selected.
+EXPECTED (video.textTracks[0].mode == 'disabled') OK
+EXPECTED (video.textTracks[1].mode == 'disabled') OK
+
+Track menu:
+0: "Off", checked
+1: "Auto (Recommended)"
+2: "English Captions 1"
+3: "English Captions 2"
+
+*** Enable both tracks.
+RUN(video.textTracks[0].mode = 'showing')
+RUN(video.textTracks[1].mode = 'showing')
+
+*** Both tracks are enabled so both should be selected.
+EXPECTED (video.textTracks[0].mode == 'showing') OK
+EXPECTED (video.textTracks[1].mode == 'showing') OK
+
+Track menu:
+0: "Off"
+1: "Auto (Recommended)"
+2: "English Captions 1", checked
+3: "English Captions 2", checked
+
+END OF TEST
+

Copied: branches/safari-537-branch/LayoutTests/media/video-controls-captions-trackmenu-includes-enabled-track.html (from rev 152547, trunk/LayoutTests/media/video-controls-captions-trackmenu-includes-enabled-track.html) (0 => 152671)


--- branches/safari-537-branch/LayoutTests/media/video-controls-captions-trackmenu-includes-enabled-track.html	                        (rev 0)
+++ branches/safari-537-branch/LayoutTests/media/video-controls-captions-trackmenu-includes-enabled-track.html	2013-07-15 23:34:23 UTC (rev 152671)
@@ -0,0 +1,65 @@
+<!DOCTYPE html>
+<html>
+    <head>
+        <title>Test that all enabled tracks are included in the track menu</title>
+        <script src=""
+        <script src=""
+        <script src=""
+        <script src=""
+        <script>
+            function startTest()
+            {
+                showTrackMenu();
+                window.setTimeout(testInitialState, 50);
+            }
+
+            function testInitialState()
+            {
+                trackListItems = trackMenuList();
+                consoleWrite("<br>*** Initially both tracks are disabled so neither should be selected.");
+                testExpected("video.textTracks[0].mode", "disabled");
+                testExpected("video.textTracks[1].mode", "disabled");
+                consoleWrite("");
+                listTrackMenu();
+
+                consoleWrite("<br>*** Enable both tracks.");
+                run("video.textTracks[0].mode = 'showing'");
+                run("video.textTracks[1].mode = 'showing'");
+
+                // Hide and show the track menu so it will be rebuilt.
+                window.setTimeout(function() { testMenuAfterEnablingTracks(); }, 50);
+                hideTrackMenu();
+                showTrackMenu();
+            }
+
+            function testMenuAfterEnablingTracks()
+            {
+                consoleWrite("<br>*** Both tracks are enabled so both should be selected.");
+                trackListItems = trackMenuList();
+                testExpected("video.textTracks[0].mode", "showing");
+                testExpected("video.textTracks[1].mode", "showing");
+                consoleWrite("");
+                listTrackMenu();
+
+                consoleWrite("");
+                endTest();
+            }
+
+            function start()
+            {
+                findMediaElement();
+                video.src = "" 'content/test');
+                waitForEvent('canplaythrough', startTest);
+            }
+        </script>
+    </head>
+
+    <body _onload_="start()">
+        <p>Test that all enabled tracks are included in the track menu.</p>
+        <video width="500" height="300" controls>
+            <track kind="captions" label="English Captions 1" src="" srclang="en">
+            <track kind="captions" label="English Captions 2" src="" srclang="en">
+        </video>
+    </body>
+</html>
+

Modified: branches/safari-537-branch/LayoutTests/platform/efl/TestExpectations (152670 => 152671)


--- branches/safari-537-branch/LayoutTests/platform/efl/TestExpectations	2013-07-15 23:31:17 UTC (rev 152670)
+++ branches/safari-537-branch/LayoutTests/platform/efl/TestExpectations	2013-07-15 23:34:23 UTC (rev 152671)
@@ -1543,6 +1543,7 @@
 webkit.org/b/101670 media/video-controls-captions-trackmenu-hide-on-click.html [ Skip ]
 webkit.org/b/101670 media/video-controls-captions-trackmenu-hide-on-click-outside.html [ Skip ]
 webkit.org/b/101670 media/track/track-user-preferences.html [ Skip ]
+webkit.org/b/101670 media/video-controls-captions-trackmenu-includes-enabled-track.html [ Skip ]
 
 # Fails until we enable the Resource Timing API.
 webkit.org/b/61138 http/tests/w3c/webperf/submission/Google/resource-timing [ Skip ]

Modified: branches/safari-537-branch/LayoutTests/platform/gtk/TestExpectations (152670 => 152671)


--- branches/safari-537-branch/LayoutTests/platform/gtk/TestExpectations	2013-07-15 23:31:17 UTC (rev 152670)
+++ branches/safari-537-branch/LayoutTests/platform/gtk/TestExpectations	2013-07-15 23:34:23 UTC (rev 152671)
@@ -759,6 +759,7 @@
 webkit.org/b/101670 media/video-controls-captions-trackmenu-hide-on-click.html [ Skip ]
 webkit.org/b/101670 media/video-controls-captions-trackmenu-hide-on-click-outside.html [ Skip ]
 webkit.org/b/101670 media/track/track-user-preferences.html [ Skip ]
+webkit.org/b/101670 media/video-controls-captions-trackmenu-includes-enabled-track.html [ Skip ]
 
 webkit.org/b/107194 storage/indexeddb/database-quota.html [ Timeout ]
 webkit.org/b/107194 storage/indexeddb/pending-activity-workers.html [ Timeout ]

Modified: branches/safari-537-branch/LayoutTests/platform/qt/TestExpectations (152670 => 152671)


--- branches/safari-537-branch/LayoutTests/platform/qt/TestExpectations	2013-07-15 23:31:17 UTC (rev 152670)
+++ branches/safari-537-branch/LayoutTests/platform/qt/TestExpectations	2013-07-15 23:34:23 UTC (rev 152671)
@@ -2522,6 +2522,7 @@
 webkit.org/b/101670 media/video-controls-captions-trackmenu-hide-on-click.html [ Skip ]
 webkit.org/b/101670 media/video-controls-captions-trackmenu-hide-on-click-outside.html [ Skip ]
 webkit.org/b/101670 media/track/track-user-preferences.html [ Skip ]
+webkit.org/b/101670 media/video-controls-captions-trackmenu-includes-enabled-track.html [ Skip ]
 
 # New, but failing test
 webkit.org/b/101035 fast/images/exif-orientation-image-document.html

Modified: branches/safari-537-branch/LayoutTests/platform/win/TestExpectations (152670 => 152671)


--- branches/safari-537-branch/LayoutTests/platform/win/TestExpectations	2013-07-15 23:31:17 UTC (rev 152670)
+++ branches/safari-537-branch/LayoutTests/platform/win/TestExpectations	2013-07-15 23:34:23 UTC (rev 152671)
@@ -2525,6 +2525,7 @@
 media/video-controls-captions-trackmenu-hide-on-click.html
 media/video-controls-captions-trackmenu-hide-on-click-outside.html
 media/video-controls-visible-exiting-fullscreen.html
+media/video-controls-captions-trackmenu-includes-enabled-track.html
 
 # Fails on JSC platforms due to gc timing issues
 # https://bugs.webkit.org/show_bug.cgi?id=106957

Modified: branches/safari-537-branch/Source/WebCore/ChangeLog (152670 => 152671)


--- branches/safari-537-branch/Source/WebCore/ChangeLog	2013-07-15 23:31:17 UTC (rev 152670)
+++ branches/safari-537-branch/Source/WebCore/ChangeLog	2013-07-15 23:34:23 UTC (rev 152671)
@@ -1,5 +1,26 @@
 2013-07-15  Lucas Forschler  <lforsch...@apple.com>
 
+        Merge r152547
+
+    2013-07-10  Eric Carlson  <eric.carl...@apple.com>
+
+            [Mac] every enabled text track should be listed in the track menu
+            https://bugs.webkit.org/show_bug.cgi?id=118477
+
+            Reviewed by Jer Noble.
+
+            Test: media/video-controls-captions-trackmenu-includes-enabled-track.html
+
+            * html/shadow/MediaControlElements.cpp:
+            (WebCore::MediaControlClosedCaptionsTrackListElement::updateDisplay): Don't select the
+                "Off" menu item if a track is enabled.
+
+            * page/CaptionUserPreferencesMediaAF.cpp:
+            (WebCore::CaptionUserPreferencesMediaAF::sortedTrackListForMenu): Always include a track
+                that is showing in the menu. Add more diagnostic logging.
+
+2013-07-15  Lucas Forschler  <lforsch...@apple.com>
+
         Merge r152532
 
     2013-07-10  James Craig  <ja...@cookiecrook.com>

Modified: branches/safari-537-branch/Source/WebCore/html/shadow/MediaControlElements.cpp (152670 => 152671)


--- branches/safari-537-branch/Source/WebCore/html/shadow/MediaControlElements.cpp	2013-07-15 23:31:17 UTC (rev 152670)
+++ branches/safari-537-branch/Source/WebCore/html/shadow/MediaControlElements.cpp	2013-07-15 23:34:23 UTC (rev 152671)
@@ -817,7 +817,6 @@
     if (!document()->page())
         return;
     CaptionUserPreferences::CaptionDisplayMode displayMode = document()->page()->group().captionPreferences()->captionDisplayMode();
-    bool trackIsSelected = displayMode != CaptionUserPreferences::Automatic && displayMode != CaptionUserPreferences::ForcedOnly;
 
     HTMLMediaElement* mediaElement = toParentMediaElement(this);
     if (!mediaElement)
@@ -829,6 +828,9 @@
 
     rebuildTrackListMenu();
 
+    RefPtr<Element> offMenuItem;
+    bool trackMenuItemSelected = false;
+
     for (unsigned i = 0, length = m_menuItems.size(); i < length; ++i) {
         RefPtr<Element> trackItem = m_menuItems[i];
 
@@ -841,10 +843,7 @@
             continue;
 
         if (textTrack == TextTrack::captionMenuOffItem()) {
-            if (displayMode == CaptionUserPreferences::ForcedOnly)
-                trackItem->classList()->add(selectedClassValue, ASSERT_NO_EXCEPTION);
-            else
-                trackItem->classList()->remove(selectedClassValue, ASSERT_NO_EXCEPTION);
+            offMenuItem = trackItem;
             continue;
         }
 
@@ -856,11 +855,19 @@
             continue;
         }
 
-        if (trackIsSelected && textTrack->mode() == TextTrack::showingKeyword())
+        if (displayMode != CaptionUserPreferences::Automatic && textTrack->mode() == TextTrack::showingKeyword()) {
+            trackMenuItemSelected = true;
             trackItem->classList()->add(selectedClassValue, ASSERT_NO_EXCEPTION);
-        else
+        } else
             trackItem->classList()->remove(selectedClassValue, ASSERT_NO_EXCEPTION);
     }
+
+    if (offMenuItem) {
+        if (displayMode == CaptionUserPreferences::ForcedOnly && !trackMenuItemSelected)
+            offMenuItem->classList()->add(selectedClassValue, ASSERT_NO_EXCEPTION);
+        else
+            offMenuItem->classList()->remove(selectedClassValue, ASSERT_NO_EXCEPTION);
+    }
 #endif
 }
 

Modified: branches/safari-537-branch/Source/WebCore/page/CaptionUserPreferencesMediaAF.cpp (152670 => 152671)


--- branches/safari-537-branch/Source/WebCore/page/CaptionUserPreferencesMediaAF.cpp	2013-07-15 23:31:17 UTC (rev 152670)
+++ branches/safari-537-branch/Source/WebCore/page/CaptionUserPreferencesMediaAF.cpp	2013-07-15 23:34:23 UTC (rev 152671)
@@ -794,16 +794,27 @@
         TextTrack* track = trackList->item(i);
         String language = displayNameForLanguageLocale(track->language());
 
-        if (track->containsOnlyForcedSubtitles())
+        if (track->containsOnlyForcedSubtitles()) {
+            LOG(Media, "CaptionUserPreferencesMac::sortedTrackListForMenu - skipping '%s' track with language '%s' because it contains only forced subtitles", track->kind().string().utf8().data(), language.utf8().data());
             continue;
+        }
         
         if (track->isEasyToRead()) {
+            LOG(Media, "CaptionUserPreferencesMac::sortedTrackListForMenu - adding '%s' track with language '%s' because it is 'easy to read'", track->kind().string().utf8().data(), language.utf8().data());
             if (!language.isEmpty())
                 languagesIncluded.add(language);
             tracksForMenu.append(track);
             continue;
         }
 
+        if (track->mode() == TextTrack::showingKeyword()) {
+            LOG(Media, "CaptionUserPreferencesMac::sortedTrackListForMenu - adding '%s' track with language '%s' because it is already visible", track->kind().string().utf8().data(), language.utf8().data());
+            if (!language.isEmpty())
+                languagesIncluded.add(language);
+            tracksForMenu.append(track);
+            continue;
+        }
+
         if (!language.isEmpty() && track->isMainProgramContent()) {
             bool isAccessibilityTrack = track->kind() == track->captionsKeyword();
             if (prefersAccessibilityTracks) {
@@ -828,6 +839,8 @@
         if (!language.isEmpty())
             languagesIncluded.add(language);
         tracksForMenu.append(track);
+
+        LOG(Media, "CaptionUserPreferencesMac::sortedTrackListForMenu - adding '%s' track with language '%s', is%s main program content", track->kind().string().utf8().data(), language.utf8().data(), track->isMainProgramContent() ? "" : " NOT");
     }
 
     // Now that we have filtered for the user's accessibility/translation preference, add  all tracks with a unique language without regard to track type.
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to