[Webkit-unassigned] [Bug 61140] [GTK] Add HTML5 Notifications support

[bugzilla-daemon at webkit.org]

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


Philippe Normand <pnormand at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #112683|review?                     |review-
               Flag|                            |




--- Comment #49 from Philippe Normand <pnormand at igalia.com>  2011-11-03 07:53:41 PST ---
(From update of attachment 112683)
View in context: https://bugs.webkit.org/attachment.cgi?id=112683&action=review

> Source/WebKit/gtk/WebCoreSupport/NotificationPresenterGtk.h:31
> +#if ENABLE(NOTIFICATIONS)

Can you move the #if before the includes as I suggested in the previous two reviews?

> Source/WebKit/gtk/webkit/webkitwebnotification.cpp:275
> +char* webkit_web_notification_to_string (WebKitWebNotification* notification)
> +{
> +    return webkit_web_notification_is_html(notification) ?
> +      g_strdup_printf("DESKTOP NOTIFICATION: contents at %s", webkit_web_notification_get_uri(notification)) :
> +      g_strdup_printf("DESKTOP NOTIFICATION:%s icon %s, title %s, text %s",
> +                      webkit_web_notification_get_direction(notification) == GTK_TEXT_DIR_RTL ? "(RTL)" : "",
> +                      webkit_web_notification_get_icon_uri(notification),
> +                      webkit_web_notification_get_title(notification),
> +                      webkit_web_notification_get_body(notification));
> +}

AFAIK this is used only in DRT and quite specific to the tests. Please move this code to DRT, no need to be a function either.

> Source/WebKit/gtk/webkit/webkitwebnotification.cpp:286
> +            || (coreNotification1 == coreNotification2)

Can you remove this? As asked in the previous 2 reviews....

>> Source/WebKit/gtk/webkit/webkitwebview.h:85
>> +{
> 
> This { should be at the end of the previous line  [whitespace/braces] [4]

Please fix this

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