WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
138717
[Media] Timeline scrubber not updating as the video plays
https://bugs.webkit.org/show_bug.cgi?id=138717
Summary
[Media] Timeline scrubber not updating as the video plays
Dean Jackson
Reported
2014-11-13 16:28:07 PST
[Media] Timeline scrubber not updating as the video plays
Attachments
Patch
(4.98 KB, patch)
2014-11-13 16:45 PST
,
Dean Jackson
no flags
Details
Formatted Diff
Diff
Patch
(5.92 KB, patch)
2014-11-14 11:27 PST
,
Dean Jackson
no flags
Details
Formatted Diff
Diff
Patch
(6.14 KB, patch)
2014-11-14 11:47 PST
,
Dean Jackson
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Dean Jackson
Comment 1
2014-11-13 16:45:21 PST
Created
attachment 241519
[details]
Patch
Jer Noble
Comment 2
2014-11-13 19:48:30 PST
Dino, I don't think this is the right fix. The problem is that the controls aren't updating when they go from hidden to showing; instead, all the deferred updates need to be performed at that time.
Dean Jackson
Comment 3
2014-11-14 03:13:43 PST
(In reply to
comment #2
)
> Dino, I don't think this is the right fix. The problem is that the controls > aren't updating when they go from hidden to showing; instead, all the > deferred updates need to be performed at that time.
The problem is that there are five states: - hidden - show - paused - :hover - no CSS class (which is kind-of a hidden, but not with display none) (Note: this is fairly confusing, not only because the tense changes in the state names :), but because the hover state is not able to be queried from script easily) I think we can simplify this into a single "visible" state (although I'd probably call it "hidden"). I'm not sure we need the existing "hidden" - the display: none doesn't buy us much if we're smart about not updating the controls all the time. You're right that the controls don't update when they become visible. I think we should add that in showControls(), although I think it would need to go in the deferred call to timeline metrics update (or that should also update the controls). The reason this current patch passes the test is that it gets an event from the media which triggers the scrubber update.
Dean Jackson
Comment 4
2014-11-14 11:27:18 PST
Created
attachment 241602
[details]
Patch
Jer Noble
Comment 5
2014-11-14 11:35:04 PST
Comment on
attachment 241602
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=241602&action=review
> Source/WebCore/Modules/mediacontrols/mediaControlsApple.js:1038 > + this.updateTime();
You need to call this.updateProgress() here too.
> Source/WebCore/Modules/mediacontrols/mediaControlsApple.js:1053 > - return !this.isAudio() && (!this.controls.panel.classList.contains(this.ClassNames.show) || this.controls.panel.classList.contains(this.ClassNames.hidden)); > + return !this.isAudio() && this.controls.panel.classList.contains(this.ClassNames.hidden);
With this change, the controls will continue to update when they are hidden due to inactivity during playback. Is this what you want?
Dean Jackson
Comment 6
2014-11-14 11:46:42 PST
(In reply to
comment #5
)
> Comment on
attachment 241602
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=241602&action=review
> > > Source/WebCore/Modules/mediacontrols/mediaControlsApple.js:1038 > > + this.updateTime(); > > You need to call this.updateProgress() here too.
Yeah.
> > > Source/WebCore/Modules/mediacontrols/mediaControlsApple.js:1053 > > - return !this.isAudio() && (!this.controls.panel.classList.contains(this.ClassNames.show) || this.controls.panel.classList.contains(this.ClassNames.hidden)); > > + return !this.isAudio() && this.controls.panel.classList.contains(this.ClassNames.hidden); > > With this change, the controls will continue to update when they are hidden > due to inactivity during playback. Is this what you want?
That's probably ok, since the updates would be fairly rare. e.g. we're not getting the playback tick events.
Dean Jackson
Comment 7
2014-11-14 11:47:31 PST
Created
attachment 241608
[details]
Patch
Jer Noble
Comment 8
2014-11-14 12:12:06 PST
(In reply to
comment #6
)
> (In reply to
comment #5
) > > Comment on
attachment 241602
[details]
> > Patch > > > > View in context: > >
https://bugs.webkit.org/attachment.cgi?id=241602&action=review
> > > > > Source/WebCore/Modules/mediacontrols/mediaControlsApple.js:1038 > > > + this.updateTime(); > > > > You need to call this.updateProgress() here too. > > Yeah. > > > > > > Source/WebCore/Modules/mediacontrols/mediaControlsApple.js:1053 > > > - return !this.isAudio() && (!this.controls.panel.classList.contains(this.ClassNames.show) || this.controls.panel.classList.contains(this.ClassNames.hidden)); > > > + return !this.isAudio() && this.controls.panel.classList.contains(this.ClassNames.hidden); > > > > With this change, the controls will continue to update when they are hidden > > due to inactivity during playback. Is this what you want? > > That's probably ok, since the updates would be fairly rare. e.g. we're not > getting the playback tick events.
No, the opposite. The controls are hidden during playback, so you'd be getting many, many playback tick events.
Dean Jackson
Comment 9
2014-11-14 12:58:23 PST
(In reply to
comment #8
)
> > > With this change, the controls will continue to update when they are hidden > > > due to inactivity during playback. Is this what you want? > > > > That's probably ok, since the updates would be fairly rare. e.g. we're not > > getting the playback tick events. > > No, the opposite. The controls are hidden during playback, so you'd be > getting many, many playback tick events.
Sorry, I misunderstood your comment. I'm pretty sure they don't, for a couple of reasons: - I turned on repaint counters and didn't see painting while the controls were hidden - As we transition to the hidden state, the opacity transition causes handlePanelTransitionEnd, which adds the hidden class. controlsAreHidden() tests for hidden. So we'll update as we are fading out (which would be correct) but not once we've actually disappeared.
Jer Noble
Comment 10
2014-11-14 13:07:56 PST
Comment on
attachment 241608
[details]
Patch (In reply to
comment #9
)
> (In reply to
comment #8
) > > > > > With this change, the controls will continue to update when they are hidden > > > > due to inactivity during playback. Is this what you want? > > > > > > That's probably ok, since the updates would be fairly rare. e.g. we're not > > > getting the playback tick events. > > > > No, the opposite. The controls are hidden during playback, so you'd be > > getting many, many playback tick events. > > Sorry, I misunderstood your comment. > > I'm pretty sure they don't, for a couple of reasons: > > - I turned on repaint counters and didn't see painting while the controls > were hidden > > - As we transition to the hidden state, the opacity transition causes > handlePanelTransitionEnd, which adds the hidden class. controlsAreHidden() > tests for hidden. > > So we'll update as we are fading out (which would be correct) but not once > we've actually disappeared.
Okay, r=me.
Dean Jackson
Comment 11
2014-11-14 13:56:05 PST
Committed
r176139
: <
http://trac.webkit.org/changeset/176139
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug