[Webkit-unassigned] [Bug 61140] [GTK] Add HTML5 Notifications support
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Sep 23 06:12:43 PDT 2011
https://bugs.webkit.org/show_bug.cgi?id=61140
Philippe Normand <pnormand at igalia.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #104354|review? |review-
Flag| |
--- Comment #42 from Philippe Normand <pnormand at igalia.com> 2011-09-23 06:12:43 PST ---
(From update of attachment 104354)
View in context: https://bugs.webkit.org/attachment.cgi?id=104354&action=review
> Source/WebKit/gtk/WebCoreSupport/ChromeClientGtk.cpp:777
> +NotificationPresenter* ChromeClient::notificationPresenter() const
If this method creates a Presenter it should be named something like createNotificationPresenter() and probably pass its reference to the caller, I think, like createPopupMenu() does for instance.
> Source/WebKit/gtk/WebCoreSupport/NotificationPresenterGtk.h:31
> +#if ENABLE(NOTIFICATIONS)
This could just go before the includes
> Source/WebKit/gtk/webkit/webkitwebnotification.cpp:79
> + g_error ("%s", error->message);
> + g_error_free (error);
No space before ( please :)
> Source/WebKit/gtk/webkit/webkitwebnotification.cpp:88
> + * notification. Internally, it propagate an "error" event in the DOM
s/propagate/propagates
> Source/WebKit/gtk/webkit/webkitwebnotification.cpp:103
> + * notification. Internally, it propagate a "display" event in the DOM
Ditto
> Source/WebKit/gtk/webkit/webkitwebnotification.cpp:118
> + * been clicked. Internally, it propagate a "click" event in the DOM
Ditto
> Source/WebKit/gtk/webkit/webkitwebnotification.cpp:177
> + static HashMap<KURL, GdkPixbuf* > pixbuf_cache;
> +
> + KURL icon_uri = core(WEBKIT_DOM_NOTIFICATION(self))->iconURL();
These variables are unused it seems.
> Source/WebKit/gtk/webkit/webkitwebnotification.cpp:254
> + * Returns: whether @self replaces (as defined in the Notifications specs)
Replace what?
> Source/WebKit/gtk/webkit/webkitwebnotification.cpp:255
> + * other.
other... notification I guess!
> Source/WebKit/gtk/webkit/webkitwebnotification.cpp:290
> + return ((notification == notification2)
> + || (coreNotification1 == coreNotification2)
Hum is it really necessary to compare both the WebKitWebNotifications and Notifications? I feel like one comparison would be enough here.
> Source/WebKit/gtk/webkit/webkitwebnotificationprivate.h:4
> + * webkit-web-notification.h - Header for WebKitWebNotification
> + *
> + * Copyright (C) 2010 Igalia S.L.
header name and copyright year are wrong
> Source/WebKit/gtk/webkit/webkitwebview.cpp:2769
> + * For each notification, one of WebKitNotification:displayed or
> + * WebKitNotification:failed *MUST* be called at actual display-time so
Missing : ?
> Source/WebKit/gtk/webkit/webkitwebview.cpp:2795
> + * remove it from the display; if it has not yet be displayed,
s/be/been
> Source/WebKit/gtk/webkit/webkitwebview.cpp:2796
> + * the user agent must prevent its being displayed.
"must prevent its being displayed" is odd, do you mean "must prevent its display" ?
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list