[Webkit-unassigned] [Bug 40005] [Qt] Platform plugin support for Notifications UI

[bugzilla-daemon at webkit.org]

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





--- Comment #9 from Kenneth Rohde Christiansen <kenneth at webkit.org>  2010-06-11 18:20:52 PST ---
(In reply to comment #8)
> (In reply to comment #6)
> Thank yoy for taking the time to review.
> > WebKit/qt/Api/qwebkitplatformplugin.h:61
> >  +  class QWebNotificationUi : public QObject
> > I dont like the Ui part of the name. Isn't this more or less like a notification controller? or notification dispatcher?
> > 
> I'll change it to QWebNotificationPresenter
> 
> > WebKit/qt/Api/qwebkitplatformplugin.h:68
> >  +      virtual void showNotification() = 0;
> > If this class is a notification itself, this should probably just be called show() or appear()
> I will change it to "show".

Actually since you are renaming to QWebNotificationPresenter, showNotification makes sense :-) so no need to rename.

> 
> > 
> > WebKit/qt/Api/qwebkitplatformplugin.h:81
> >  +          Notifications
> > Maybe we should extend this with SingleSelections as well?
> > 
> Maks sense to me, but perhaps a separate patch, and also with a build flag, so one can get implement notifications but not the SingleSelect if they want to ?

Yes, not in this patch.

> 
> > WebKit/qt/Api/qwebkitplatformplugin.h:72
> >  +  };
> > What string do I receive??
> This was supposed to be for HTML notifications, if a link was clicked. This is not yet supported, so I will leave it out.

Sounds good.

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