Commit 6ddcabed authored by eric.carlson@apple.com's avatar eric.carlson@apple.com
Browse files

Adding a text track should not make controls visible

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

Source/WebCore: 

Reviewed by Dean Jackson.

Test: media/media-captions-no-controls.html

* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::updateActiveTextTrackCues): Call updateTextTrackDisplay.
(WebCore::HTMLMediaElement::updateTextTrackDisplay): New, call mediaControls->updateTextTrackDisplay()
    if we have controls.
(WebCore::HTMLMediaElement::setClosedCaptionsVisible):  Call updateTextTrackDisplay.
(WebCore::HTMLMediaElement::createMediaControls): Hide controls if they should not be 
    visible. Minor drive by cleanup.
(WebCore::HTMLMediaElement::configureTextTrackDisplay): Drive by cleanup, pull the three lines
    from updateClosedCaptionsControls inline and delete it because this was the only caller.
* html/HTMLMediaElement.h:

LayoutTests: 

Reviewed by  Dean Jackson.

* media/media-captions-no-controls-expected.txt: Added.
* media/media-captions-no-controls.html: Added.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@140862 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 8bc257bd
2013-01-25 Eric Carlson <eric.carlson@apple.com>
Adding a text track should not make controls visible
https://bugs.webkit.org/show_bug.cgi?id=107956
Reviewed by Dean Jackson.
* media/media-captions-no-controls-expected.txt: Added.
* media/media-captions-no-controls.html: Added.
2013-01-25 Erik Arvidsson <arv@chromium.org>
 
Unreviewed Chromium rebaseline.
Tests that adding a text track does not make controls visible.
EVENT(canplaythrough)
** Initial state: no text tracks, controls should not be visible **
EXPECTED (video.textTracks.length == '0') OK
EXPECTED (video.controls == 'false') OK
EXPECTED (internals.shadowRoot(video) == 'null') OK
** Add a text track, controls should not become visible **
RUN(video.addTextTrack('captions'))
EXPECTED (video.textTracks.length == '1') OK
EXPECTED (video.controls == 'false') OK
EXPECTED (internals.shadowRoot(video) == 'null') OK
** Enable controls **
RUN(video.setAttribute('controls','controls'))
EXPECTED (video.textTracks.length == '1') OK
EXPECTED (video.controls == 'true') OK
EXPECTED (internals.shadowRoot(video) != 'null') OK
EXPECTED (controlsElement.style['display'] != 'none') OK
END OF TEST
<!DOCTYPE html>
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<script src=media-file.js></script>
<script src=video-test.js></script>
<script src=media-controls.js></script>
<script>
function canplaythrough()
{
consoleWrite("<br>** Initial state: no text tracks, controls should not be visible **");
testExpected("video.textTracks.length", 0);
testExpected("video.controls", false, '==');
testExpected("internals.shadowRoot(video)", null);
consoleWrite("<br>** Add a text track, controls should not become visible **");
run("video.addTextTrack('captions')");
testExpected("video.textTracks.length", 1);
testExpected("video.controls", false);
testExpected("internals.shadowRoot(video)", null);
consoleWrite("<br>** Enable controls **");
run("video.setAttribute('controls','controls')");
testExpected("video.textTracks.length", 1);
testExpected("video.controls", true);
controlsElement = mediaControlsElement(internals.shadowRoot(video).firstChild, "-webkit-media-controls-panel");
testExpected("internals.shadowRoot(video)", null, "!=");
testExpected("controlsElement.style['display']", 'none', "!=");
consoleWrite("");
endTest();
}
function start()
{
findMediaElement();
waitForEvent('canplaythrough', canplaythrough);
video.src = findMediaFile("video", "content/test");
}
</script>
</head>
<body onload="start()">
<video>
</video>
<p>Tests that adding a text track does not make controls visible.</p>
</body>
</html>
2013-01-25 Eric Carlson <eric.carlson@apple.com>
Adding a text track should not make controls visible
https://bugs.webkit.org/show_bug.cgi?id=107956
Reviewed by Dean Jackson.
Test: media/media-captions-no-controls.html
* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::updateActiveTextTrackCues): Call updateTextTrackDisplay.
(WebCore::HTMLMediaElement::updateTextTrackDisplay): New, call mediaControls->updateTextTrackDisplay()
if we have controls.
(WebCore::HTMLMediaElement::setClosedCaptionsVisible): Call updateTextTrackDisplay.
(WebCore::HTMLMediaElement::createMediaControls): Hide controls if they should not be
visible. Minor drive by cleanup.
(WebCore::HTMLMediaElement::configureTextTrackDisplay): Drive by cleanup, pull the three lines
from updateClosedCaptionsControls inline and delete it because this was the only caller.
* html/HTMLMediaElement.h:
2013-01-25 Elliott Sprehn <esprehn@chromium.org>
 
