On Wed, Feb 8, 2012 at 4:58 PM, Jon Lee <jon...@apple.com> wrote: > Hi WebKit! > > I am interested in refactoring the API for web notifications, and would > like your feedback. We are working on getting permission to join the Web > Notifications working group, but thought that while doing that I could try > to get some general consensus from the WebKit community about these changes > prior to proposing them to the WG. >
Glad there is continued interest in Notifications! I definitely hope you join the WG and work these concerns out there as well. > The basic intents of the refactoring are to a) align the API with the > design of recent APIs in the WebApps WG and elsewhere, and b) to simplify > the API by removing unnecessary bits. With these changes I think we can put > the API in a good place for future enhancements. > > ==== > Here are the proposed changes: > > 1. Move the factory method to a constructor function. > > new WebKitNotification(title, body, [optional] iconURL) > > This is similar to the spec [NOTIF], except that the icon URL has been > moved to the end as an optional argument. Notification platforms may choose > not to load the URL for privacy reasons, and I think it should not be a > required component for a text notification. > Seems reasonable that icon should be optional. > 2. Remove HTML notifications. > > It has been removed from the spec, and we don't intend on ever supporting > HTML notifications. I brought this issue up before; is there an update on > this front from any other platforms? > It has indeed been removed from the spec that's moving forward. I'd probably argue it could continue as an experimental feature, but will leave this alone for now lest it distract from the other items. 3. Use static functions on the notification constructor for permissions > checks. > > By moving them there, it allows us to remove window.webkitNotifications > and the NotificationCenter class, cleaning up the API. > I think that would be consistent with moving from a factory model to a constructor model. The Feature-permissions effort was intended to create a common interface for handling permissions, but it may still be too far off and sticking with a scoped approach is more practical. The only concern is that it is an implementation change that only partially reaches what has been spec'd, which might leave some continuing confusion among users of Notifications as to the authoritative version of the interface. > 4. Adjust the permission functions. > > WebKitNotification.permissionLevel() --> String > > Similar to checkPermissions(), synchronous call. The name aligns with > similar functionality in the DAP Feature Permissions spec [FEAT], but > scoped specifically to Web Notifications. Returns one of three strings: > "granted", "denied", and "default". This follows the current best practice > from the WebApps WG where strings are being used instead of enums. > > WebKitNotification.requestPermission(callback) > > An asynchronous call, as it is currently. The callback specified includes > one argument which represents the permission, similar to the return values > in permissionLevel(). (Currently it is just a void callback.) If the user > had already made a policy decision, then the callback is invoked > immediately (with either "granted" or "denied"). Otherwise the UA may ask > the user for permission, and the callback is invoked when the user makes a > decision (cancelling the request does not invoke the callback). Any > notifications posted in the meantime would be queued until then. Multiple > calls to this function should just result in one visible request. > This matches the FeaturePermissions spec with the exception of the decision as a parameter, which is reasonable though redundant since the current permission level is already easily read. The queueing of notifications discussed later. > 5. Remove replaceId. > > This could already be done by canceling a previous notification and > posting a new one. The idea of replacing a notification with another one > without alerting the user is a feature that we don't intend on supporting. > Just so the use case is clear here, it is not equivalent to canceling and showing from the webapp's perspective, though it is to the browser. The reason for replaceId is when the same application (e.g., Gmail) is open in multiple browser windows. When a new message arrives, both tabs will discover this and lacking communication between them, attempt to show a notification. But if both notifications are tagged as corresponding to the same real-world event, they are seamlessly merged and the user only sees one. > 6. Improve show() behavior. > > Calling show() when the site has been denied permission should call > onerror(). If no decision was made, then the notification should remain > queued, and only until after the user has decided should onshow() or > onerror() be called. I also think show() can only be used once. Subsequent > invocations should call onerror(). > The permission-denied behavior you want is as spec'd (error event). The permission-unknown behavior you propose seems more difficult for authors, since at the point of calling show() it would be impossible to distinguish between being queued for space limitation and being queued waiting for permission. If you limit this to the case where requestPermission() has already been called, as opposed to any permissionLevel=unknown state, that might make more sense. We could even introduce a new constant for permissionLevel of "request-pending" which would make this easy to specify cleanly. > > 7. Remove ondisplay. > > The spec uses onshow instead. > > ==== > More minor implementation details: > > 1. Rename NotificationPresenter to NotificationClient. > > This is just to make it follow the nomenclature of all page clients. > > 2. Simplify Notification and merge NotificationContents into it. > > I am unsure what the purpose of moving the data out into a separate class > was, but it seems unnecessary. > > ==== > > If you got this far, thanks for reading. Any and all feedback welcome! > > Regards, > Jon > > [FEAT] http://dev.w3.org/2009/dap/perms/FeaturePermissions.html > [NOTIF] > http://dev.w3.org/2006/webapi/WebNotifications/publish/Notifications.html > > _______________________________________________ > webkit-dev mailing list > webkit-dev@lists.webkit.org > http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev > >
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev