WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(41.85 KB, patch)
2013-01-26 07:15 PST
,
Philippe Normand
no flags
Details
Formatted Diff
Diff
Patch
(41.89 KB, patch)
2013-01-30 07:00 PST
,
Philippe Normand
no flags
Details
Formatted Diff
Diff
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.
Top of Page
Format For Printing
XML
Clone This Bug