[Track] Closed Caption button shouldn't be visible if all the track resources have...
authorvcarbune@chromium.org <vcarbune@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 1 Feb 2013 04:03:04 +0000 (04:03 +0000)
committervcarbune@chromium.org <vcarbune@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 1 Feb 2013 04:03:04 +0000 (04:03 +0000)
https://bugs.webkit.org/show_bug.cgi?id=106285

Reviewed by Eric Carlson.

Source/WebCore:

Updated existing test cases.

* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::textTracksAreReady): Checked whether there are tracks not loaded
when the algorithm began.
(WebCore::HTMLMediaElement::textTrackReadyStateChanged): If the ready state changed to
FailedToLoad the media controls should check whether there are other caption tracks
and hide the button if not.
(WebCore::HTMLMediaElement::didAddTrack): Added trigger to closedCaptionsTrackChanged.
(WebCore::HTMLMediaElement::hasClosedCaptions): Updated check to skip tracks that
failed to load.
* html/shadow/MediaControls.cpp:
(WebCore::MediaControls::reset): Used the newly added method.
(WebCore::MediaControls::refreshClosedCaptionsButtonVisibility): Added container method for
default behaviour for refreshing the visibility of the caption button.
(WebCore::MediaControls::closedCaptionTracksChanged): Used the newly added method.
(WebCore):
* html/shadow/MediaControls.h:
(MediaControls):

* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::textTracksAreReady):
(WebCore::HTMLMediaElement::textTrackReadyStateChanged):
(WebCore::HTMLMediaElement::setReadyState):
(WebCore::HTMLMediaElement::didAddTrack):
(WebCore::HTMLMediaElement::hasClosedCaptions):
* html/shadow/MediaControls.cpp:
(WebCore::MediaControls::reset):
(WebCore::MediaControls::refreshClosedCaptionsButtonVisibility):
(WebCore::MediaControls::closedCaptionTracksChanged):
(WebCore):
* html/shadow/MediaControls.h:
(MediaControls):

LayoutTests:

Updated tests to include improved behavior.

* media/video-controls-captions-expected.txt: Updated.
* media/video-controls-captions.html: Updated.

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

LayoutTests/ChangeLog
LayoutTests/media/video-controls-captions-expected.txt
LayoutTests/media/video-controls-captions.html
Source/WebCore/ChangeLog
Source/WebCore/html/HTMLMediaElement.cpp
Source/WebCore/html/shadow/MediaControls.cpp
Source/WebCore/html/shadow/MediaControls.h

index a9d82d4..ce0bb1c 100644 (file)
@@ -1,3 +1,15 @@
+2013-01-31  Victor Carbune  <vcarbune@chromium.org>
+
+        [Track] Closed Caption button shouldn't be visible if all the track resources have failed loading
+        https://bugs.webkit.org/show_bug.cgi?id=106285
+
+        Reviewed by Eric Carlson.
+
+        Updated tests to include improved behavior.
+
+        * media/video-controls-captions-expected.txt: Updated.
+        * media/video-controls-captions.html: Updated.
+
 2013-01-31  Dima Gorbik  <dgorbik@apple.com>
 
         REGRESSION(r140231): media track layout tests crashing
index d0c0970..1490edb 100644 (file)
@@ -28,6 +28,23 @@ No text track cue with display id '-webkit-media-text-track-display' is currentl
 EXPECTED (captionsButtonCoordinates[0] <= '0') OK
 EXPECTED (captionsButtonCoordinates[1] <= '0') OK
 
+** Add non-default text track through HTML with unloadable URI **
+EXPECTED (track.readyState == '0') OK
+EXPECTED (track.track.mode == 'disabled') OK
+EXPECTED (video.textTracks.length == '1') OK
+
+** Caption button should be visible and enabled because we have a captions track.
+EXPECTED (captionsButtonCoordinates[0] > '0') OK
+EXPECTED (captionsButtonCoordinates[1] > '0') OK
+EXPECTED (captionsButtonElement.disabled == 'false') OK
+
+*** Click the CC button.
+** Track failed to load **
+
+** Caption button should not be visible as there are no caption tracks.
+EXPECTED (captionsButtonCoordinates[0] <= '0') OK
+EXPECTED (captionsButtonCoordinates[1] <= '0') OK
+
 ** Add a text track through JS to the video element **
 
 ** Caption button should be visible and enabled because we have a captions track.
