RESOLVED FIXED 107398
[GTK][GStreamer] FullscreenVideoControllerGtk implementation 
https://bugs.webkit.org/show_bug.cgi?id=107398
Summary [GTK][GStreamer] FullscreenVideoControllerGtk implementation
Philippe Normand
Reported 2013-01-20 07:12:07 PST
See also bug 106760
Attachments
Patch (41.86 KB, patch)
2013-01-20 07:23 PST, Philippe Normand 		no flags
DetailsFormatted DiffDiff
Patch (41.85 KB, patch)
2013-01-26 07:15 PST, Philippe Normand 		no flags
DetailsFormatted DiffDiff
Patch (41.89 KB, patch)
2013-01-30 07:00 PST, Philippe Normand 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Philippe Normand
Comment 1 2013-01-20 07:23:51 PST
Created attachment 183672 [details] Patch
Philippe Normand
Comment 2 2013-01-26 07:15:10 PST
Created attachment 184864 [details] Patch
Philippe Normand
Comment 3 2013-01-30 07:00:42 PST
Created attachment 185497 [details] Patch Rebased against ToT
Gustavo Noronha (kov)
Comment 4 2013-01-31 05:11:31 PST
Comment on attachment 185497 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=185497&action=review > Source/WebCore/platform/graphics/gtk/FullscreenVideoControllerGtk.h:82 > + unsigned long m_exitFullcreenActionActivateSignalHandler; Two naming nits about this class: these really aren't handlers but rather ids, so how about s/Handler/ID/? And secondly, keeping Gtk out of this class' name would avoid having an #if PLATFORM() block for each of GStreamer-based ports in the MediaPlayerPrivate part of this patch, wouldn't it?
Philippe Normand
Comment 5 2013-01-31 06:16:17 PST
Hum but how would that work? I expect each port to have its own FullscreenController implementation deriving from the base class. Having a Gtk subsclass kinda makes sense to me :) I guess I'm misunderstanding your point
Philippe Normand
Comment 6 2013-02-01 02:21:18 PST
Carlos clarified this for me, the create method should be moved to the GStreamer base class, avoiding the ugly #define FullscreenVideoControllerClass in the player so I can directly use the base class there.
Philippe Normand
Comment 7 2013-02-01 02:27:25 PST
Committed r141566: <http://trac.webkit.org/changeset/141566>
Philippe Normand
Comment 8 2013-02-01 02:28:26 PST
Comment on attachment 185497 [details] Patch Clearing r+ flag of landed patch
Note You need to log in before you can comment on or make changes to this bug.