| Summary: | [GTK][GStreamer] FullscreenVideoControllerGtk implementation | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Philippe Normand <pnormand> | ||||||||
| Component: | Media | Assignee: | 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
Philippe Normand
2013-01-20 07:12:07 PST
Created attachment 183672 [details]
Patch
Created attachment 184864 [details]
Patch
Created attachment 185497 [details]
Patch
Rebased against ToT
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? 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 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. Committed r141566: <http://trac.webkit.org/changeset/141566> Comment on attachment 185497 [details]
Patch
Clearing r+ flag of landed patch
|