whatwg / notifications

Notifications API Standard
https://notifications.spec.whatwg.org/
Other
134 stars 47 forks source link

Supporting multiple image definitions for a Notification #28

Open beverloo opened 9 years ago

beverloo commented 9 years ago

Currently Notifications support a single image to be defined, but on today's world of many different screen densities we may want to consider a syntax for multiple syntax.

My proposal is to adopt the Web Manifest's syntax for defining an icon size: http://w3c.github.io/manifest/#icons-member

This would look as follows:

registration.showNotification('My Notification', {
  body: 'Hello, world!',
  icon: '/legacy-icon.png',  // optional, only used as fall-back
  icons: [
    { src: '/icon-1x.png',
      type: 'image/png',
      density: 1 },
    { src: '/icon-2x.png',
      density: 2 },
    { src: 'icon.ico',
      sizes: '128x128 256x256' }
  ]
});

On a tangent, we may also want to consider some sort of "small icon", to be used when the platform displays such icons in a status bar. (E.g. my little flag man on Android -- we're going to show a generic Notification icon when shipping them.)

+@mounirlamouri

mounirlamouri commented 9 years ago

I think it's a great idea to re-use the icons syntax from the Manifest.

Though, I'm not sure what we should do wrt the "small icon". What would be the best way to describe this icon? Should that be an icon that represents the website but not the notification? or the type of notification?

beverloo commented 9 years ago

Many applications only use a single icon, usually their logo, as it's a quick indicator for the user that an event in context to that app has happened. From that point of view a site-specific icon could work.

Two notable examples of where offering more freedom would be good would be Gmail, which either displays a single e-mail icon or a stack of e-mail icons depending on the number of messages waiting, or Calendar, which displays a small icon with the current day in the center.

(Chrome won't be able to support this for at least another year, so it's a much lower priority than the current icon in multiple densities.)

mrdewitt commented 9 years ago

In chrome apps/extensions, we have created the concept of "appIconMask" which is an alpha-only icon that can be used to overlay a solid color. This enables things like the little flag man without too much work on the platform side, since the color information is thrown out anyway.

annevk commented 9 years ago

See http://lists.w3.org/Archives/Public/public-web-notification/2014Apr/0012.html though it seems that concern is addressed through density. @zcorpan needs to generalize the images code path so all of this can utilize it.

annevk commented 9 years ago

@foolip @richtr Media Session has this problem as well. We should come up with a solution that works for both and elsewhere. Since manifests are still somewhat at risk I'd prefer us reusing a pattern of something that's implemented, like <picture>.

foolip commented 9 years ago

Very interesting. Media Session will indeed need this, because you'll want to change the icon of the media playback notification. However, we also want to change the lockscreen background. Both of these are currently in MediaMetadata.artwork, but it would probably make sense to split them into icon and artwork or perhaps background.

It seems like https://github.com/whatwg/mediasession/issues/58 applies very much here as well. How do we expose the load of this kind of multi-image icon, and how do we make it possible to preload the icon using Fetch?

richtr commented 9 years ago

@annevk wrote:

I'd prefer us reusing a pattern of something that's implemented, like <picture>.

Do you mean letting icon also take multiple comma-separated image candidate strings in the same way srcset works? I honestly prefer that idea over using the manifest syntax or introducing icons.

mounirlamouri commented 8 years ago

Why is Manifest "at risk"? It is implemented by Edge, Firefox, Google and Opera and used in the wild.

mvano commented 8 years ago

A background image could also be used for Wear devices. Currently, they fall back to using the notification icon, but that is often not of sufficiently high resolution, and looks a little blurry.

mvano commented 8 years ago

I'd like to split off the discussion of small icons into issue #65.

mvano commented 8 years ago

I've been prototyping multiple image definitions in Chrome, and so far the Manifest syntax seems like a good match. The idl would look like this:

dictionary NotificationIcon {
    USVString src;
    DOMString type;
    DOMString sizes;
    double density;
};

dictionary NotificationOptions {
    sequence<NotificationIcon> icons = [];
};

interface Notification {
  readonly attribute sequence<NotificationIcon> icons;
};
beverloo commented 8 years ago

In order to not duplicate the existing icon properties have you considered using a typedef instead? It's very unlikely that we'll be able to deprecate icon anytime soon.

typedef (USVString or sequence<NotificationIcon>) NotificationIcon;

dictionary NotificationOptions {
    // ...
    NotificationIcon icon;
    // ...
};

The getter is interesting. It could just return whatever was passed in the dictionary (so either an USVString or the sequence), although it probably should be a FrozenArray?

Additionally, what's your thought on having a platform key? This is likely to be a bit controversial, but since most browsers defer to the platform notification systems which have wildly varying user interfaces, requirements and masking, it would provide developers with a much more convenient way to select an appropriate icon.

annevk commented 8 years ago

If we are going to go object-oriented, sizes should be a sequence. If we're not, we should follow the example set by srcset, imo. Though I guess srcset is not really applicable since we don't care that much about layout here, just CSS pixels?

annevk commented 8 years ago

What is a platform key?

beverloo commented 8 years ago

Operating system. windows, mac, android and so on. It won't be nice to specify, and we may want to shy away from having a normative list (MDN instead?), but it doesn't feel entirely inappropriate given that they are meant to be displayed in the platform's notification center.

annevk commented 8 years ago

Oh I see, I would prefer to start out with something basic for v1. <link rel=icon> doesn't even have that feature.

beverloo commented 8 years ago

I'm not against punting, but the difference with <link rel=icon> is that our icons will be displayed in UI outside the browser's control. It does make sense to have that discussion separately.

mvano commented 8 years ago

The concept of platform doesn't seem to fully address the problem. An OS has a theme and OS themes and OS versions do not have a 1:1 relationship. A theme can remain unchanged across multiple OS versions e.g. the Material theme on Android 5 and 6. On the other hand, one OS version may support multiple themes e.g. OSX Blue and Graphite. To further complicate things, a theme can evolve subtly across OS versions without being officially renamed.

Also, on Android, it is common for OEMs to apply their own themes (skins). And finally, in many OS’s the user can install additional themes or customize parts of it.

Defining this would be tricky, but I agree it would be helpful to have a reasonable solution. Perhaps something for a separate issue, @beverloo?

mvano commented 8 years ago

It seems that srcset is bundled with a set of additional features we don't need, but that might not matter.

The width descriptor isn't quite a match for icons is it? It seems better to explicitly define width and height, even though most platforms expect square icons. Windows does not in all cases expect square images though. Aren't they all in pixels?

mvano commented 8 years ago

I do prefer an object oriented approach. No big need to stick everything in a string for which we'd need to define parsing rules.

annevk commented 8 years ago

Can sizes just be a sequence of integers representing raw pixels then? As in, [width1, height1, width2, height2, ...]. Not including sizes would be equivalent to <link rel=icon sizes>'s any. A list with a odd-numbered length would be invalid and treated like any.

mvano commented 8 years ago

A sequence of (unsigned) integers would work, and be simple. A more detailed design could be to define a Size with width and height. Then I think we'd have something like this, summarizing the discussion so far:

dictionary NotificationIconSize {
  unsigned long width;
  unsigned long height;
};

dictionary NotificationIcon {
  USVString src;
  DOMString type;
  sequence<NotificationIconSize> sizes = [];
  double density;
};

typedef (USVString or sequence<NotificationIcon>) NotificationIcon;

dictionary NotificationOptions {
   NotificationIcon icon;
};

interface Notification {
  readonly attribute NotificationIcon icon;
};

Maybe we can typedef any into NotificationIconSize somehow?

ithinkihaveacat commented 8 years ago

Wild suggestion: could some variation on HTTP Client Hints be used to get platform- and context-specific notification icons? (That is, there's just one URL in the source, but if the client passes along hints, and server can be bothered honoring them, it can deliver different images?)

(Whatever happen I hope authors will be able to avoid the favicon situation, where to do things right, pages get stuffed with multiple <link> and <meta> tags sprinkled with apple-touch-icon, icon, mask-icon, shortcut icon, msapplication-TileImage attributes, etc.)

annevk commented 8 years ago

We could, I wonder if this is still convenient to use though. Why do you think size needs to be its own object? (Also, can Chrome these days easily return a JavaScript object from a getter?)

annevk commented 8 years ago

@ithinkihaveacat we could do that too. I guess it depends on where you favor the complexity. Client Hints is also not something adopted by other browsers than Chrome yet and just yesterday some issues were raised against the specification (based on Mozilla dev.platform discussion).

beverloo commented 8 years ago

@annevk FrozenArray is still blocked, but hopefully we'll be able to use it in Q2. Sequences are fine, but they'll be values and not references (i.e. equality comparison fails).

Let's reserve the platform/theme discussion for another issue for when we agree on the basic approach. I think it's something we'll want to address, but Michael raises very good points.

@ithinkihaveacat I'm mostly ambivalent, but err towards a client-side preference to reduce the amount of necessary server-side logic. An ideal implementation would have cached images as part of the install event, which raises a question of which Client-Hints values to send— notifications may be displayed outside of the UA's control.

annevk commented 8 years ago

@beverloo I was referring to the dictionary @mvano proposes to return from Notification instances. (And please don't introduce an attribute getter that keeps returning fresh instances anywhere. That's terrible. x.y === x.y should be true.)

beverloo commented 8 years ago

To be clear, I fully agree. We deliberately haven't yet shipped two getters (actions and vibrate) because of this.

mvano commented 8 years ago

Simplifying the sizes syntax seems like a good idea as the common case would be to specify simply no or one size. Only .ico files contain multiples.

dictionary NotificationIcon {
  USVString src;
  DOMString type = "";
  sequence<unsigned long> sizes = [];
  double density = 1;
};

typedef (USVString or sequence<NotificationIcon>) NotificationIcons;

dictionary NotificationOptions {
   NotificationIcons icon;
};

interface Notification {
  readonly attribute NotificationIcons icon;
};

So common usage might look like this:

var Notification = new Notification('Title', {
    body: 'Some content',
    icon: [
      {src: 'icon_any.png'},
      {src: 'icon_64.png', sizes: [64, 64]},
      {src: 'icon_128.png', sizes: [128, 128]}
    ],
  }
);

I have a question about the typedef. The input should use a sequence, and the getter a FrozenArray, right? Then I don't think we can use the typedef this way.

annevk commented 8 years ago

Yeah, you can't, unfortunately. I wonder if we can use [SameObject] for icon now it'll return an object sometimes.

johnmellor commented 8 years ago

How will you select which icon to use (assuming there is no exact match for density and/or size)? The manifest spec doesn't seem to define this. Perhaps it's ok for that to be UA-specific.

@annevk wrote:

Can sizes just be a sequence of integers representing raw pixels then? As in, [width1, height1, width2, height2, ...]

It seems odd to have a dictionary with src,type,sizes,density - exactly as in the manifest spec - and yet have the sizes attribute use different syntax. Additionally, I'd argue that the manifest's syntax is clearer - it's obvious what "16x16 64x64" means, but less obvious what [16,16,64,64] means (the latter could be x,y,w,h).

@beverloo wrote:

An ideal implementation would have cached images as part of the install event

It seems inefficient for the Service Worker to have to download all possible icons for offline use. Perhaps we should expose the preferred icon sizes/densities/types ahead of time (several permutations may be needed, e.g. a connected smartwatch might prefer a larger size).

@beverloo wrote:

Let's reserve the platform/theme discussion for another issue for when we agree on the basic approach.

Consider following https://github.com/w3c/manifest/issues/361.

mvano commented 8 years ago

How will you select which icon to use (assuming there is no exact match for density and/or size)? The manifest spec doesn't seem to define this. Perhaps it's ok for that to be UA-specific.

I think we can say something along the lines of: "the user agent should select the most suitable icon taking into account screen size and density, memory, and bandwidth" and the exact formula for that balance is an implementation detail. If a user agent makes it look unexpectedly poor, for example by always selecting the smallest image in order to optimize resource usage, that would seem like a quality of implementation issue. But, if the user has, for example, enabled a bandwidth optimizing feature, that might be the right thing to do.

mvano commented 8 years ago

Can sizes just be a sequence of integers representing raw pixels then? As in, [width1, height1, width2, height2, ...]

It seems odd to have a dictionary with src,type,sizes,density - exactly as in the manifest spec - and yet have the sizes attribute use different syntax. Additionally, I'd argue that the manifest's syntax is clearer - it's obvious what "16x16 64x64" means, but less obvious what [16,16,64,64] means (the latter could be x,y,w,h).

Specifying multiple sizes is really only for the benefit of .ico files. I expect that in the common case there will be just one. So a common icon definition might look like this in the various discussed formats.

  {src: 'icon.png', sizes: '128x128'}
  {src: 'icon.png', sizes: [{width: 128, height: 128}]}
  {src: 'icon.png', sizes: [128, 128]}

The integer array format seems easy to use, and it is simplest to specify and implement, for example, no need to specify how to parse the string.

domenic commented 8 years ago

Coming here from Chrome's intent-to-implement. I honestly like the original proposal the best...

Another note... many proposals still have the Notification interface returning a sequence<NotificationIcon>, but it should be FrozenArray<NotificationIcon>. (Also, I thought you couldn't have sequences of dictionaries? Meh, maybe you can.)

slightlyoff commented 8 years ago

I agree with @domenic on trying to unify the formats.

mvano commented 8 years ago

It lets older UAs still continue to work, instead of trying to convert the array to a string then do a HTTP request against the resulting garbage URL

It seems that any reasonable implementation would just avoid or abort the HTTP request for invalid URLs. But yeah, it would mean that older UAs end up displaying no icon, or a default fallback. Using separate attributes would avoid this, and the awkward union type for the return value of the getter.

And yes, the getters should all use FrozenArray.

mvano commented 8 years ago

I'm happy to have the sizes attribute be a string with the goal of alignment with other specs. It's not difficult to use from a web developer's perspective. @annevk you seemed like the one most in favor of using an array of ints, and you're the editor of this spec. What do you think?

mounirlamouri commented 8 years ago

/CC @marcoscaceres

FWIW, the Manifest spec used sizes: "1x1 2x2" format to be consistent with the <link> element. This said, I'm not sure how sizes: [128, 128] would be correct because that's only defining one size. I've always felt icky about this string format and assumed that we would have to change to something better but I've never heard developers complaining and any other format would be way too verbose (sizes: [{height: 1, width: 1}, {height: 2, with: 2}] or sizes: ["1x1", "2x2"]).

BTW, should we change the Manifest spec to make the icon format re-usable in Notification instead of have both spec define something that is, on purpose, compatible?

mvano commented 8 years ago

BTW, should we change the Manifest spec to make the icon format re-usable in Notification instead of have both spec define something that is, on purpose, compatible?

If we fully align the specs, reusing that bit would be really good.

annevk commented 8 years ago

@mounirlamouri you'd just do [128, 128, 512, 512] for multiple sizes, but as pointed out the assumed common case is a single size. Is there telemetry for the ico format? Maybe multiple sizes is unnecessary.

I don't think we can reuse the manifest specification directly since it defines a format, not an API.

mounirlamouri commented 8 years ago

I don't think it would be crazy for the Manifest spec to describe its format via IDL if that can help.

foolip commented 8 years ago

This is a long thread and I apologize for being lazy and not reading it all. The value of reusing the same syntax seems high to me. If one could define this as nested IDL dictionaries and sequences, would that then be acceptable to integrate into Notifications?

mvano commented 8 years ago

One thing to keep in mind is that the manifest spec recently changed by dropping icon density: https://github.com/w3c/manifest/issues/450

annevk commented 8 years ago

@foolip you can't use IDL to describe a JSON-based format, but we could match the syntax in theory. It's just not clear that the complexity is worth it. E.g., multiple sizes is only for the ico format (and SVG, I suppose, but folks keep arguing that vector graphics here can only be used for a single size, since pixels need to be arranged differently per icon size), is that often used?

foolip commented 8 years ago

Is the sizes syntax the only trouble here? Would simply using a string with the existing microsyntax ("72x72 96x96 128x128 256x256") not be acceptable? If it's not possible to copy-paste between both contexts, then that would be really unfortunate. If string manipulation is terrible, then one might consider a utility interface like URLSearchParams that's able to both parse and serliaze the microsyntax.

annevk commented 8 years ago

@foolip I wonder whether it's needed given that multiple sizes are only for "ico". The string syntax is also rather ugly and unneeded when you can express numbers and data structures in JSON / IDL. However, I guess I can live with it if everyone else is comfortable with it.

foolip commented 8 years ago

The constraint is that the JSON manifest format is already shipped, so anything other than a subset of that has the downside of almost-but-not-quite compatible syntax.

Even if multiple sizes are an edge case, a size on the form "48x48" (or equivalent) is still needed, right? Otherwise one is left with only src and type, which isn't the subset we need for Media Session at least :)

mounirlamouri commented 8 years ago

We could add size that would take only one size in addition of sizes but that sounds like a very low value and potentially confusing change.

foolip commented 8 years ago

I agree, that doesn't sound very useful. If we're going to have a microsyntax like "48x48", then we might as well match in all the details as well.

mvano commented 8 years ago

Even though multiple sizes might in practice only be for .ico, that is still nice to have. I don't see great gain in removing it, especially as manifest already has it.