[Webkit-unassigned] [Bug 73544] [EFL] Add Web Notification feature for Efl port

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=73544





--- Comment #6 from Kihong Kwon <kihong.kwon at samsung.com>  2011-12-09 03:20:21 PST ---
Fix all of your comment

(In reply to comment #3)
> (From update of attachment 117390 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=117390&action=review
> 
> > ChangeLog:6
> > +        This is a implementation about web notification for efl
> 
> %s/a/an/g.
> 
> > ChangeLog:8
> > +        * Source/cmake/OptionsEfl.cmake: elable ENABLE_NOTIFICATIONS feture
> 
> %s/elable/Enable/g
> 
> In addition, I think you need to make a test case for this feature.
> 
> > Source/WebKit/efl/ewk/ewk_private.h:234
> > +WebCore::NotificationPresenter* ewk_view_notification_presenter_get(const Evas_Object *o);
> 
> Do not use abbreviation in internal function.
> 
> > Source/WebKit/efl/ewk/ewk_view.cpp:139
> > +    WebCore::NotificationPresenter *notificationPresenter;
> 
> Move '*' to Data type.
> 
> > Source/WebKit/efl/ewk/ewk_view.cpp:3937
> > +#if ENABLE(NOTIFICATIONS)
> 
> Move ENABLE(NOTIFICATION) macro into function.
> 
> > Source/WebKit/efl/ewk/ewk_view.cpp:3938
> > +WebCore::NotificationPresenter* ewk_view_notification_presenter_get(const Evas_Object* o)
> 
> Do not use abbreviation in internal function. Change o with ewkView.
> 
> > Source/WebKit/efl/ewk/ewk_view.cpp:3946
> > +{
> 
> You should use ENABLE(NOTIFICATION) macro for implementation of public APIs.
> 
> > Source/WebKit/efl/ewk/ewk_view.cpp:3984
> > +    // START TEMP
> 
> What does mean ?
> 
> > Source/WebKit/efl/ewk/ewk_view.cpp:3986
> > +    // END TEMP
> 
> ditto.
> 
> > Source/WebKit/efl/ewk/ewk_view.cpp:3996
> > +void ewk_view_notification_clicked(const Evas_Object* ewkView, const Ewk_Notification* ewk_notification)
> 
> Do not use '_' in parameter variable.
> 
> > Source/WebKit/efl/ewk/ewk_view.cpp:4003
> > +void ewk_view_notification_closed(const Evas_Object* ewkView, const Ewk_Notification* ewk_notification)
> 
> ditto.
> 
> > Source/WebKit/efl/ewk/ewk_view.cpp:4010
> > +void ewk_view_notification_error(const Evas_Object* ewkView, const Ewk_Notification* ewk_notification)
> 
> ditto.
> 
> > Source/WebKit/efl/ewk/ewk_view.h:341
> > +/* #if ENABLE(NOTIFICATIONS) */
> 
> Remove this comment.
> 
> > Source/WebKit/efl/ewk/ewk_view.h:351
> > +/* #endif */
> 
> ditto.
> 
> > Source/WebKit/efl/ewk/ewk_view.h:2310
> > +/* #if ENABLE(NOTIFICATIONS) */
> 
> Remove this comment.
> 
> > Source/WebKit/efl/ewk/ewk_view.h:2319
> > +EAPI void ewk_view_notification_permission_callback(const Evas_Object* ewkView, Eina_Bool permitted, const char* domain);
> 
> Use abbreviation instead of full name
> 
> > Source/WebKit/efl/ewk/ewk_view.h:2338
> > +EAPI void ewk_view_notification_displayed(const Evas_Object* ewkView, const Ewk_Notification*);
> 
> We should use abbreviation in public header files. 
> 
> See also EFL WebKit Coding Style document.
> http://trac.webkit.org/wiki/EFLWebKitCodingStyle
> 
> > Source/WebKit/efl/ewk/ewk_view.h:2347
> > +EAPI void ewk_view_notification_clicked(const Evas_Object* ewkView, const Ewk_Notification*);
> 
> ditto.
> 
> > Source/WebKit/efl/ewk/ewk_view.h:2356
> > +EAPI void ewk_view_notification_closed(const Evas_Object* ewkView, const Ewk_Notification*);
> 
> ditto.
> 
> > Source/WebKit/efl/ewk/ewk_view.h:2366
> > +/* #endif */
> 
> Remove this comment.
> 
> > Tools/ChangeLog:6
> > +        This is a implementation about web notification for efl
> 
> %s/a/an/g

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.