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: Dominic Mazzoni <dmazzoni@chromium.org> Reviewed-by: Tommy Steimel <steimel@chromium.org> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#550487}(cherry picked from commit 096153b) Reviewed-on: https://chromium-review.googlesource.com/1014181 Reviewed-by: Becca Hughes <beccahughes@chromium.org> Cr-Commit-Position: refs/branch-heads/3396@{#15} Cr-Branched-From: 9ef2aa8-refs/heads/master@{#550428}
nwjs · Apr 16, 2018 · 883aaf5 · 883aaf5
1 parent 7b70b32
commit 883aaf5
Show file tree

Hide file tree

Showing 7 changed files with 87 additions and 27 deletions.
diff --git a/chrome/browser/resources/chromeos/chromevox/cvox2/background/output_test.extjs b/chrome/browser/resources/chromeos/chromevox/cvox2/background/output_test.extjs
@@ -244,7 +244,8 @@ TEST_F('OutputE2ETest', 'Audio', function() {
            {value: new Output.NodeSpan(el.parent), start: 24, end: 35}],
           o);
 
-      el = el.nextSibling.nextSibling;
+      // TODO(dmazzoni/dtseng): Replace with a query.
+      el = el.nextSibling.nextSibling.nextSibling;
       var prevRange = range;
       range = cursors.Range.fromNode(el);
       var o = new Output()