Consider all ancestors not just parentElement when disconnecting frames
......@@ -1178,9 +1178,10 @@ void HTMLMediaElement::updateActiveTextTrackCues(float movieTime)
// Even though the active set has not changed, it is possible that the
// the mode of a track has changed from 'hidden' to 'showing' and the
// cues have not yet been rendered.
// Note: don't call updateTextTrackDisplay() unless we have controls because it will
// create them.
if (hasMediaControls())
mediaControls()->updateTextTrackDisplay();
updateTextTrackDisplay();
return;
}
......@@ -1318,8 +1319,8 @@ void HTMLMediaElement::updateActiveTextTrackCues(float movieTime)
// Update the current active cues.
m_currentlyActiveCues = currentCues;
if (activeSetChanged && hasMediaControls())
mediaControls()->updateTextTrackDisplay();
if (activeSetChanged)
updateTextTrackDisplay();
}
bool HTMLMediaElement::textTracksAreReady() const
......@@ -4149,6 +4150,16 @@ bool HTMLMediaElement::closedCaptionsVisible() const
return m_closedCaptionsVisible;
}
#if ENABLE(VIDEO_TRACK)
void HTMLMediaElement::updateTextTrackDisplay()
{
if (!hasMediaControls() && !createMediaControls())
return;
mediaControls()->updateTextTrackDisplay();
}
#endif
void HTMLMediaElement::setClosedCaptionsVisible(bool closedCaptionVisible)
{
LOG(Media, "HTMLMediaElement::setClosedCaptionsVisible(%s)", boolString(closedCaptionVisible));
......@@ -4164,7 +4175,8 @@ void HTMLMediaElement::setClosedCaptionsVisible(bool closedCaptionVisible)
m_disableCaptions = !m_closedCaptionsVisible;
markCaptionAndSubtitleTracksAsUnconfigured();
mediaControls()->updateTextTrackDisplay();
updateTextTrackDisplay();
}
#else
if (hasMediaControls())
......@@ -4285,21 +4297,24 @@ bool HTMLMediaElement::createMediaControls()
if (hasMediaControls())
return true;
ExceptionCode ec;
RefPtr<MediaControls> controls = MediaControls::create(document());
if (!controls)
RefPtr<MediaControls> mediaControls = MediaControls::create(document());
if (!mediaControls)
return false;
controls->setMediaController(m_mediaController ? m_mediaController.get() : static_cast<MediaControllerInterface*>(this));
controls->reset();
mediaControls->setMediaController(m_mediaController ? m_mediaController.get() : static_cast<MediaControllerInterface*>(this));
mediaControls->reset();
if (isFullscreen())
controls->enteredFullscreen();
mediaControls->enteredFullscreen();
if (!shadow())
createShadowSubtree();
ASSERT(userAgentShadowRoot());
userAgentShadowRoot()->appendChild(controls, ec);
userAgentShadowRoot()->appendChild(mediaControls, ASSERT_NO_EXCEPTION);
if (!controls() || !inDocument())
mediaControls->hide();
return true;
}
......@@ -4348,17 +4363,10 @@ void HTMLMediaElement::configureTextTrackDisplay()
if (!hasMediaControls() && !createMediaControls())
return;
updateClosedCaptionsControls();
}
void HTMLMediaElement::updateClosedCaptionsControls()
{
if (hasMediaControls()) {
mediaControls()->changedClosedCaptionsVisibility();
if (RuntimeEnabledFeatures::webkitVideoTrackEnabled())
mediaControls()->updateTextTrackDisplay();
}
mediaControls()->changedClosedCaptionsVisibility();
if (RuntimeEnabledFeatures::webkitVideoTrackEnabled())
updateTextTrackDisplay();
}
void HTMLMediaElement::captionPreferencesChanged()
......
......@@ -258,7 +258,7 @@ public:
bool userIsInterestedInThisTrackKind(String) const;
bool textTracksAreReady() const;
void configureTextTrackDisplay();
void updateClosedCaptionsControls();
void updateTextTrackDisplay();
// TextTrackClient
virtual void textTrackReadyStateChanged(TextTrack*);
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment