Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(9)

Unified Diff: Source/core/html/shadow/MediaControlElements.cpp

Issue 614263002: Drop all timeline seeks outside of the seekable range. (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@seek
Patch Set: Created 6 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: Source/core/html/shadow/MediaControlElements.cpp
diff --git a/Source/core/html/shadow/MediaControlElements.cpp b/Source/core/html/shadow/MediaControlElements.cpp
index 2439bb62e1be659462f2a824b329da9646fcf7fd..d0ab0dfc2dc79baef3ef0019049bb51fa975192f 100644
--- a/Source/core/html/shadow/MediaControlElements.cpp
+++ b/Source/core/html/shadow/MediaControlElements.cpp
@@ -37,6 +37,7 @@
#include "core/frame/LocalFrame.h"
#include "core/html/HTMLVideoElement.h"
#include "core/html/MediaController.h"
+#include "core/html/TimeRanges.h"
#include "core/html/shadow/MediaControls.h"
#include "core/html/track/TextTrack.h"
#include "core/html/track/vtt/VTTRegionList.h"
@@ -57,6 +58,14 @@ static const AtomicString& getMediaControlTimeRemainingDisplayElementShadowPseud
// This is the duration from mediaControls.css
static const double fadeOutDuration = 0.3;
+// Snap seeks to the nearest seekable() range if they're <= this percentage of the duration apart.
+// I.e., if the seekable() range is [4s, 5s] and the duration 10s, clicks on the timeline >= 3s or <= 6s
+// would snap to 4 and 5 respectively. Clicks outside of those ranges would be dropped.
+//
+// This ensures accidental clicks don't cause annoying timeline jumps, or worse, restart the entire
+// playback process when only a seek to zero is available.
+static const double snapPercentageForSeeks = 10 / 100.0;
philipj_slow 2014/10/01 11:14:22 This is 0.1 and not a percentage :)
+
static bool isUserInteractionEvent(Event* event)
{
const AtomicString& type = event->type();
@@ -418,10 +427,21 @@ void MediaControlTimelineElement::defaultEventHandler(Event* event)
if (event->type() == EventTypeNames::input) {
// FIXME: This will need to take the timeline offset into consideration
// once that concept is supported, see https://crbug.com/312699
- if (mediaElement().controller())
+ if (mediaElement().controller()) {
mediaElement().controller()->setCurrentTime(time);
philipj_slow 2014/10/01 11:14:22 Should the same rule not apply to a media element
DaleCurtis 2014/10/14 18:06:26 Done.
philipj_slow 2014/10/16 18:52:51 We appear to have miscommunicated :) I mean that t
DaleCurtis 2014/10/16 19:51:50 Sorry I still don't understand. :| Are you saying
philipj_slow 2014/10/16 20:03:18 Oops, sloppy reading on my part! You've already do
- else
- mediaElement().setCurrentTime(time, IGNORE_EXCEPTION);
+ } else {
+ // Only pass through seeks that are within the seekable range or close enough that we're sure
+ // the user didn't make an accidental click.
+ RefPtrWillBeRawPtr<TimeRanges> seekableRanges = mediaElement().seekable();
+ if (seekableRanges->contain(time)) {
+ mediaElement().setCurrentTime(time, IGNORE_EXCEPTION);
+ } else {
+ const double duration = getFloatingPointAttribute(maxAttr);
philipj_slow 2014/10/01 11:14:22 It took me a while to realize that the duration is
+ const double nearestTime = seekableRanges->nearest(time, mediaElement().currentTime());
+ if (duration && std::abs(time - nearestTime) / duration <= snapPercentageForSeeks)
+ mediaElement().setCurrentTime(time, IGNORE_EXCEPTION);
+ }
+ }
}
RenderSlider* slider = toRenderSlider(renderer());
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698