Bug 126301

Summary:

[Gtk] Add fast-forward and reverse media controls

Product:

WebKit

Reporter:

Brendan Long <self>

Component:

New Bugs

Assignee:

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:

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

Brendan Long

Reported 2013-12-30 12:41:43 PST

[Gtk] Add fast-forward and reverse media controls

Attachments
Patch (6.30 KB, patch) 2013-12-30 13:16 PST, Brendan Long	beidson: review-	Details Formatted Diff Diff
Screenshot of new controls (14.53 KB, image/png) 2013-12-30 13:18 PST, Brendan Long	no flags	Details
View All Add attachment proposed patch, testcase, etc.

Brendan Long

Comment 1 2013-12-30 13:16:06 PST

Created attachment 220123 [details] Patch

Brendan Long

Comment 2 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.

Brendan Long

Comment 3 2014-01-06 12:03:53 PST

Philippe, what do you think of this?

Philippe Normand

Comment 4 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 :)

Xabier Rodríguez Calvar

Comment 5 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.

Brendan Long

Comment 6 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?

Xabier Rodríguez Calvar

Comment 7 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.

Xabier Rodríguez Calvar

Comment 8 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?

Brendan Long

Comment 9 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.

William Jon McCann

Comment 10 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?

Brendan Long

Comment 11 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.

Xabier Rodríguez Calvar

Comment 12 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.

Brendan Long

Comment 13 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 ;)

Xabier Rodríguez Calvar

Comment 14 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.

Martin Robinson

Comment 15 2016-03-09 12:34:39 PST

Is this patch still valid with the media controls rewrite?

Brendan Long

Comment 16 2016-03-09 12:44:11 PST

Probably not.

Brady Eidson

Comment 17 2016-05-24 22:05:53 PDT

Comment on attachment 220123 [details] Patch Assuming that patches for review since 2013 are stale, r-

Note You need to log in before you can comment on or make changes to this bug.