Bug 107398

Summary: [GTK][GStreamer] FullscreenVideoControllerGtk implementation
Product: WebKit Reporter: Philippe Normand <pnormand>
Component: MediaAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: eric.carlson, feature-media-reviews, gustavo, menard, mrobinson, pnormand, webkit.review.bot, zan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 106760, 107458    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

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.