index ee792c2..8f827ca 100644 (file)
@@ -9,6 +9,7 @@
     <script>
         var captionsButtonElement;
         var captionsButtonCoordinates;
+        var track;
 
         function addTextTrackThroughJS()
         {
             t.addCue(new TextTrackCue(0.0, 10.0, 'Some random caption text'));
         }
 
+        function addUnloadableHTMLTrackElement()
+        {
+            consoleWrite("");
+            consoleWrite("** Add non-default text track through HTML with unloadable URI **");
+
+            track = document.createElement("track");
+            track.setAttribute("kind", "captions");
+            track.setAttribute("srclang", "en");
+            track.setAttribute("src", "invalid.vtt");
+
+            track.addEventListener("error", trackError);
+
+            video.appendChild(track);
+            testExpected("track.readyState", HTMLTrackElement.NONE);
+            testExpected("track.track.mode", "disabled");
+            testExpected("video.textTracks.length", 1);
+        }
+
         function removeHTMLTrackElement()
         {
             consoleWrite("");
             removeHTMLTrackElement();
             testClosedCaptionsButtonVisibility(false);
 
+            addUnloadableHTMLTrackElement();
+            testClosedCaptionsButtonVisibility(true);
+
+            consoleWrite("");
+            clickCCButton();
+        }
+
+        function trackError()
+        {
+            consoleWrite("** Track failed to load **");
+            testClosedCaptionsButtonVisibility(false);
+
             addTextTrackThroughJS();
             testClosedCaptionsButtonVisibility(true);
 
             endTest();
         }
 
+
         function loaded()
         {
             findMediaElement();
index b8ddc48..59c1c6e 100644 (file)
@@ -1,3 +1,44 @@
+2013-01-31  Victor Carbune  <vcarbune@chromium.org>
+
+        [Track] Closed Caption button shouldn't be visible if all the track resources have failed loading
+        https://bugs.webkit.org/show_bug.cgi?id=106285
+
+        Reviewed by Eric Carlson.
+
+        Updated existing test cases.
+
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::textTracksAreReady): Checked whether there are tracks not loaded
+        when the algorithm began.
+        (WebCore::HTMLMediaElement::textTrackReadyStateChanged): If the ready state changed to
+        FailedToLoad the media controls should check whether there are other caption tracks
+        and hide the button if not.
+        (WebCore::HTMLMediaElement::didAddTrack): Added trigger to closedCaptionsTrackChanged.
+        (WebCore::HTMLMediaElement::hasClosedCaptions): Updated check to skip tracks that
+        failed to load.
+        * html/shadow/MediaControls.cpp:
+        (WebCore::MediaControls::reset): Used the newly added method.
+        (WebCore::MediaControls::refreshClosedCaptionsButtonVisibility): Added container method for
+        default behaviour for refreshing the visibility of the caption button.
+        (WebCore::MediaControls::closedCaptionTracksChanged): Used the newly added method.
+        (WebCore):
+        * html/shadow/MediaControls.h:
+        (MediaControls):
+
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::textTracksAreReady):
+        (WebCore::HTMLMediaElement::textTrackReadyStateChanged):
+        (WebCore::HTMLMediaElement::setReadyState):
+        (WebCore::HTMLMediaElement::didAddTrack):
+        (WebCore::HTMLMediaElement::hasClosedCaptions):
+        * html/shadow/MediaControls.cpp:
+        (WebCore::MediaControls::reset):
+        (WebCore::MediaControls::refreshClosedCaptionsButtonVisibility):
+        (WebCore::MediaControls::closedCaptionTracksChanged):
+        (WebCore):
+        * html/shadow/MediaControls.h:
+        (MediaControls):
+
 2013-01-31  Dima Gorbik  <dgorbik@apple.com>
 
         REGRESSION(r140231): media track layout tests crashing
