Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Supporting multiple image definitions for a Notification #28

Open
beverloo opened this issue Dec 14, 2014 · 62 comments
Open

Supporting multiple image definitions for a Notification #28

beverloo opened this issue Dec 14, 2014 · 62 comments

Comments

@beverloo
Copy link
Member

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
Copy link
Member

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
Copy link
Member Author

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
Copy link

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
Copy link
Member

annevk commented Dec 16, 2014

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
Copy link
Member

annevk commented Jul 30, 2015

@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
Copy link
Member

foolip commented Jul 30, 2015

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 w3c/mediasession#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
Copy link
Member

richtr commented Jul 30, 2015

@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
Copy link
Member

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

@mvano
Copy link
Contributor

mvano commented Jan 18, 2016

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
Copy link
Contributor

mvano commented Mar 10, 2016

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

@mvano
Copy link
Contributor

mvano commented Mar 10, 2016

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
Copy link
Member Author

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
Copy link
Member

annevk commented Mar 10, 2016

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
Copy link
Member

annevk commented Mar 10, 2016

What is a platform key?

@beverloo
Copy link
Member Author

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
Copy link
Member

annevk commented Mar 10, 2016

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

@beverloo
Copy link
Member Author

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
Copy link
Contributor

mvano commented Mar 10, 2016

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
Copy link
Contributor

mvano commented Mar 10, 2016

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
Copy link
Contributor

mvano commented Mar 10, 2016

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
Copy link
Member

annevk commented Mar 11, 2016

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
Copy link
Contributor

mvano commented Mar 11, 2016

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
Copy link

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
Copy link
Member

annevk commented Mar 11, 2016

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
Copy link
Member

annevk commented Mar 11, 2016

@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
Copy link
Member Author

@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.

@mvano
Copy link
Contributor

mvano commented Mar 18, 2016

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
Copy link
Member

/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
Copy link
Contributor

mvano commented Mar 18, 2016

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
Copy link
Member

annevk commented Mar 18, 2016

@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
Copy link
Member

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

@foolip
Copy link
Member

foolip commented Apr 28, 2016

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
Copy link
Contributor

mvano commented Apr 28, 2016

One thing to keep in mind is that the manifest spec recently changed by dropping icon density: w3c/manifest#450

@annevk
Copy link
Member

annevk commented Apr 29, 2016

@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
Copy link
Member

foolip commented Apr 29, 2016

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
Copy link
Member

annevk commented Apr 29, 2016

@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
Copy link
Member

foolip commented Apr 29, 2016

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
Copy link
Member

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
Copy link
Member

foolip commented Apr 29, 2016

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
Copy link
Contributor

mvano commented Apr 29, 2016

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.

@mounirlamouri
Copy link
Member

FTR, it's not Manifest specific, it's coming from <link rel=icon>. Manifest folks are plagiarists like that 😱

@annevk
Copy link
Member

annevk commented Jun 9, 2016

Here is an alternative idea. Since it seems likely many more APIs going forward will want to show some kind of icon (we have two entry points for notifications here and one for media sessions per @foolip), perhaps we can come up with an API to determine the appropriate icon before feeding it to the final API.

That way we don't have to continue duplicating the API surface for each API that might need this.

@mvano
Copy link
Contributor

mvano commented Jun 9, 2016

The simplest thing would be to define an Icon interface and / or dictionary with src, type, and sizes in some lower level spec like HTML. What do you think?

@domenic
Copy link
Member

domenic commented Jun 9, 2016

I think @annevk's idea was something like

const chosenSrc = window.chooseImage([
    { src: '/icon-1x.png',
      type: 'image/png',
      density: 1 },
    { src: '/icon-2x.png',
      density: 2 },
    { src: 'icon.ico',
      sizes: '128x128 256x256' }
  ]);

which would return one of /icon-1x.png, /icon-2x.png, or icon.ico. No need for new classes like Icon. Then specs like this one would just take a string, instead of an array of objects.

@annevk
Copy link
Member

annevk commented Jun 9, 2016

Implementations might support different sizes for notifications and media session artwork and such, so it might require some kind of destination input as well. Not sure if that's a size we expose as a static somewhere or something else.

@mounirlamouri
Copy link
Member

I think this is missing two major points:

  • the image has a target size and the target size might not be known by the developer. For example, a Notification might have a small and large image, the size of the image will depend on the device, it's OS and the version of the OS.
  • the image might be used in different devices which means that window wouldn't be the right place.

@mvano
Copy link
Contributor

mvano commented Jun 9, 2016

I really think exposing some kind of Icon interface / dictionary is the most urgent bit for reducing duplication.

As for the url selection algorithm, there are a few issues:

  • The Manifest and Media Session specs don't have one and don't seem to want one.
  • Url selection may be asynchronous so it's not possible to expose the selected url on anything that is returned synchronously.
  • As Mounir pointed out, the implementation may want to use more than one of the provided icon sources in different rendering contexts.

So I'm tempted to drop this from the Notifications spec as well, and really the only thing to deduplicate is the Icon.

@annevk
Copy link
Member

annevk commented Jun 9, 2016

@mvano I think that's a reasonable alternative. An Icon object that you can feed to various places.

@coox
Copy link

coox commented Nov 3, 2016

Profane question from an enthusiastic visitor: am I right to assume that the proposal would also cover the badge use case?

If so, would it be preferred to open a separate issue tracking just multiple badge definitions?

@annevk
Copy link
Member

annevk commented Nov 4, 2016

I think we would apply it to all image fields. No need for a separate issue I think unless implementers want to tackle them one-by-one over time.

@beverloo
Copy link
Member Author

beverloo commented Nov 4, 2016

+1. From an implementation point of view it makes very little different whether we do it for one attribute or multiple.

@beverloo
Copy link
Member Author

beverloo commented Aug 9, 2018

The Web App Manifest now exposes an ImageResource type, which would be great for this purpose. We're currently moving Background Fetch to use it, and I'd be happy to enable this for the Notification API too :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests