Skip to content

GitLab

T
tangled
Commit 1647baca authored by philipj@opera.com's avatar philipj@opera.com
Browse files
Options

Ignore MediaController in the video fullscreen button logic 

There were three problems:

 1. When fullScreenEnabled() was false, HTMLMediaElement's
    enterFullscreen() was used as a fallback, but that function checks
    the setting and does nothing. This has been the case since cleanup
    from April 2013, r149489: https://codereview.chromium.org/14670004

 2. MediaController's hasVideo() returned true if any of the slave
    elements had video, which could show the button for a video element
    with no video.

 3. The fullscreen button didn't do anything for slaved media elements.

The added tests verify that (1) no fullscreen button is shown when the
fullscreen feature is disabled or (2) when there is no video track, and
(3) that clicking the fullscreen button actually enters fullscreen.

BUG=351661

Review URL: https://codereview.chromium.org/196793021

git-svn-id: svn://svn.chromium.org/blink/trunk@169429 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent f3eaf81c
Show whitespace changes
Inline Side-by-side
Showing with 120 additions and 34 deletions
third_party/WebKit/LayoutTests/media/video-controls-fullscreen-disabled-expected.txt 0 → 100644
View file @ 1647baca
This is a testharness.js-based test.
PASS video controls fullscreen button with fullscreen feature disabled
Harness: the test ran to completion.
third_party/WebKit/LayoutTests/media/video-controls-fullscreen-disabled.html 0 → 100644
View file @ 1647baca
<!DOCTYPE html>
<html>
<head>
<title>video controls fullscreen button with fullscreen feature disabled</title>
<script src="../resources/testharness.js"></script>
<script src="../resources/testharnessreport.js"></script>
<script src="media-file.js"></script>
<script src="media-controls.js"></script>
<script src="video-controls-fullscreen.js"></script>
</head>
<body>
<div id="log"></div>
<script>fullscreen_test(null, false);</script>
</body>
</html>
third_party/WebKit/LayoutTests/media/video-controls-fullscreen-expected.txt 0 → 100644
View file @ 1647baca
This is a testharness.js-based test.
PASS video controls fullscreen button
Harness: the test ran to completion.
third_party/WebKit/LayoutTests/media/video-controls-fullscreen-with-controller-expected.txt 0 → 100644
View file @ 1647baca
This is a testharness.js-based test.
PASS video controls fullscreen button with MediaController
Harness: the test ran to completion.
third_party/WebKit/LayoutTests/media/video-controls-fullscreen-with-controller.html 0 → 100644
View file @ 1647baca
<!DOCTYPE html>
<html>
<head>
<title>video controls fullscreen button with MediaController</title>
<script src="../resources/testharness.js"></script>
<script src="../resources/testharnessreport.js"></script>
<script src="media-file.js"></script>
<script src="media-controls.js"></script>
<script src="video-controls-fullscreen.js"></script>
</head>
<body>
<div id="log"></div>
<script>fullscreen_test(new MediaController(), true);</script>
</body>
</html>
third_party/WebKit/LayoutTests/media/video-controls-fullscreen.html 0 → 100644
View file @ 1647baca
<!DOCTYPE html>
<html>
<head>
<title>video controls fullscreen button</title>
<script src="../resources/testharness.js"></script>
<script src="../resources/testharnessreport.js"></script>
<script src="media-file.js"></script>
<script src="media-controls.js"></script>
<script src="video-controls-fullscreen.js"></script>
</head>
<body>
<div id="log"></div>
<script>fullscreen_test(null, true);</script>
</body>
</html>
third_party/WebKit/LayoutTests/media/video-controls-fullscreen.js 0 → 100644
View file @ 1647baca
"use strict";
function fullscreen_test(controller, fullScreenEnabled)
{
if (window.internals)
window.internals.settings.setFullScreenEnabled(fullScreenEnabled);
async_test(function(t)
{
var v1 = document.createElement("video");
var v2 = document.createElement("video");
v1.controls = v2.controls = true;
v1.controller = v2.controller = controller;
v1.src = findMediaFile("video", "content/test");
v2.src = findMediaFile("audio", "content/test");
document.body.appendChild(v1);
document.body.appendChild(v2);
// load event fires when both video elements are ready
window.addEventListener("load", t.step_func(function()
{
function assert_button_hidden(elm)
{
assert_array_equals(mediaControlsButtonDimensions(elm, "fullscreen-button"), [0, 0]);
}
// no fullscreen button for a video element with no video track
assert_button_hidden(v2);
if (fullScreenEnabled) {
// click the fullscreen button
var coords = mediaControlsButtonCoordinates(v1, "fullscreen-button");
eventSender.mouseMoveTo(coords[0], coords[1]);
eventSender.mouseDown();
eventSender.mouseUp();
// wait for the fullscreenchange event
} else {
// no fullscreen button when fullscreen is disabled
assert_button_hidden(v1);
t.done();
}
}));
v1.addEventListener("webkitfullscreenchange", t.step_func(function()
{
t.done();
}));
v2.addEventListener("webkitfullscreenchange", t.step_func(function()
{
assert_unreached();
}));
});
}
third_party/WebKit/LayoutTests/media/video-enter-fullscreen-without-user-gesture.html
View file @ 1647baca
<html>
<head>
<title>Test that video webkitRequestFullscreen() works without any user gesture if the requirement is removed.</title>
<script src=media-controls.js></script>
<script src=media-file.js></script>
<script src=video-test.js></script>
<script>
......
third_party/WebKit/Source/core/html/HTMLMediaElement.h
View file @ 1647baca
......@@ -84,7 +84,7 @@ public:
MediaPlayer* player() const { return m_player.get(); }
blink::WebMediaPlayer* webMediaPlayer() const { return m_player ? m_player->webMediaPlayer() : 0; }
virtual bool hasVideo() const OVERRIDE { return false; }
virtual bool hasVideo() const { return false; }
virtual bool hasAudio() const OVERRIDE FINAL;
bool supportsSave() const;
......@@ -233,7 +233,7 @@ public:
bool hasSingleSecurityOrigin() const { return !m_player || m_player->hasSingleSecurityOrigin(); }
bool isFullscreen() const;
virtual void enterFullscreen() OVERRIDE FINAL;
void enterFullscreen();
bool hasClosedCaptions() const;
bool closedCaptionsVisible() const;
......
third_party/WebKit/Source/core/html/MediaController.cpp
View file @ 1647baca
......@@ -565,15 +565,6 @@ bool MediaController::hasAudio() const
return false;
}
bool MediaController::hasVideo() const
{
for (size_t index = 0; index < m_mediaElements.size(); ++index) {
if (m_mediaElements[index]->hasVideo())
return true;
}
return false;
}
void MediaController::beginScrubbing()
{
for (size_t index = 0; index < m_mediaElements.size(); ++index)
......
third_party/WebKit/Source/core/html/MediaController.h
View file @ 1647baca
......@@ -84,10 +84,7 @@ public:
enum PlaybackState { WAITING, PLAYING, ENDED };
const AtomicString& playbackState() const;
virtual void enterFullscreen() OVERRIDE { }
virtual bool hasAudio() const OVERRIDE;
virtual bool hasVideo() const OVERRIDE;
virtual void beginScrubbing() OVERRIDE;
virtual void endScrubbing() OVERRIDE;
......
third_party/WebKit/Source/core/html/MediaControllerInterface.h
View file @ 1647baca
......@@ -49,10 +49,7 @@ public:
virtual bool muted() const = 0;
virtual void setMuted(bool) = 0;
virtual void enterFullscreen() = 0;
virtual bool hasAudio() const = 0;
virtual bool hasVideo() const = 0;
virtual void beginScrubbing() = 0;
virtual void endScrubbing() = 0;
......
third_party/WebKit/Source/core/html/shadow/MediaControlElements.cpp
View file @ 1647baca
......@@ -38,7 +38,6 @@
#include "core/events/MouseEvent.h"
#include "core/events/ThreadLocalEventNames.h"
#include "core/frame/LocalFrame.h"
#include "core/frame/Settings.h"
#include "core/html/HTMLVideoElement.h"
#include "core/html/shadow/MediaControls.h"
#include "core/html/track/TextTrack.h"
......@@ -588,19 +587,10 @@ PassRefPtr<MediaControlFullscreenButtonElement> MediaControlFullscreenButtonElem
void MediaControlFullscreenButtonElement::defaultEventHandler(Event* event)
{
if (event->type() == EventTypeNames::click) {
// Only use the new full screen API if the fullScreenEnabled setting has
// been explicitly enabled. Otherwise, use the old fullscreen API. This
// allows apps which embed a WebView to retain the existing full screen
// video implementation without requiring them to implement their own full
// screen behavior.
if (document().settings() && document().settings()->fullScreenEnabled()) {
if (FullscreenElementStack::isActiveFullScreenElement(&mediaElement()))
FullscreenElementStack::from(document()).webkitCancelFullScreen();
else
FullscreenElementStack::from(document()).requestFullScreenForElement(&mediaElement(), 0, FullscreenElementStack::ExemptIFrameAllowFullScreenRequirement);
} else {
mediaControllerInterface().enterFullscreen();
}
event->setDefaultHandled();
}
HTMLInputElement::defaultEventHandler(event);
......
third_party/WebKit/Source/core/html/shadow/MediaControls.cpp
View file @ 1647baca
......@@ -29,6 +29,7 @@
#include "bindings/v8/ExceptionStatePlaceholder.h"
#include "core/events/MouseEvent.h"
#include "core/frame/Settings.h"
#include "core/html/HTMLMediaElement.h"
#include "core/html/MediaController.h"
#include "core/rendering/RenderTheme.h"
......@@ -191,7 +192,7 @@ void MediaControls::reset()
refreshClosedCaptionsButtonVisibility();
if (mediaControllerInterface().hasVideo())
if (mediaElement().hasVideo() && document().settings() && document().settings()->fullScreenEnabled())
m_fullScreenButton->show();
else
m_fullScreenButton->hide();
......@@ -247,7 +248,7 @@ void MediaControls::playbackProgressed()
m_timeline->setPosition(mediaControllerInterface().currentTime());
updateCurrentTimeDisplay();
if (!m_isMouseOverControls && mediaControllerInterface().hasVideo())
if (!m_isMouseOverControls && mediaElement().hasVideo())
makeTransparent();
}
......
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