[Webkit-unassigned] [Bug 40004] [Qt] Support for loading notification icons

[bugzilla-daemon at webkit.org]

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





--- Comment #7 from Kenneth Rohde Christiansen <kenneth at webkit.org>  2010-06-05 12:27:56 PST ---
(From update of attachment 57803)
WebKit/qt/WebCoreSupport/NotificationIconLoader.cpp:44
 +  NotificationIconLoader::NotificationIconLoader(NotificationPresenterClientQt* presenter, ScriptExecutionContext* context, const KURL& url)
It this NotificationIconLoader supposed to be Qt only? Currently it seems that way as it received a *ClientQt. Should it be renamed to QtNotificationIconLoader or is it possible to make it take a NotificationPresenterClient instead?

WebKit/qt/WebCoreSupport/NotificationIconLoader.cpp:75
 +      m_data += QByteArray(data, lengthReceived);
Seems a bit Qt'ish so far :-)

WebKit/qt/WebCoreSupport/NotificationIconLoader.cpp:83
 +  void NotificationIconLoader::didFail(const ResourceError&)
Shouldn't you handle the error somehow?

 103     if (notification->iconURL().isEmpty())
 104         displayNotification(notification, QByteArray());
 105     else
 106         startLoadNotificationIcon(notification);

It would seem more logical to do this the other way around

if (has icon)
   load it


WebKit/qt/WebCoreSupport/NotificationPresenterClientQt.cpp:296
 +  void NotificationPresenterClientQt::removeReplacedNotificationFromLoadQueue(Notification* notification)
Why isnt it removed when being replaced?

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