Re: Feedback from Safari on Web Notifications

On Tue, Mar 6, 2012 at 4:48 PM, Maciej Stachowiak <mjs@apple.com> wrote:

>
> Hello Web Notifications WG,
>
> The Safari team is in the process of getting approval to officially join
> the Web Notifications WG, but it is taking a while. In the meantime, we've
> been exploring Web Notifications and had some thoughts about changes to the
> API. We've discussed these on webkit-dev to make sure other WebKit-using
> vendors are reasonably on board with this. Even though we haven't
> officially joined the WG yet, we'd like to pass along this feedback now for
> wider discussion and hopefully incorporation into the spec.
>
> Regards,
> Maciej
>

Thanks for the feedback, Maciej.  Looking forward to Safari's involvement
in the WG.  Responses inline below.

 -John


> =====
>
> We have the following proposal of changes to the spec:
>
> 1. Make the iconURL optional.
>
> Notification platforms may choose not to load the URL for privacy reasons,
> and we think it should not be a required component for a text notification.
>
> This means rearranging the constructor arguments to:
>
> new Notification(title, body, [optional] iconURL)
>
> And removing the steps from the various algorithms in Section 6 that fire
> an error event in the case that fetching iconURL causes an error.
>

These changes make sense to me; unless there is objection I'll make these
changes in the spec.


>
> 2. Use static functions on the notification constructor for permissions
> checks.
>
> Notification.permissionLevel() --> String
>
> The name aligns with similar functionality in the DAP Feature Permissions
> spec [FEAT], but scoped specifically to Web Notifications. The method
> 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.
>
> The Feature Permissions spec allows for 4 enums, two of which denote
> default behavior if the user hasn't explicitly determined a policy. We
> think 3 are only necessary, and any UA that wishes to grant permission by
> default should just return "granted".
>
> From a flow perspective, it makes sense for a UA to deny by default, for
> any web site to detect that it's denied, and then request the UA for
> permission. Assuming the UA's default policy is the opposite, the flow
> doesn't fit. That is, assume a UA allows all notifications by default, and
> the web site detects a default-granted policy. We don't think any website
> would then ask the user to deny itself the privilege of posting
> notifications. In this case, the UA should prompt the user for denial
> directly, and not assume that the web site will handle the default-granted
> case. Therefore, we think UA's that want to give permission by default
> should just return "granted".
>
> Notification.requestPermission(callback)
>
> The callback specified includes one argument which represents the
> permission, similar to the return values in Notification.permissionLevel().
> If the user had already made a policy decision, then the callback is
> invoked with the decision included as the parameter (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 will
> have onerror() called. Multiple calls to this function should just result
> in one visible request.
>
> Although there is already a way to query for the permission, it seems
> weird from a programming flow perspective that the callback would not
> contain information about the user's decision as an argument.
>

I think the biggest question that the group needs to address here is
whether the Notifications spec should continue depending on the Feature
Permissions for its permissions behavior (and perhaps forward these
proposals there to be applied in the general sense), or should we remove
that dependency and return to writing our own permissions behavior scoped
to notifications?


> 3. Replace setReplaceId() with a property id.
>
> It isn't clear in the spec that the reason for this property is to avoid
> posting duplicate notifications by multiple frames, so adding some
> description about that would be useful. Platforms may differ in how they
> handle notifications with the same ID, so we think it is appropriate to
> remove the "replace" prefix. It looks like retrieving the ID is missing,
> and would be useful for the web site, so we should make it an assignable
> attribute.
>

I can add more explanations for this to the spec, although this is not the
exclusive use case of replaceId -- there are also single-frame use cases
(keeping a chat notification updated with the latest message, for example).
 In those cases I think "id" alone suggests uniqueness that doesn't
actually exist, whereas replaceId suggests the real relationship.  The term
is already used by a notification platform [1] (as "replaces_id") for the
same behavior.

As far as dealing with platform differences, I think the goal of the spec
should be to abstract away those differences as much as possible so that
web authors can expect consistency.  In the replaceID case we can do this
by specifying that either the notification is replaced, if possible; or if
not, the old one closed and the new one opened, which is not exactly the
same but very close.  It would be much worse to specify that there is an ID
property and the underlying platform will do whatever it chooses with it.

[1] https://wiki.ubuntu.com/NotifyOSD

Also, is it clear that this id should be ignored if the string is null or
> empty?
>

I'll change the spec so that this is clear.


>
> 4. Improve show() behavior.
>
> We think show() can only be used once per notification. Subsequent
> invocations should call onerror().
>

Agreed that show() is only meant to be used once.  Ian Hickson has made the
related suggestion that show() should not exist as a method, but rather
that show() be implicit in the constructor.  I don't prefer that because it
requires that all possible optional parameters of the notification be
provided in the constructor, which makes the interface harder to use.  I
prefer using onerror to deal with multiple attempts to show, but this
should be resolved by the WG.


>
> 5. Rename cancel() to close().
>
> From a naming perspective, the asymmetry between cancel() and close()
> seems odd.
>

Did you mean to point out an asymmetry between cancel() and *show*() ?  The
reason "cancel" makes sense rather than "close" is that the behavior
depends on the state of the notification.  A notification which has been
shown will be closed, but a notification that is queued pending available
space on the display will be canceled before being shown.  IMHO "cancel"
covers both situations better than "close", but if the group prefers close
I can make that change to the spec.


> [FEAT] http://dev.w3.org/2009/dap/perms/FeaturePermissions.html
>
>