Multiple icon definitions

[mvano]

I think the initial patch has the general shape of what I want to do for #28.

I did find there's two ways we could go with the attribute getters on a notification, for example for the icon. To me it's not clear whether we need a single icon getter that reflects the selected url, or both icon and icons which reflect the exact input from options.

Previously icon reflected a serialization of the options icon input that was parsed with baseURL, not the exact original input. That means, for example, that if parsing fails, the getter returns the empty string. The implications for NotificationIcon are not clear to me: would invalid input such as for type or sizes need to be dropped in the same way? That might surprise developers.

---

Preview | Diff

[annevk]

I know we have this indentation elsewhere, but if the <p> is not the sole child of an <li> it really should go on its own line and get indented. And this <ol> too.

[annevk]

I think it would be nicer if icon/badge simply reflected the selected URL.

[annevk]

(And we'd do away with icons/badges.)

[mvano]

Thanks Anne, I've cleaned it up a bit as discussed. Of note:

• Just return the selected URL, do not expose getters for the multiple inputs.
• As input and output are now more obviously asymmetrical (the missing getters on the output) I had to split NotificationAction into a NotificationActionOptions dictionary and a NotificationAction interface.
• Afaict using an interface with readonly attributes for the output has the happy side effect of not needing to explicitly freeze the action, so I deleted that wrinkle.

[annevk]

<p> needs to be on a new line.

[annevk]

This looks pretty good I think. Who else should we ask for review?

[mvano]

CC @beverloo @foolip @mounirlamouri @xxyzzzq in case there's something really crazy in here. If necessary we can probably align a detail or two as needed in a followup change.

[annevk]

I'll aim to land this Friday (maybe ping me during the day just in case) unless someone has major concerns.

[foolip]

There's definitely some similarity here with w3c/mediasession#129

Feedback on that PR appreciated, and is there a good place to put the common bits? Looks to me at first glace like the whole "list of images" concept that uses the [{src:'foo'}] syntax can and should be shared.

[foolip]

What's the conclusion of this? Looks like the issue was side-stepped by not exposing an icons member on NotificationAction, is that a permanent situation?

[mvano]

Only exposing the selected URL seems much easier to understand. I don't see a strong use case for reflecting all the exact input of the options dictionaries, so I don't expect us to add that. Note that we already didn't expose the exact input of the icon urls, we reflected the result of resolving with baseURL.

Also, by using an interface with readonly attributes, I don't think we need to freeze the object anymore, which was a strange wrinkle.

[foolip]

So is the resource chosen synchronously so that it can be immediately reflected through a single icon attribute? Doesn't that mean that the bits implementing the notification API need to know quite a bit about the UI of the platform its running on?

AFACIT, the thing that was frozen here wasn't the value returned by the attribute (which is readonly), but the NotificationAction objects in that array. Otherwise one can do things like notification.actions[0].icon = 'newicon.png' which would have no effect. Exact same problem as discussed for Media Session artwork, in other words.

[mvano]

Good question about synchronously reflecting the selected URL... that might require an async IPC message in Chrome. I'll have to think about that one.

The general solution to avoid having to freeze dictionaries seems to be to instead return interfaces with readonly attributes. NotificationAction now is an interface that only has readonly attributes, and assigning a new value to icon has no effect, intentionally. The property value does not change, and internally nothing changes. No exception is thrown.

[annevk]

Closing this due to lack of activity. If there's still interest let's discuss in #28.