diff --git a/third_party/WebKit/LayoutTests/media/controls/modern/overflow-button-disabled-no-source.html b/third_party/WebKit/LayoutTests/media/controls/modern/overflow-button-disabled-no-source.html
@@ -0,0 +1,25 @@
+<!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>
diff --git a/...nk/renderer/modules/media_controls/elements/media_control_overflow_menu_button_element.cc b/...nk/renderer/modules/media_controls/elements/media_control_overflow_menu_button_element.cc
@@ -42,7 +42,9 @@ void MediaControlOverflowMenuButtonElement::UpdateShownState() {
 }
 
 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()) {
       Platform::Current()->RecordAction(
           UserMetricsAction("Media.Controls.OverflowClose"));

diff --git a/third_party/blink/renderer/modules/media_controls/media_controls_impl.cc b/third_party/blink/renderer/modules/media_controls/media_controls_impl.cc
@@ -496,7 +496,7 @@ MediaControlsImpl* MediaControlsImpl::Create(HTMLMediaElement& media_element,
 //  +-MediaControlTextTrackListItemSubtitles
 //       (-internal-media-controls-text-track-list-kind-subtitles)
 void MediaControlsImpl::InitializeControls() {
-  if (IsModern() && MediaElement().IsHTMLVideoElement()) {
+  if (IsModern() && ShouldShowVideoControls()) {
     loading_panel_ = new MediaControlLoadingPanelElement(*this);
     ParserAppendChild(loading_panel_);
   }
@@ -524,7 +524,7 @@ void MediaControlsImpl::InitializeControls() {
 
   // If using the modern media controls, the buttons should belong to a
   // 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);
     scrubbing_message_ = new MediaControlScrubbingMessageElement(*this);
   }
@@ -604,8 +604,7 @@ void MediaControlsImpl::PopulatePanel() {
     media_button_panel_->setInnerHTML(StringOrTrustedHTML::FromString(""));
 
   Element* button_panel = panel_;
-  if (IsModern() && MediaElement().IsHTMLVideoElement() &&
-      !is_acting_as_audio_controls_) {
+  if (IsModern() && ShouldShowVideoControls()) {
     MaybeParserAppendChild(panel_, scrubbing_message_);
     panel_->ParserAppendChild(overlay_play_button_);
     panel_->ParserAppendChild(media_button_panel_);
@@ -616,7 +615,7 @@ void MediaControlsImpl::PopulatePanel() {
   button_panel->ParserAppendChild(current_time_display_);
   button_panel->ParserAppendChild(duration_display_);
 
-  if (IsModern() && MediaElement().IsHTMLVideoElement()) {
+  if (IsModern() && ShouldShowVideoControls()) {
     MediaControlElementsHelper::CreateDiv(
         "-internal-media-controls-button-spacer", button_panel);
   }
@@ -676,8 +675,7 @@ void MediaControlsImpl::UpdateCSSClassFromState() {
   StringBuilder builder;
   builder.Append(kStateCSSClasses[state]);
 
-  if (MediaElement().ShouldShowControls() &&
-      MediaElement().IsHTMLVideoElement() && !is_acting_as_audio_controls_ &&
+  if (MediaElement().ShouldShowControls() && ShouldShowVideoControls() &&
       !VideoElement().HasAvailableVideoFrame() &&
       VideoElement().PosterImageURL().IsEmpty() &&
       state <= ControlsState::kLoadingMetadata) {
@@ -701,6 +699,23 @@ void MediaControlsImpl::UpdateCSSClassFromState() {
 
   if (loading_panel_)
     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 {
@@ -793,7 +808,7 @@ void MediaControlsImpl::Reset() {
 void MediaControlsImpl::OnControlsListUpdated() {
   BatchedControlUpdate batch(this);
 
-  if (ShouldShowDisabledControls()) {
+  if (IsModern() && ShouldShowVideoControls()) {
     fullscreen_button_->SetIsWanted(true);
     fullscreen_button_->setAttribute(HTMLNames::disabledAttr,
                                      ShouldShowFullscreenButton(MediaElement())
@@ -1150,7 +1165,7 @@ void MediaControlsImpl::UpdateOverflowMenuWanted() const {
 
   // 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.
-  if (MediaElement().IsHTMLVideoElement() && !is_acting_as_audio_controls_) {
+  if (ShouldShowVideoControls()) {
     // Allocate vertical room for overlay play button if necessary.
     WebSize overlay_play_button_size = overlay_play_button_->GetSizeOrDefault();
     if (controls_size.height >= overlay_play_button_size.height &&
@@ -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_->SetIsWanted(overflow_wanted);
 
@@ -1450,7 +1468,7 @@ void MediaControlsImpl::OnVolumeChange() {
     volume_slider_->SetIsWanted(MediaElement().HasAudio() &&
                                 !PreferHiddenVolumeControls(GetDocument()));
   }
-  if (ShouldShowDisabledControls()) {
+  if (IsModern()) {
     mute_button_->SetIsWanted(true);
     mute_button_->setAttribute(
         HTMLNames::disabledAttr,
@@ -1495,7 +1513,13 @@ void MediaControlsImpl::OnDurationChange() {
 
   // Update the displayed current time/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
   // of the current time no longer depends on the duration.
   UpdateCurrentTimeDisplay();
@@ -1808,9 +1832,13 @@ void MediaControlsImpl::StopActingAsAudioControls() {
   Reset();
 }
 
-bool MediaControlsImpl::ShouldShowDisabledControls() const {
-  return IsModern() && MediaElement().IsHTMLVideoElement() &&
-         !is_acting_as_audio_controls_;
+bool MediaControlsImpl::ShouldShowAudioControls() const {
+  return IsModern() &&
+         (MediaElement().IsHTMLAudioElement() || is_acting_as_audio_controls_);
+}
+
+bool MediaControlsImpl::ShouldShowVideoControls() const {
+  return MediaElement().IsHTMLVideoElement() && !ShouldShowAudioControls();
 }
 
 void MediaControlsImpl::NetworkStateChanged() {

diff --git a/third_party/blink/renderer/modules/media_controls/media_controls_impl.h b/third_party/blink/renderer/modules/media_controls/media_controls_impl.h
@@ -278,7 +278,9 @@ class MODULES_EXPORT MediaControlsImpl final : public HTMLDivElement,
   void StartActingAsAudioControls();
   void StopActingAsAudioControls();
 
-  bool ShouldShowDisabledControls() const;
+  // Returns true/false based on which set of controls to display.
+  bool ShouldShowAudioControls() const;
+  bool ShouldShowVideoControls() const;
 
   // Node
   bool IsMediaControls() const override { return true; }

diff --git a/third_party/blink/renderer/modules/media_controls/resources/ic_no_source.svg b/third_party/blink/renderer/modules/media_controls/resources/ic_no_source.svg
diff --git a/third_party/blink/renderer/modules/media_controls/resources/modernMediaControls.css b/third_party/blink/renderer/modules/media_controls/resources/modernMediaControls.css
@@ -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);
 }
 
+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-fullscreen-button:disabled {
   opacity: 0.3;
@@ -365,6 +367,7 @@ input[pseudo="-webkit-media-controls-timeline" i]::-webkit-slider-thumb {
   margin-top: -4px;
   flex: 0 0 0;
 }
+
 video::-webkit-media-controls:not(.audio-only) input[pseudo="-webkit-media-controls-timeline" i]::-webkit-slider-thumb {
   background: #FFFFFF;
   box-shadow: unset;
@@ -639,18 +642,12 @@ video::-webkit-media-controls.audio-only [pseudo="-internal-media-controls-overf
  */
 
 .use-default-poster {
-  background: #F1F3F4;
-}
-
-.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;
+  background: #333;
 }
 
-.state-no-source input[pseudo="-webkit-media-controls-overlay-play-button" i],
-.state-no-source div[pseudo="-internal-media-controls-button-panel" i],
-video::-webkit-media-controls.state-no-source input[pseudo="-webkit-media-controls-timeline" i] {
-  visibility: hidden;
+.state-no-source input[pseudo="-webkit-media-controls-overlay-play-button" i]::-internal-media-controls-overlay-play-button-internal {
+  opacity: .3;
+  background-image: -webkit-image-set(url(ic_no_source.svg) 1x);
 }
 
 /**