[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 #3 from Gyuyoung Kim <gyuyoung.kim at samsung.com>  2011-12-01 19:32:52 PST ---
(From update of attachment 117390)
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.