Commit 096153b9 authored by Becca Hughes's avatar Becca Hughes Committed by Commit Bot

Media Controls: Update preload=none state

Update preload=none state based on the new mocks.

Also adds ShouldShow[Audio/Video]Controls to MediaControlsImpl to
simplify the logic around which set of controls we should show.

BUG=818659

No-Try: true

Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ide3bed15161aa5e1e1096275212c718eacb24a77
Reviewed-on: https://chromium-review.googlesource.com/993757
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Reviewed-by: default avatarDominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: default avatarTommy Steimel <steimel@chromium.org>
Reviewed-by: default avatarMounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550487}
parent 9f80e74b
...@@ -244,7 +244,8 @@ TEST_F('OutputE2ETest', 'Audio', function() { ...@@ -244,7 +244,8 @@ TEST_F('OutputE2ETest', 'Audio', function() {
{value: new Output.NodeSpan(el.parent), start: 24, end: 35}], {value: new Output.NodeSpan(el.parent), start: 24, end: 35}],
o); o);
el = el.nextSibling.nextSibling; // TODO(dmazzoni/dtseng): Replace with a query.
el = el.nextSibling.nextSibling.nextSibling;
var prevRange = range; var prevRange = range;
range = cursors.Range.fromNode(el); range = cursors.Range.fromNode(el);
var o = new Output() var o = new Output()
......
<!DOCTYPE html>
<html>
<title>Test that the overflow menu is shown but disabled with no source.</title>
<script src="../../../resources/testharness.js"></script>
<script src="../../../resources/testharnessreport.js"></script>
<script src="../../media-controls.js"></script>
<video controls width=400 controlsList=nodownload></video>
<script>
async_test(t => {
const video = document.querySelector('video');
// Make sure the button is visible and disabled.
assert_true(overflowButton(video).disabled);
assert_equals('', overflowButton(video).style.display);
// Set the source and start playing.
video.src = '../../content/60_sec_video.webm';
video.play().then(t.step_func_done(() => {
// Make sure the button has been hidden and is no longer disabled.
assert_false(overflowButton(video).disabled);
assert_equals('none', overflowButton(video).style.display);
}), t.unreached_func());
});
</script>
</html>
...@@ -42,7 +42,9 @@ void MediaControlOverflowMenuButtonElement::UpdateShownState() { ...@@ -42,7 +42,9 @@ void MediaControlOverflowMenuButtonElement::UpdateShownState() {
} }
void MediaControlOverflowMenuButtonElement::DefaultEventHandler(Event* event) { void MediaControlOverflowMenuButtonElement::DefaultEventHandler(Event* event) {
if (event->type() == EventTypeNames::click) { // Only respond to a click event if we are not disabled.
if (!hasAttribute(HTMLNames::disabledAttr) &&
event->type() == EventTypeNames::click) {
if (GetMediaControls().OverflowMenuVisible()) { if (GetMediaControls().OverflowMenuVisible()) {
Platform::Current()->RecordAction( Platform::Current()->RecordAction(
UserMetricsAction("Media.Controls.OverflowClose")); UserMetricsAction("Media.Controls.OverflowClose"));
......
...@@ -496,7 +496,7 @@ MediaControlsImpl* MediaControlsImpl::Create(HTMLMediaElement& media_element, ...@@ -496,7 +496,7 @@ MediaControlsImpl* MediaControlsImpl::Create(HTMLMediaElement& media_element,
// +-MediaControlTextTrackListItemSubtitles // +-MediaControlTextTrackListItemSubtitles
// (-internal-media-controls-text-track-list-kind-subtitles) // (-internal-media-controls-text-track-list-kind-subtitles)
void MediaControlsImpl::InitializeControls() { void MediaControlsImpl::InitializeControls() {
if (IsModern() && MediaElement().IsHTMLVideoElement()) { if (IsModern() && ShouldShowVideoControls()) {
loading_panel_ = new MediaControlLoadingPanelElement(*this); loading_panel_ = new MediaControlLoadingPanelElement(*this);
ParserAppendChild(loading_panel_); ParserAppendChild(loading_panel_);
} }
...@@ -524,7 +524,7 @@ void MediaControlsImpl::InitializeControls() { ...@@ -524,7 +524,7 @@ void MediaControlsImpl::InitializeControls() {
// If using the modern media controls, the buttons should belong to a // If using the modern media controls, the buttons should belong to a
// seperate button panel. This is because they are displayed in two lines. // seperate button panel. This is because they are displayed in two lines.
if (IsModern() && MediaElement().IsHTMLVideoElement()) { if (IsModern() && ShouldShowVideoControls()) {
media_button_panel_ = new MediaControlButtonPanelElement(*this); media_button_panel_ = new MediaControlButtonPanelElement(*this);
scrubbing_message_ = new MediaControlScrubbingMessageElement(*this); scrubbing_message_ = new MediaControlScrubbingMessageElement(*this);
} }
...@@ -604,8 +604,7 @@ void MediaControlsImpl::PopulatePanel() { ...@@ -604,8 +604,7 @@ void MediaControlsImpl::PopulatePanel() {
media_button_panel_->setInnerHTML(StringOrTrustedHTML::FromString("")); media_button_panel_->setInnerHTML(StringOrTrustedHTML::FromString(""));
Element* button_panel = panel_; Element* button_panel = panel_;
if (IsModern() && MediaElement().IsHTMLVideoElement() && if (IsModern() && ShouldShowVideoControls()) {
!is_acting_as_audio_controls_) {
MaybeParserAppendChild(panel_, scrubbing_message_); MaybeParserAppendChild(panel_, scrubbing_message_);
panel_->ParserAppendChild(overlay_play_button_); panel_->ParserAppendChild(overlay_play_button_);
panel_->ParserAppendChild(media_button_panel_); panel_->ParserAppendChild(media_button_panel_);
...@@ -616,7 +615,7 @@ void MediaControlsImpl::PopulatePanel() { ...@@ -616,7 +615,7 @@ void MediaControlsImpl::PopulatePanel() {
button_panel->ParserAppendChild(current_time_display_); button_panel->ParserAppendChild(current_time_display_);
button_panel->ParserAppendChild(duration_display_); button_panel->ParserAppendChild(duration_display_);
if (IsModern() && MediaElement().IsHTMLVideoElement()) { if (IsModern() && ShouldShowVideoControls()) {
MediaControlElementsHelper::CreateDiv( MediaControlElementsHelper::CreateDiv(
"-internal-media-controls-button-spacer", button_panel); "-internal-media-controls-button-spacer", button_panel);
} }
...@@ -676,8 +675,7 @@ void MediaControlsImpl::UpdateCSSClassFromState() { ...@@ -676,8 +675,7 @@ void MediaControlsImpl::UpdateCSSClassFromState() {
StringBuilder builder; StringBuilder builder;
builder.Append(kStateCSSClasses[state]); builder.Append(kStateCSSClasses[state]);
if (MediaElement().ShouldShowControls() && if (MediaElement().ShouldShowControls() && ShouldShowVideoControls() &&
MediaElement().IsHTMLVideoElement() && !is_acting_as_audio_controls_ &&
!VideoElement().HasAvailableVideoFrame() && !VideoElement().HasAvailableVideoFrame() &&
VideoElement().PosterImageURL().IsEmpty() && VideoElement().PosterImageURL().IsEmpty() &&
state <= ControlsState::kLoadingMetadata) { state <= ControlsState::kLoadingMetadata) {
...@@ -701,6 +699,23 @@ void MediaControlsImpl::UpdateCSSClassFromState() { ...@@ -701,6 +699,23 @@ void MediaControlsImpl::UpdateCSSClassFromState() {
if (loading_panel_) if (loading_panel_)
loading_panel_->UpdateDisplayState(); loading_panel_->UpdateDisplayState();
// If we are in the "no-source" state we should show the overflow menu on a
// video element.
if (IsModern()) {
if (state == kNoSource) {
// Check if the overflow menu has the "disabled" attribute set so we avoid
// unnecessarily resetting it.
if (ShouldShowVideoControls() &&
!overflow_menu_->hasAttribute(HTMLNames::disabledAttr)) {
overflow_menu_->setAttribute(HTMLNames::disabledAttr, "");
UpdateOverflowMenuWanted();
}
} else if (overflow_menu_->hasAttribute(HTMLNames::disabledAttr)) {
overflow_menu_->removeAttribute(HTMLNames::disabledAttr);
UpdateOverflowMenuWanted();
}
}
} }
MediaControlsImpl::ControlsState MediaControlsImpl::State() const { MediaControlsImpl::ControlsState MediaControlsImpl::State() const {
...@@ -793,7 +808,7 @@ void MediaControlsImpl::Reset() { ...@@ -793,7 +808,7 @@ void MediaControlsImpl::Reset() {
void MediaControlsImpl::OnControlsListUpdated() { void MediaControlsImpl::OnControlsListUpdated() {
BatchedControlUpdate batch(this); BatchedControlUpdate batch(this);
if (ShouldShowDisabledControls()) { if (IsModern() && ShouldShowVideoControls()) {
fullscreen_button_->SetIsWanted(true); fullscreen_button_->SetIsWanted(true);
fullscreen_button_->setAttribute(HTMLNames::disabledAttr, fullscreen_button_->setAttribute(HTMLNames::disabledAttr,
ShouldShowFullscreenButton(MediaElement()) ShouldShowFullscreenButton(MediaElement())
...@@ -1150,7 +1165,7 @@ void MediaControlsImpl::UpdateOverflowMenuWanted() const { ...@@ -1150,7 +1165,7 @@ void MediaControlsImpl::UpdateOverflowMenuWanted() const {
// The video controls are more than one row so we need to allocate vertical // The video controls are more than one row so we need to allocate vertical
// room and hide the overlay play button if there is not enough room. // room and hide the overlay play button if there is not enough room.
if (MediaElement().IsHTMLVideoElement() && !is_acting_as_audio_controls_) { if (ShouldShowVideoControls()) {
// Allocate vertical room for overlay play button if necessary. // Allocate vertical room for overlay play button if necessary.
WebSize overlay_play_button_size = overlay_play_button_->GetSizeOrDefault(); WebSize overlay_play_button_size = overlay_play_button_->GetSizeOrDefault();
if (controls_size.height >= overlay_play_button_size.height && if (controls_size.height >= overlay_play_button_size.height &&
...@@ -1227,6 +1242,9 @@ void MediaControlsImpl::UpdateOverflowMenuWanted() const { ...@@ -1227,6 +1242,9 @@ void MediaControlsImpl::UpdateOverflowMenuWanted() const {
} }
} }
// The overflow menu is always wanted if it has the "disabled" attr set.
overflow_wanted =
overflow_wanted || overflow_menu_->hasAttribute(HTMLNames::disabledAttr);
overflow_menu_->SetDoesFit(overflow_wanted); overflow_menu_->SetDoesFit(overflow_wanted);
overflow_menu_->SetIsWanted(overflow_wanted); overflow_menu_->SetIsWanted(overflow_wanted);
...@@ -1450,7 +1468,7 @@ void MediaControlsImpl::OnVolumeChange() { ...@@ -1450,7 +1468,7 @@ void MediaControlsImpl::OnVolumeChange() {
volume_slider_->SetIsWanted(MediaElement().HasAudio() && volume_slider_->SetIsWanted(MediaElement().HasAudio() &&
!PreferHiddenVolumeControls(GetDocument())); !PreferHiddenVolumeControls(GetDocument()));
} }
if (ShouldShowDisabledControls()) { if (IsModern()) {
mute_button_->SetIsWanted(true); mute_button_->SetIsWanted(true);
mute_button_->setAttribute( mute_button_->setAttribute(
HTMLNames::disabledAttr, HTMLNames::disabledAttr,
...@@ -1495,7 +1513,13 @@ void MediaControlsImpl::OnDurationChange() { ...@@ -1495,7 +1513,13 @@ void MediaControlsImpl::OnDurationChange() {
// Update the displayed current time/duration. // Update the displayed current time/duration.
duration_display_->SetCurrentValue(duration); duration_display_->SetCurrentValue(duration);
duration_display_->SetIsWanted(std::isfinite(duration));
// Show the duration display if we have a duration or if we are showing the
// audio controls without a source.
duration_display_->SetIsWanted(
std::isfinite(duration) ||
(ShouldShowAudioControls() && State() == kNoSource));
// TODO(crbug.com/756698): Determine if this is still needed since the format // TODO(crbug.com/756698): Determine if this is still needed since the format
// of the current time no longer depends on the duration. // of the current time no longer depends on the duration.
UpdateCurrentTimeDisplay(); UpdateCurrentTimeDisplay();
...@@ -1808,9 +1832,13 @@ void MediaControlsImpl::StopActingAsAudioControls() { ...@@ -1808,9 +1832,13 @@ void MediaControlsImpl::StopActingAsAudioControls() {
Reset(); Reset();
} }
bool MediaControlsImpl::ShouldShowDisabledControls() const { bool MediaControlsImpl::ShouldShowAudioControls() const {
return IsModern() && MediaElement().IsHTMLVideoElement() && return IsModern() &&
!is_acting_as_audio_controls_; (MediaElement().IsHTMLAudioElement() || is_acting_as_audio_controls_);
}
bool MediaControlsImpl::ShouldShowVideoControls() const {
return MediaElement().IsHTMLVideoElement() && !ShouldShowAudioControls();
} }
void MediaControlsImpl::NetworkStateChanged() { void MediaControlsImpl::NetworkStateChanged() {
......
...@@ -278,7 +278,9 @@ class MODULES_EXPORT MediaControlsImpl final : public HTMLDivElement, ...@@ -278,7 +278,9 @@ class MODULES_EXPORT MediaControlsImpl final : public HTMLDivElement,
void StartActingAsAudioControls(); void StartActingAsAudioControls();
void StopActingAsAudioControls(); void StopActingAsAudioControls();
bool ShouldShowDisabledControls() const; // Returns true/false based on which set of controls to display.
bool ShouldShowAudioControls() const;
bool ShouldShowVideoControls() const;
// Node // Node
bool IsMediaControls() const override { return true; } bool IsMediaControls() const override { return true; }
......
<svg xmlns="http://www.w3.org/2000/svg" width="24px" height="24px" viewBox="0 0 24 24" fill="#000000">
<path fill="none" d="M0 0h24v24H0zm0 0h24v24H0zm21 19c0 1.1-.9 2-2 2H5c-1.1 0-2-.9-2-2V5c0-1.1.9-2 2-2h14c1.1 0 2 .9 2 2"/>
<path fill="none" d="M0 0h24v24H0z"/>
<path d="M21 5v6.59l-3-3.01-4 4.01-4-4-4 4-3-3.01V5c0-1.1.9-2 2-2h14c1.1 0 2 .9 2 2zm-3 6.42l3 3.01V19c0 1.1-.9 2-2 2H5c-1.1 0-2-.9-2-2v-6.58l3 2.99 4-4 4 4 4-3.99z"/>
</svg>
...@@ -232,6 +232,8 @@ video::-webkit-media-controls:not(.audio-only) [pseudo="-webkit-media-controls-p ...@@ -232,6 +232,8 @@ video::-webkit-media-controls:not(.audio-only) [pseudo="-webkit-media-controls-p
background-image: -webkit-image-set(url(ic_fullscreen_exit_white.svg) 1x); background-image: -webkit-image-set(url(ic_fullscreen_exit_white.svg) 1x);
} }
audio::-webkit-media-controls-mute-button:disabled,
video::-internal-media-controls-overflow-button:disabled,
video::-webkit-media-controls-mute-button:disabled, video::-webkit-media-controls-mute-button:disabled,
video::-webkit-media-controls-fullscreen-button:disabled { video::-webkit-media-controls-fullscreen-button:disabled {
opacity: 0.3; opacity: 0.3;
...@@ -365,6 +367,7 @@ input[pseudo="-webkit-media-controls-timeline" i]::-webkit-slider-thumb { ...@@ -365,6 +367,7 @@ input[pseudo="-webkit-media-controls-timeline" i]::-webkit-slider-thumb {
margin-top: -4px; margin-top: -4px;
flex: 0 0 0; flex: 0 0 0;
} }
video::-webkit-media-controls:not(.audio-only) input[pseudo="-webkit-media-controls-timeline" i]::-webkit-slider-thumb { video::-webkit-media-controls:not(.audio-only) input[pseudo="-webkit-media-controls-timeline" i]::-webkit-slider-thumb {
background: #FFFFFF; background: #FFFFFF;
box-shadow: unset; box-shadow: unset;
...@@ -639,18 +642,12 @@ video::-webkit-media-controls.audio-only [pseudo="-internal-media-controls-overf ...@@ -639,18 +642,12 @@ video::-webkit-media-controls.audio-only [pseudo="-internal-media-controls-overf
*/ */
.use-default-poster { .use-default-poster {
background: #F1F3F4; background: #333;
}
.state-no-metadata input[pseudo="-webkit-media-controls-timeline" i]::-webkit-slider-thumb,
.state-no-metadata div[pseudo="-webkit-media-controls-current-time-display" i] {
display: none;
} }
.state-no-source input[pseudo="-webkit-media-controls-overlay-play-button" i], .state-no-source input[pseudo="-webkit-media-controls-overlay-play-button" i]::-internal-media-controls-overlay-play-button-internal {
.state-no-source div[pseudo="-internal-media-controls-button-panel" i], opacity: .3;
video::-webkit-media-controls.state-no-source input[pseudo="-webkit-media-controls-timeline" i] { background-image: -webkit-image-set(url(ic_no_source.svg) 1x);
visibility: hidden;
} }
/** /**
......
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