Skip to content

GitLab

T
tangled
Commit 305d9733 authored by Jazz Xu's avatar Jazz Xu Committed by Commit Bot
Browse files
Options

[Media Controls] Show volume slider only if user hover mute button for a while (200ms) 

Bug: 904554
Change-Id: I3026b7c47df4f3950450b5fd721ae84bab00e382
Reviewed-on: https://chromium-review.googlesource.com/c/1332718Reviewed-by: default avatarTommy Steimel <steimel@chromium.org>
Commit-Queue: Jazz Xu <jazzhsu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607790}
parent 85ddfa5c
Hide whitespace changes
Inline Side-by-side
Showing with 68 additions and 6 deletions
third_party/WebKit/LayoutTests/media/controls/volume-slider-accidental-hover.html 0 → 100644
View file @ 305d9733
<!DOCTYPE html>
<title>Accidental hovering mute button should not show volume slider.</title>
<script src='../../resources/testharness.js'></script>
<script src='../../resources/testharnessreport.js'></script>
<script src='../media-controls.js'></script>
<video controls width="400"></video>
<script>
async_test(t => {
const video = document.querySelector('video');
video.src = '../content/test.ogv';
enableTestMode(video);
video.onloadedmetadata = t.step_func(() => {
const muteBtn = muteButton(video);
const currentTime = currentTimeElement(video);
assert_false(isVolumeSliderOpen(video), "volume slider should start closed");
hoverOverControl(muteBtn, t.step_func(() => {
assert_false(isVolumeSliderOpen(video), "volume slider should not open yet");
hoverOverControl(currentTime, t.step_func_done(() => {
assert_false(isVolumeSliderOpen(video), "volume slider should not open if its accidental pass through");
}));
}));
});
});
</script>
third_party/WebKit/LayoutTests/media/controls/volume-slider.html
View file @ 305d9733
......@@ -57,7 +57,7 @@ function testMediaElement(t, element) {
// Hovering over the mute button before preloading should not open the volume
// slider.
hoverOverControl(muteBtn, t.step_func(() => {
hoverMuteButtonAndWaitForVolumeSliderTimeout(element, t.step_func(() => {
assert_false(isVolumeSliderOpen(element), element.id + ': volume slider should not open before metadata load');
// Move the cursor away from the mute button and start testing the loaded
......@@ -72,7 +72,7 @@ function testMediaElement(t, element) {
// Hovering over the mute button after preloading should open the volume
// slider.
hoverOverControl(muteBtn, t.step_func(() => {
hoverMuteButtonAndWaitForVolumeSliderTimeout(element, t.step_func(() => {
assert_true(isVolumeSliderOpen(element), element.id + ': volume slider should open when hovering the mute button');
// Moving the cursor onto the volume slider should not hide the volume slider.
......
third_party/WebKit/LayoutTests/media/media-controls.js
View file @ 305d9733
......@@ -427,6 +427,17 @@ function runAfterDoubleTapTimerFired(func) {
setTimeout(func, doubleTapTimeoutMs);
}
function runAfterVolumeSliderShowed(func) {
// 200ms timer plus 100ms slack.
const volumeSliderTimeoutMs = 200 + 100;
setTimeout(func, volumeSliderTimeoutMs)
}
function hoverMuteButtonAndWaitForVolumeSliderTimeout(video, func) {
const muteBtn = muteButton(video);
hoverOverControl(muteBtn, runAfterVolumeSliderShowed(func));
}
function hasEnabledFullscreenButton(element) {
var button = fullscreenButton(element);
return !button.disabled && button.style.display != "none";
......
third_party/blink/renderer/modules/media_controls/media_controls_impl.cc
View file @ 305d9733
......@@ -136,6 +136,10 @@ const char kImmersiveModeCSSClass[] = "immersive-mode";
// The delay between two taps to be recognized as a double tap gesture.
constexpr WTF::TimeDelta kDoubleTapDelay = TimeDelta::FromMilliseconds(300);
// The time user have to hover on mute button to show volume slider.
constexpr WTF::TimeDelta kTimeToShowVolumeSlider =
TimeDelta::FromMilliseconds(200);
// The number of seconds to jump when double tapping.
constexpr int kNumberOfSecondsToJump = 10;
......@@ -395,7 +399,11 @@ MediaControlsImpl::MediaControlsImpl(HTMLMediaElement& media_element)
tap_timer_(
media_element.GetDocument().GetTaskRunner(TaskType::kInternalMedia),
this,
&MediaControlsImpl::TapTimerFired) {
&MediaControlsImpl::TapTimerFired),
volume_slider_wanted_timer_(
media_element.GetDocument().GetTaskRunner(TaskType::kInternalMedia),
this,
&MediaControlsImpl::VolumeSliderWantedTimerFired) {
// On touch devices, start with the assumption that the user will interact via
// touch events.
Settings* settings = media_element.GetDocument().GetSettings();
......@@ -2162,14 +2170,24 @@ void MediaControlsImpl::StartHideMediaControlsIfNecessary() {
StartHideMediaControlsTimer();
}
void MediaControlsImpl::VolumeSliderWantedTimerFired(TimerBase*) {
volume_slider_->OpenSlider();
}
void MediaControlsImpl::OpenVolumeSliderIfNecessary() {
if (ShouldOpenVolumeSlider())
volume_slider_->OpenSlider();
if (ShouldOpenVolumeSlider() && !volume_slider_wanted_timer_.IsActive()) {
volume_slider_wanted_timer_.StartOneShot(kTimeToShowVolumeSlider,
FROM_HERE);
}
}
void MediaControlsImpl::CloseVolumeSliderIfNecessary() {
if (ShouldCloseVolumeSlider())
if (ShouldCloseVolumeSlider()) {
volume_slider_->CloseSlider();
if (volume_slider_wanted_timer_.IsActive())
volume_slider_wanted_timer_.Stop();
}
}
bool MediaControlsImpl::ShouldOpenVolumeSlider() const {
......
third_party/blink/renderer/modules/media_controls/media_controls_impl.h
View file @ 305d9733
......@@ -142,6 +142,7 @@ class MODULES_EXPORT MediaControlsImpl final : public HTMLDivElement,
void ToggleOverflowMenu();
bool OverflowMenuVisible();
void VolumeSliderWantedTimerFired(TimerBase*);
void OpenVolumeSliderIfNecessary();
void CloseVolumeSliderIfNecessary();
......@@ -415,6 +416,10 @@ class MODULES_EXPORT MediaControlsImpl final : public HTMLDivElement,
TaskRunnerTimer<MediaControlsImpl> tap_timer_;
bool is_paused_for_double_tap_ = false;
// Timer to delay showing the volume slider to avoid accidental triggering
// of the slider
TaskRunnerTimer<MediaControlsImpl> volume_slider_wanted_timer_;
bool is_test_mode_ = false;
};
......
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