Bug 126301

Summary: [Gtk] Add fast-forward and reverse media controls
Product: WebKit Reporter: Brendan Long <self>
Component: New BugsAssignee: Brendan Long <b.long>
Status: RESOLVED INVALID    
Severity: Normal CC: calvaris, commit-queue, eric.carlson, esprehn+autocc, glenn, gyuyoung.kim, jer.noble, macpherson, menard, mrobinson, pnormand, william.jon.mccann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 123097    
Bug Blocks:    
Attachments:
Description Flags
Patch
beidson: review-
Screenshot of new controls none

Description Brendan Long 2013-12-30 12:41:43 PST
[Gtk] Add fast-forward and reverse media controls
Comment 1 Brendan Long 2013-12-30 13:16:06 PST
Created attachment 220123 [details]
Patch
Comment 2 Brendan Long 2013-12-30 13:18:46 PST
Created attachment 220124 [details]
Screenshot of new controls

Here's a screenshot of the new controls.

I'm not sure if you guys will want this, but someone on my project did, so I figured I'd offer it to you. I think the various media control patches would need to be rebaselined, but I'm on Fedora, so I don't think the pixel tests would pass if I did it.
Comment 3 Brendan Long 2014-01-06 12:03:53 PST
Philippe, what do you think of this?
Comment 4 Philippe Normand 2014-01-26 20:37:56 PST
Adding Xabier in CC.

I don't think this patch is worth the effort for the current/old controls but it'd be good to have it in the new controls :)
Comment 5 Xabier Rodríguez Calvar 2014-02-10 00:05:47 PST
(In reply to comment #4)
> Adding Xabier in CC.
> 
> I don't think this patch is worth the effort for the current/old controls but it'd be good to have it in the new controls :)

Agree. Anyway, I'm adding Jon McCann to see what he thinks about it.
Comment 6 Brendan Long 2014-02-10 08:03:02 PST
(In reply to comment #4)
> I don't think this patch is worth the effort for the current/old controls but it'd be good to have it in the new controls :)

How would I turn these new controls on?
Comment 7 Xabier Rodríguez Calvar 2014-02-10 09:46:15 PST
(In reply to comment #6)
> (In reply to comment #4)
> > I don't think this patch is worth the effort for the current/old controls but it'd be good to have it in the new controls :)
> 
> How would I turn these new controls on?

You can't just turn them on now. I am working on a patch for bug 123097, which I added as a dependency today. I don't think I will change the important bits of the code too much so I think you can safely use my latest patch as a starting point. Rebasing after some changes I am planning to do shouldn't be too complicated. Other thing you can do is waiting a bit until I finished with bug 123097, which should happen in one or two days.
Comment 8 Xabier Rodríguez Calvar 2014-02-10 09:47:43 PST
Jon, can you have a look at the screenshot that Brendan uploaded and give your opinion about it?
Comment 9 Brendan Long 2014-02-10 09:49:27 PST
(In reply to comment #7)
> You can't just turn them on now. I am working on a patch for bug 123097, which I added as a dependency today. I don't think I will change the important bits of the code too much so I think you can safely use my latest patch as a starting point. Rebasing after some changes I am planning to do shouldn't be too complicated. Other thing you can do is waiting a bit until I finished with bug 123097, which should happen in one or two days.

Ok, I'll just wait until you're done.
Comment 10 William Jon McCann 2014-02-10 14:10:32 PST
Can you describe a bit about why they would be needed? And why it is better than using the position slider?
Comment 11 Brendan Long 2014-02-10 14:58:23 PST
(In reply to comment #10)
> Can you describe a bit about why they would be needed? And why it is better than using the position slider?

These change the playback speed, not position. We wanted it to make testing the DLNA "Remote UI" spec more convenient, but I don't know if normal people would be interested.
Comment 12 Xabier Rodríguez Calvar 2014-02-11 07:36:48 PST
(In reply to comment #11)
> > Can you describe a bit about why they would be needed? And why it is better than using the position slider?
> 
> These change the playback speed, not position. We wanted it to make testing the DLNA "Remote UI" spec more convenient, but I don't know if normal people would be interested.

Jon, if you don't find them interesting, we can just add them with a compilation conditional or something.
Comment 13 Brendan Long 2014-02-11 07:45:59 PST
(In reply to comment #12)
> Jon, if you don't find them interesting, we can just add them with a compilation conditional or something.

You don't have to add them at all if you don't think they're useful. You won't hurt my feelings ;)
Comment 14 Xabier Rodríguez Calvar 2014-02-11 08:51:45 PST
(In reply to comment #13)
> > Jon, if you don't find them interesting, we can just add them with a compilation conditional or something.
> 
> You don't have to add them at all if you don't think they're useful. You won't hurt my feelings ;)

:D I wasn't thinking about your feelings but about your needs in combination to what the designers (Jon in this case) think is best.
Comment 15 Martin Robinson 2016-03-09 12:34:39 PST
Is this patch still valid with the media controls rewrite?
Comment 16 Brendan Long 2016-03-09 12:44:11 PST
Probably not.
Comment 17 Brady Eidson 2016-05-24 22:05:53 PDT
Comment on attachment 220123 [details]
Patch

Assuming that patches for review since 2013 are stale, r-