index 5db2675..ceaf264 100644 (file)
@@ -1331,7 +1331,8 @@ bool HTMLMediaElement::textTracksAreReady() const
     // in the disabled state when the element's resource selection algorithm last started now
     // have a text track readiness state of loaded or failed to load.
     for (unsigned i = 0; i < m_textTracksWhenResourceSelectionBegan.size(); ++i) {
-        if (m_textTracksWhenResourceSelectionBegan[i]->readinessState() == TextTrack::Loading)
+        if (m_textTracksWhenResourceSelectionBegan[i]->readinessState() == TextTrack::Loading
+            || m_textTracksWhenResourceSelectionBegan[i]->readinessState() == TextTrack::NotLoaded)
             return false;
     }
 
@@ -1343,6 +1344,12 @@ void HTMLMediaElement::textTrackReadyStateChanged(TextTrack* track)
     if (m_player && m_textTracksWhenResourceSelectionBegan.contains(track)) {
         if (track->readinessState() != TextTrack::Loading)
             setReadyState(m_player->readyState());
+    } else {
+        // The track readiness state might have changed as a result of the user
+        // clicking the captions button. In this case, a check whether all the
+        // resources have failed loading should be done in order to hide the CC button.
+        if (hasMediaControls() && track->readinessState() == TextTrack::FailedToLoad)
+            mediaControls()->refreshClosedCaptionsButtonVisibility();
     }
 }
 
@@ -1841,8 +1848,10 @@ void HTMLMediaElement::setReadyState(MediaPlayer::ReadyState state)
 
     if (shouldUpdateDisplayState) {
         updateDisplayState();
-        if (hasMediaControls())
+        if (hasMediaControls()) {
+            mediaControls()->refreshClosedCaptionsButtonVisibility();
             mediaControls()->updateStatusDisplay();
+        }
     }
 
     updatePlayState();
@@ -2901,6 +2910,9 @@ void HTMLMediaElement::didAddTrack(HTMLTrackElement* trackElement)
     // in the markup have been added.
     if (!m_parsingInProgress)
         scheduleLoad(TextTrackResource);
+
+    if (hasMediaControls())
+        mediaControls()->closedCaptionTracksChanged();
 }
 
 void HTMLMediaElement::didRemoveTrack(HTMLTrackElement* trackElement)
@@ -4126,6 +4138,9 @@ bool HTMLMediaElement::hasClosedCaptions() const
 #if ENABLE(VIDEO_TRACK)
     if (RuntimeEnabledFeatures::webkitVideoTrackEnabled() && m_textTracks)
     for (unsigned i = 0; i < m_textTracks->length(); ++i) {
+        if (m_textTracks->item(i)->readinessState() == TextTrack::FailedToLoad)
+            continue;
+
         if (m_textTracks->item(i)->kind() == TextTrack::captionsKeyword()
             || m_textTracks->item(i)->kind() == TextTrack::subtitlesKeyword())
             return true;
index 949be8d..d99911c 100644 (file)
@@ -109,12 +109,7 @@ void MediaControls::reset()
         }
     }
 
-    if (m_toggleClosedCaptionsButton) {
-        if (m_mediaController->hasClosedCaptions())
-            m_toggleClosedCaptionsButton->show();
-        else
-            m_toggleClosedCaptionsButton->hide();
-    }
+    refreshClosedCaptionsButtonVisibility();
 
     if (m_fullScreenButton) {
         if (m_mediaController->supportsFullscreen() && m_mediaController->hasVideo())
@@ -252,7 +247,7 @@ void MediaControls::changedClosedCaptionsVisibility()
         m_toggleClosedCaptionsButton->updateDisplayType();
 }
 
-void MediaControls::closedCaptionTracksChanged()
+void MediaControls::refreshClosedCaptionsButtonVisibility()
 {
     if (!m_toggleClosedCaptionsButton)
         return;
@@ -263,6 +258,11 @@ void MediaControls::closedCaptionTracksChanged()
         m_toggleClosedCaptionsButton->hide();
 }
 
+void MediaControls::closedCaptionTracksChanged()
+{
+    refreshClosedCaptionsButtonVisibility();
+}
+
 void MediaControls::enteredFullscreen()
 {
     m_isFullscreen = true;
index dafd58a..25c074b 100644 (file)
@@ -88,6 +88,7 @@ class MediaControls : public HTMLDivElement {
     virtual void changedVolume();
 
     virtual void changedClosedCaptionsVisibility();
+    virtual void refreshClosedCaptionsButtonVisibility();
     virtual void toggleClosedCaptionTrackList() { }
     virtual void closedCaptionTracksChanged();