ENABLE_MEDIA_CONTROLS_SCRIPTS support javascript/css based media controls and MAC and GTK port use it.
Created attachment 228652 [details] Patch
Created attachment 228654 [details] Patch
Comment on attachment 228654 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=228654&action=review Don't we need to rebaseline or skip on media control test cases ? Besides I wonder how to support media control UI(e.g. media control buttons). Can we share the common media control buttons with other ports if we enable the ENABLE_MEDIA_CONTROLS_SCRIPT ? > Source/WebCore/ChangeLog:9 > + Copied from mediaControlsApple.css until Efl port have specific desigin. Typo : s/have/has/g, s/desigin/design/g > Source/WebCore/ChangeLog:11 > + Copied from mediaControlsApple.js until Efl port have specific desigin. ditto.
(In reply to comment #3) > (From update of attachment 228654 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=228654&action=review > > Don't we need to rebaseline or skip on media control test cases ? Besides I wonder how to support media control UI(e.g. media control buttons). Can we share the common media control buttons with other ports if we enable the ENABLE_MEDIA_CONTROLS_SCRIPT ? > Yes, we should rebaseline test cases. Should I do it in this bug? I considered that it looks big huge but I can do if you want. MEDIA_CONTROLS_SCRIPT provides the media control which is implemented by javascript and css. Now, I copied them from mac port so media control is same with mac port now. GTK port has their own javascript/css for media control. > > Source/WebCore/ChangeLog:9 > > + Copied from mediaControlsApple.css until Efl port have specific desigin. > > Typo : s/have/has/g, s/desigin/design/g > > > Source/WebCore/ChangeLog:11 > > + Copied from mediaControlsApple.js until Efl port have specific desigin. > > ditto. Thanks, I will fix it.
(In reply to comment #4) > (In reply to comment #3) > > (From update of attachment 228654 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=228654&action=review > > > > Don't we need to rebaseline or skip on media control test cases ? Besides I wonder how to support media control UI(e.g. media control buttons). Can we share the common media control buttons with other ports if we enable the ENABLE_MEDIA_CONTROLS_SCRIPT ? > > > > Yes, we should rebaseline test cases. > Should I do it in this bug? > I considered that it looks big huge but I can do if you want. Yes, we also should handle the test case if it may influence on our test result. > MEDIA_CONTROLS_SCRIPT provides the media control which is implemented by javascript and css. > > Now, I copied them from mac port so media control is same with mac port now. > GTK port has their own javascript/css for media control. Ok, I see.
(In reply to comment #4) > >Can we share the common media control buttons with other ports if we enable the ENABLE_MEDIA_CONTROLS_SCRIPT ? > > > > MEDIA_CONTROLS_SCRIPT provides the media control which is implemented by javascript and css. > > Now, I copied them from mac port so media control is same with mac port now. > GTK port has their own javascript/css for media control. In GTK we decided to share the widgets and provide a subclass for the specific bits. This has worked perfectly so far and in the future, if you decide to separate them, it is easier than merging.
Comment on attachment 228654 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=228654&action=review Complementing my previous comment. >> Source/WebCore/ChangeLog:11 >> + Copied from mediaControlsApple.js until Efl port have specific desigin. > > ditto. As I said, I would consider seriously to share the code with Apple, for that you can take GTK controls as example and define a skeleton for the subclass or even leaving it for later, because maybe your designers do not define any specific behavior. > Source/WebCore/Modules/mediacontrols/mediaControlsEfl.js:1 > +/* If you decide to go for sharing, you need to take GTK as skeleton of even remove this. > Source/WebCore/PlatformEfl.cmake:215 > +set(WebCore_USER_AGENT_SCRIPTS > + ${WEBCORE_DIR}/Modules/mediacontrols/mediaControlsEfl.js > +) If you decide for sharing, you need to add Apple controls here too. > Source/WebCore/platform/efl/RenderThemeEfl.cpp:1042 > + return ASCIILiteral(mediaControlsEflJavaScript); If you decide for sharing, you need to touch this too.
Created attachment 228726 [details] Patch
(In reply to comment #6) > (In reply to comment #4) > > >Can we share the common media control buttons with other ports if we enable the ENABLE_MEDIA_CONTROLS_SCRIPT ? > > > > > > > MEDIA_CONTROLS_SCRIPT provides the media control which is implemented by javascript and css. > > > > Now, I copied them from mac port so media control is same with mac port now. > > GTK port has their own javascript/css for media control. > > In GTK we decided to share the widgets and provide a subclass for the specific bits. This has worked perfectly so far and in the future, if you decide to separate them, it is easier than merging. Thanks for information. I removed mediaControlsEfl.js/css. When Efl port will extend the control, I will reference Gtk port.
(In reply to comment #9) > Thanks for information. My pleasure. > I removed mediaControlsEfl.js/css. > > When Efl port will extend the control, I will reference Gtk port. Yep, it looks better now.
Comment on attachment 228726 [details] Patch LGTM. BTW, When will you extend media control for EFL port ? EFL port may need to have specific controls in future.
Comment on attachment 228726 [details] Patch Clearing flags on attachment: 228726 Committed r166872: <http://trac.webkit.org/changeset/166872>
All reviewed patches have been landed. Closing bug.