RESOLVED FIXED Bug 64428
[EFL] Add fullscreen button to media control UI 
https://bugs.webkit.org/show_bug.cgi?id=64428
Summary [EFL] Add fullscreen button to media control UI
Gyuyoung Kim
Reported 2011-07-12 20:23:29 PDT
Implement paintMediaFullscreenButton and Emit fullscreen signal in addition to implement functions for full screen in ChromeClientEfl.
Attachments
Proposed Patch (10.20 KB, patch)
2011-07-12 20:26 PDT, Gyuyoung Kim 		no flags
DetailsFormatted DiffDiff
Modified Patch (10.20 KB, patch)
2011-07-13 18:13 PDT, Gyuyoung Kim 		no flags
DetailsFormatted DiffDiff
Patch (10.14 KB, patch)
2011-07-17 21:23 PDT, Gyuyoung Kim 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Gyuyoung Kim
Comment 1 2011-07-12 20:26:17 PDT
Created attachment 100615 [details] Proposed Patch This patch doesn't add normal screen button yet on full screen mode. The normal screen button will be added after finishing implementation of full screen mode.
Raphael Kubo da Costa (:rakuco)
Comment 2 2011-07-13 06:23:46 PDT
Looks mostly OK to me, with a few nitpicks below: > Source/WebCore/ChangeLog:6 > + Implement paintMediaFullscreenButton and Emit fullscreen signal. Emit -> emit > Source/WebKit/efl/ChangeLog:6 > + Implement functions for full screen in ChrmeClientEfl in order to display a full screen button on media control UI. ChrmeClientEfl -> ChromeClientEfl > Source/WebKit/efl/DefaultTheme/widget/mediacontrol/fullscreenbutton/fullscreen_button.edc:4 > + Copyright (C) 2011 Samsung Electronics Are all these copyrights really needed for such a small file?
Gyuyoung Kim
Comment 3 2011-07-13 18:13:13 PDT
Created attachment 100746 [details] Modified Patch I fixed wrong descriptions. >> Are all these copyrights really needed for such a small file? Other .edc files have same copyrights. So, I think this file need to have same copyright as well.
Raphael Kubo da Costa (:rakuco)
Comment 4 2011-07-14 07:08:35 PDT
I Am Not A Lawyer (TM), but in theory the other files should have all those copyrights because they derive from the original work done by INdT, right? This new EDC looks like some new work whose copyright should belong only to Samsung. As for the rest, r+ from my side (dunno if you need to consult your legal side or if it can be committed as-is).
Gyuyoung Kim
Comment 5 2011-07-17 21:23:30 PDT
Created attachment 101123 [details] Patch ok, I think previous copyright is unneeded. If this patch is based on other patch which has previous copyright, I think we should mention previous copyright. But, this .edc file refer to Lucas comment. So, I leave profusion copyright.
Gyuyoung Kim
Comment 6 2011-07-17 22:46:33 PDT
Could you anyone review this patch?
Raphael Kubo da Costa (:rakuco)
Comment 7 2011-07-18 06:01:18 PDT
Informal r+ from my side.
WebKit Review Bot
Comment 8 2011-08-02 08:09:00 PDT
Comment on attachment 101123 [details] Patch Clearing flags on attachment: 101123 Committed r92189: <http://trac.webkit.org/changeset/92189>
WebKit Review Bot
Comment 9 2011-08-02 08:09:05 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.