Re: [w3c/manifest] Example suggests that `icons` `sizes` mean a minimum size, but normative text doesn't support that (#925)

@benfrancis :

> size was meant to be the exact size and not a minimum size (unless "any" is specified as the value)

But you do allow icons of size X to be used where <X is desired (by the above description) (I'll get to the actual logic of the code later, which appears to allow an icon of size X to be used either larger or smaller than X).

Quick (unrelated) drive-by code review on @benfrancis 's [algorithm](https://github.com/webianproject/shell/blob/master/chrome/shared/js/services/WebApp.js#L324):
```js
      // filter out badge and maskable icons and those without a src
      if(!icon.src ||
        icon.purpose.has('badge') ||
        icon.purpose.has('maskable')) {
        return;
      }
```

You might want to update `'badge'` to `'monochrome'` as we've changed that in the spec. But really the intent of this check seems to be "if an icon is being used for some other special purpose, ignore it". Really, then, the check should be "if the icon has an explicit purpose and does not include `"any"`". That has two benefits:

1. You will exclude unknown future purposes without having to update the code whenever one is added to the spec (e.g., you wouldn't have to update this code when we added `monochrome`). You're [required to do this](https://www.w3.org/TR/appmanifest/#dfn-processing-the-purpose-member-of-an-image) (e.g., ignore any icon with purpose `"fizzbuzz"`).
2. You will _not_ exclude icons that explicitly include "any" alongside another purpose. For instance, a purpose of `"any maskable"` should be treated exactly the same as an icon without a purpose, except that it can also be used as a maskable icon.

The second of these is critical: I've seen real-world sites that only have `"any maskable"` icons, which means they're happy to use their maskable-formatted icon as a regular icon as well. You should allow this.

Also:

```js
        var size = sizeString.split('x')[0];
```

I think you forgot `Number()`. This will store `size` and `bestSize` as a string, so they will compare lexicographically, not numerically (e.g., `"32" < "256"` is false).

> ("best available" in this case meaning the smallest icon that is larger than or equal to a requested size).

It doesn't look like that's exactly what this algorithm does. It seems to look for _either_ the smallest icon >= target size, _or_ the largest icon <= target size, biased by whichever it sees first of smaller or larger.

For instance, if your target size is 100, if your sizes are [2, 110, 105], it will choose 2 (once it sees 2, it will only ever choose icons smaller than the target), but if your sizes are [110, 2, 105], it will choose 105 (once it sees 110, it will only ever choose icons larger than the target). (That's just my interpretation from reading the code; I didn't actually run it.)

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/w3c/manifest/issues/925#issuecomment-662247479

Received on Wednesday, 22 July 2020 05:21:42 UTC