Commit 12df2289 authored by steimel's avatar steimel Committed by Commit bot

Prevent Media Controls from hiding when the user is interacting via the keyboard

This CL prevents the controls from hiding by resetting the hide timer on events in two places:

1) Listening for focusin and input events on the MediaControls via defaultEventHandler, which get fired as the user tabs through the controls and as the user changes the timeline/volume sliders respectively.

2) Listening for keypress events on the media panel via adding an event listener to the panel element in MediaControlsMediaEventListener, which gets fired when the user mutes/unmutes, turns CC on/off, etc.

BUG=701440

Review-Url: https://codereview.chromium.org/2757323002
Cr-Commit-Position: refs/heads/master@{#462990}
parent 27458c3a
...@@ -93,6 +93,7 @@ class CORE_EXPORT MediaControls : public GarbageCollectedMixin { ...@@ -93,6 +93,7 @@ class CORE_EXPORT MediaControls : public GarbageCollectedMixin {
virtual void onLoadedMetadata() = 0; virtual void onLoadedMetadata() = 0;
virtual void onEnteredFullscreen() = 0; virtual void onEnteredFullscreen() = 0;
virtual void onExitedFullscreen() = 0; virtual void onExitedFullscreen() = 0;
virtual void onPanelKeypress() = 0;
virtual void beginScrubbing() = 0; virtual void beginScrubbing() = 0;
virtual void endScrubbing() = 0; virtual void endScrubbing() = 0;
virtual void updateCurrentTimeDisplay() = 0; virtual void updateCurrentTimeDisplay() = 0;
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include "core/events/Event.h" #include "core/events/Event.h"
#include "core/html/HTMLMediaElement.h" #include "core/html/HTMLMediaElement.h"
#include "core/html/media/MediaControls.h" #include "core/html/media/MediaControls.h"
#include "core/html/shadow/MediaControlElements.h"
#include "core/html/track/TextTrackList.h" #include "core/html/track/TextTrackList.h"
namespace blink { namespace blink {
...@@ -43,6 +44,12 @@ void MediaControlsMediaEventListener::attach() { ...@@ -43,6 +44,12 @@ void MediaControlsMediaEventListener::attach() {
textTracks->addEventListener(EventTypeNames::addtrack, this, false); textTracks->addEventListener(EventTypeNames::addtrack, this, false);
textTracks->addEventListener(EventTypeNames::change, this, false); textTracks->addEventListener(EventTypeNames::change, this, false);
textTracks->addEventListener(EventTypeNames::removetrack, this, false); textTracks->addEventListener(EventTypeNames::removetrack, this, false);
// Keypress events.
if (m_mediaControls->panelElement()) {
m_mediaControls->panelElement()->addEventListener(EventTypeNames::keypress,
this, false);
}
} }
void MediaControlsMediaEventListener::detach() { void MediaControlsMediaEventListener::detach() {
...@@ -55,6 +62,11 @@ void MediaControlsMediaEventListener::detach() { ...@@ -55,6 +62,11 @@ void MediaControlsMediaEventListener::detach() {
textTracks->removeEventListener(EventTypeNames::addtrack, this, false); textTracks->removeEventListener(EventTypeNames::addtrack, this, false);
textTracks->removeEventListener(EventTypeNames::change, this, false); textTracks->removeEventListener(EventTypeNames::change, this, false);
textTracks->removeEventListener(EventTypeNames::removetrack, this, false); textTracks->removeEventListener(EventTypeNames::removetrack, this, false);
if (m_mediaControls->panelElement()) {
m_mediaControls->panelElement()->removeEventListener(
EventTypeNames::keypress, this, false);
}
} }
bool MediaControlsMediaEventListener::operator==( bool MediaControlsMediaEventListener::operator==(
...@@ -127,6 +139,13 @@ void MediaControlsMediaEventListener::handleEvent( ...@@ -127,6 +139,13 @@ void MediaControlsMediaEventListener::handleEvent(
return; return;
} }
// Keypress events.
if (event->type() == EventTypeNames::keypress) {
if (event->currentTarget() == m_mediaControls->panelElement())
m_mediaControls->onPanelKeypress();
return;
}
NOTREACHED(); NOTREACHED();
} }
......
...@@ -765,6 +765,13 @@ void MediaControlsImpl::defaultEventHandler(Event* event) { ...@@ -765,6 +765,13 @@ void MediaControlsImpl::defaultEventHandler(Event* event) {
startHideMediaControlsTimer(); startHideMediaControlsTimer();
return; return;
} }
// If the user is interacting with the controls via the keyboard, don't hide
// the controls. This will fire when the user tabs between controls (focusin)
// or when they seek either the timeline or volume sliders (input).
if (event->type() == EventTypeNames::focusin ||
event->type() == EventTypeNames::input)
resetHideMediaControlsTimer();
} }
void MediaControlsImpl::hideMediaControlsTimerFired(TimerBase*) { void MediaControlsImpl::hideMediaControlsTimerFired(TimerBase*) {
...@@ -912,6 +919,13 @@ void MediaControlsImpl::onExitedFullscreen() { ...@@ -912,6 +919,13 @@ void MediaControlsImpl::onExitedFullscreen() {
startHideMediaControlsTimer(); startHideMediaControlsTimer();
} }
void MediaControlsImpl::onPanelKeypress() {
// If the user is interacting with the controls via the keyboard, don't hide
// the controls. This is called when the user mutes/unmutes, turns CC on/off,
// etc.
resetHideMediaControlsTimer();
}
void MediaControlsImpl::notifyElementSizeChanged(ClientRect* newSize) { void MediaControlsImpl::notifyElementSizeChanged(ClientRect* newSize) {
// Note that this code permits a bad frame on resize, since it is // Note that this code permits a bad frame on resize, since it is
// run after the relayout / paint happens. It would be great to improve // run after the relayout / paint happens. It would be great to improve
......
...@@ -126,6 +126,7 @@ class MODULES_EXPORT MediaControlsImpl final : public HTMLDivElement, ...@@ -126,6 +126,7 @@ class MODULES_EXPORT MediaControlsImpl final : public HTMLDivElement,
void onLoadedMetadata() override; void onLoadedMetadata() override;
void onEnteredFullscreen() override; void onEnteredFullscreen() override;
void onExitedFullscreen() override; void onExitedFullscreen() override;
void onPanelKeypress() override;
Document& ownerDocument() { return document(); } Document& ownerDocument() { return document(); }
DECLARE_VIRTUAL_TRACE(); DECLARE_VIRTUAL_TRACE();
......
...@@ -24,6 +24,7 @@ ...@@ -24,6 +24,7 @@
#include "platform/heap/Handle.h" #include "platform/heap/Handle.h"
#include "platform/testing/EmptyWebMediaPlayer.h" #include "platform/testing/EmptyWebMediaPlayer.h"
#include "platform/testing/HistogramTester.h" #include "platform/testing/HistogramTester.h"
#include "platform/testing/TestingPlatformSupport.h"
#include "platform/testing/UnitTestHelpers.h" #include "platform/testing/UnitTestHelpers.h"
#include "public/platform/WebMouseEvent.h" #include "public/platform/WebMouseEvent.h"
#include "public/platform/WebScreenInfo.h" #include "public/platform/WebScreenInfo.h"
...@@ -160,7 +161,9 @@ enum DownloadActionMetrics { ...@@ -160,7 +161,9 @@ enum DownloadActionMetrics {
class MediaControlsImplTest : public ::testing::Test { class MediaControlsImplTest : public ::testing::Test {
protected: protected:
virtual void SetUp() { virtual void SetUp() { initializePage(); }
void initializePage() {
Page::PageClients clients; Page::PageClients clients;
fillWithEmptyClients(clients); fillWithEmptyClients(clients);
clients.chromeClient = new MockChromeClient(); clients.chromeClient = new MockChromeClient();
...@@ -176,7 +179,7 @@ class MediaControlsImplTest : public ::testing::Test { ...@@ -176,7 +179,7 @@ class MediaControlsImplTest : public ::testing::Test {
m_pageHolder->frame().settings()->setScriptEnabled(true); m_pageHolder->frame().settings()->setScriptEnabled(true);
} }
void simulateRouteAvailabe() { void simulateRouteAvailable() {
m_mediaControls->mediaElement().remoteRouteAvailabilityChanged( m_mediaControls->mediaElement().remoteRouteAvailabilityChanged(
WebRemotePlaybackAvailability::DeviceAvailable); WebRemotePlaybackAvailability::DeviceAvailable);
} }
...@@ -321,7 +324,7 @@ TEST_F(MediaControlsImplTest, CastButtonRequiresRoute) { ...@@ -321,7 +324,7 @@ TEST_F(MediaControlsImplTest, CastButtonRequiresRoute) {
ASSERT_FALSE(isElementVisible(*castButton)); ASSERT_FALSE(isElementVisible(*castButton));
simulateRouteAvailabe(); simulateRouteAvailable();
ASSERT_TRUE(isElementVisible(*castButton)); ASSERT_TRUE(isElementVisible(*castButton));
} }
...@@ -335,7 +338,7 @@ TEST_F(MediaControlsImplTest, CastButtonDisableRemotePlaybackAttr) { ...@@ -335,7 +338,7 @@ TEST_F(MediaControlsImplTest, CastButtonDisableRemotePlaybackAttr) {
ASSERT_NE(nullptr, castButton); ASSERT_NE(nullptr, castButton);
ASSERT_FALSE(isElementVisible(*castButton)); ASSERT_FALSE(isElementVisible(*castButton));
simulateRouteAvailabe(); simulateRouteAvailable();
ASSERT_TRUE(isElementVisible(*castButton)); ASSERT_TRUE(isElementVisible(*castButton));
mediaControls().mediaElement().setBooleanAttribute( mediaControls().mediaElement().setBooleanAttribute(
...@@ -352,7 +355,7 @@ TEST_F(MediaControlsImplTest, CastOverlayDefault) { ...@@ -352,7 +355,7 @@ TEST_F(MediaControlsImplTest, CastOverlayDefault) {
mediaControls(), "-internal-media-controls-overlay-cast-button"); mediaControls(), "-internal-media-controls-overlay-cast-button");
ASSERT_NE(nullptr, castOverlayButton); ASSERT_NE(nullptr, castOverlayButton);
simulateRouteAvailabe(); simulateRouteAvailable();
ASSERT_TRUE(isElementVisible(*castOverlayButton)); ASSERT_TRUE(isElementVisible(*castOverlayButton));
} }
...@@ -362,7 +365,7 @@ TEST_F(MediaControlsImplTest, CastOverlayDisableRemotePlaybackAttr) { ...@@ -362,7 +365,7 @@ TEST_F(MediaControlsImplTest, CastOverlayDisableRemotePlaybackAttr) {
ASSERT_NE(nullptr, castOverlayButton); ASSERT_NE(nullptr, castOverlayButton);
ASSERT_FALSE(isElementVisible(*castOverlayButton)); ASSERT_FALSE(isElementVisible(*castOverlayButton));
simulateRouteAvailabe(); simulateRouteAvailable();
ASSERT_TRUE(isElementVisible(*castOverlayButton)); ASSERT_TRUE(isElementVisible(*castOverlayButton));
mediaControls().mediaElement().setBooleanAttribute( mediaControls().mediaElement().setBooleanAttribute(
...@@ -380,7 +383,7 @@ TEST_F(MediaControlsImplTest, CastOverlayMediaControlsDisabled) { ...@@ -380,7 +383,7 @@ TEST_F(MediaControlsImplTest, CastOverlayMediaControlsDisabled) {
ASSERT_NE(nullptr, castOverlayButton); ASSERT_NE(nullptr, castOverlayButton);
EXPECT_FALSE(isElementVisible(*castOverlayButton)); EXPECT_FALSE(isElementVisible(*castOverlayButton));
simulateRouteAvailabe(); simulateRouteAvailable();
EXPECT_TRUE(isElementVisible(*castOverlayButton)); EXPECT_TRUE(isElementVisible(*castOverlayButton));
document().settings()->setMediaControlsEnabled(false); document().settings()->setMediaControlsEnabled(false);
...@@ -793,4 +796,46 @@ TEST_F(MediaControlsImplTest, MAYBE_TimelineMetricsDragBackAndForth) { ...@@ -793,4 +796,46 @@ TEST_F(MediaControlsImplTest, MAYBE_TimelineMetricsDragBackAndForth) {
9 /* (-4m, -2m] */, 1); 9 /* (-4m, -2m] */, 1);
} }
TEST_F(MediaControlsImplTest, ControlsRemainVisibleDuringKeyboardInteraction) {
ScopedTestingPlatformSupport<TestingPlatformSupportWithMockScheduler>
m_platform;
// DocumentParserTiming has DCHECKS to make sure time > 0.0.
m_platform->advanceClockSeconds(1);
// Need to reinitialize page since we changed the platform.
initializePage();
ensureSizing();
Element* panel = mediaControls().panelElement();
mediaControls().mediaElement().setBooleanAttribute(HTMLNames::controlsAttr,
true);
mediaControls().mediaElement().setSrc("http://example.com");
mediaControls().mediaElement().play();
// Controls start out visible.
EXPECT_TRUE(isElementVisible(*panel));
// Tabbing between controls prevents controls from hiding.
m_platform->runForPeriodSeconds(2);
mediaControls().dispatchEvent(Event::create("focusin"));
m_platform->runForPeriodSeconds(2);
EXPECT_TRUE(isElementVisible(*panel));
// Seeking on the timeline or volume bar prevents controls from hiding.
mediaControls().dispatchEvent(Event::create("input"));
m_platform->runForPeriodSeconds(2);
EXPECT_TRUE(isElementVisible(*panel));
// Pressing a key prevents controls from hiding.
mediaControls().panelElement()->dispatchEvent(Event::create("keypress"));
m_platform->runForPeriodSeconds(2);
EXPECT_TRUE(isElementVisible(*panel));
// Once user interaction stops, controls can hide.
m_platform->runForPeriodSeconds(2);
EXPECT_FALSE(isElementVisible(*panel));
}
} // namespace blink } // namespace blink
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