Bug 144700 - Media Controls: Scrubber should not follow actual video time, causes scrubber to be jumpy
Summary: Media Controls: Scrubber should not follow actual video time, causes scrubber...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-05-06 11:56 PDT by Roger Fong
Modified: 2015-05-07 15:33 PDT (History)
3 users (show)

See Also:


Attachments
patch (2.31 KB, patch)
2015-05-06 12:01 PDT, Roger Fong
jer.noble: review-
Details | Formatted Diff | Diff
patch (5.20 KB, patch)
2015-05-06 23:10 PDT, Roger Fong
no flags Details | Formatted Diff | Diff
patch (4.32 KB, patch)
2015-05-07 10:05 PDT, Roger Fong
no flags Details | Formatted Diff | Diff
patch (2.92 KB, patch)
2015-05-07 11:57 PDT, Roger Fong
jer.noble: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Roger Fong 2015-05-06 11:56:55 PDT
We should
a) draw the scrubber based on the input range position
b) also, update the time and draw the timeline on timelinemousemove, not wrappermousemove.

rdar://problem/19997548
Comment 1 Roger Fong 2015-05-06 12:01:17 PDT
Created attachment 252502 [details]
patch
Comment 2 Dean Jackson 2015-05-06 12:27:30 PDT
Comment on attachment 252502 [details]
patch

I guess we have to do the same for iOS.
Comment 3 Jer Noble 2015-05-06 12:30:11 PDT
Comment on attachment 252502 [details]
patch

This doesn't solve the underlying problem. You are still querying the <video> element's currentTime when interacting with the timeline slider.
Comment 4 Jer Noble 2015-05-06 12:32:11 PDT
Comment on attachment 252502 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=252502&action=review

> Source/WebCore/Modules/mediacontrols/mediaControlsApple.js:885
> +            this.updateTime();

updateTime() sets timeline.value based on video.currentTime. That's the underlying problem.
Comment 5 Roger Fong 2015-05-06 23:10:54 PDT
Created attachment 252572 [details]
patch
Comment 6 Darin Adler 2015-05-07 09:33:16 PDT
Comment on attachment 252572 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=252572&action=review

> Source/WebCore/platform/graphics/cocoa/FontCascadeCocoa.mm:369
> -        CGContextSetFontAntialiasingStyle(cgContext, kCGFontAntialiasingStyleUnfilteredCustomDilation);
> +        CGContextSetFontAntialiasingStyle(cgContext, kCGFontAntialiasingStyleCustomDilation);

Surely this was included in the patch by accident?
Comment 7 Roger Fong 2015-05-07 10:05:09 PDT
> Surely this was included in the patch by accident?

whoops yup
Comment 8 Roger Fong 2015-05-07 10:05:46 PDT
Created attachment 252597 [details]
patch
Comment 9 Jer Noble 2015-05-07 10:36:56 PDT
Comment on attachment 252597 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=252597&action=review

> Source/WebCore/Modules/mediacontrols/mediaControlsApple.js:888
>      handleTimelineMouseMove: function(event)
>      {
> +        this.updateControlsWhileScrubbing();
> +

Shouldn't this be handled in handleTimelineInput()?  (And in handleTimelineChange())? Doing it this way means that the controls won't update when the timeline is changed through Accessibility.

> Source/WebCore/Modules/mediacontrols/mediaControlsApple.js:1408
> +        
> +        if (this.isWaitingForBuffer)
> +            return;

Why is this necessary? Surely we're not getting "timechange" events when we're in the "waiting" state.
Comment 10 Roger Fong 2015-05-07 11:57:31 PDT
Created attachment 252603 [details]
patch
Comment 11 Roger Fong 2015-05-07 12:01:23 PDT
(In reply to comment #10)
> Created attachment 252603 [details]
> patch

I think handleTimelineInput is sufficient since we get an input and change event at the same timeline position when ending the scrub.
Comment 12 Roger Fong 2015-05-07 12:03:14 PDT
I should probably do an iOS fix in this patch as well
Comment 13 Roger Fong 2015-05-07 15:33:34 PDT
http://trac.webkit.org/changeset/183953