Bug 1181602 - Display pin badge for Firefox Apps r=benfrancis #30894
Conversation
albertopq
force-pushed
the
1181602-icon-apps
branch
from
86f6906
to
c4eacd1
Compare
albertopq
force-pushed
the
1181602-icon-apps
branch
from
c4eacd1
to
8dad033
Compare
| @@ -2476,6 +2489,18 @@ | |||
| siteObj); | |||
| }; | |||
|
|
|||
| function _convertToWebManifestIcons(manifest) { | |||
There was a problem hiding this comment.
Should this method be part of IconsHelper rather than AppWindow?
albertopq
force-pushed
the
1181602-icon-apps
branch
from
8dad033
to
e8c57d7
Compare
| @@ -48,7 +48,16 @@ | |||
| iconTargetSize = iconTargetSize * window.devicePixelRatio; | |||
|
|
|||
| // First look for an icon in the manifest. | |||
| if (siteObj.webManifestUrl && siteObj.webManifest) { | |||
| if (siteObj.manifest) { | |||
There was a problem hiding this comment.
Shouldn't we prioritise the web manifest spec that is standardised over the open web app manifest that is proprietary?
First, check for icons in web manifest, if none, fallback to open web app manifest.
cc @benfrancis
There was a problem hiding this comment.
Yes, maybe. My concern going forward is that if we prioritise W3C manifests over Firefox manifests in general is that if both are provided there may be features supported by the Firefox manifest (like web activities) that aren't supported in the W3C manifest.
That isn't really a problem for icons though so I'd be happy to look for a web manifest first.
There was a problem hiding this comment.
Should I change the priority then?
There was a problem hiding this comment.
Given that I only know of one app which has both a Firefox App manifest and a W3C manifest at the moment, it isn't urgent, but what you did is fine, thank you :)
albertopq
force-pushed
the
1181602-icon-apps
branch
from
e8c57d7
to
a2b0cb3
Compare
albertopq
force-pushed
the
1181602-icon-apps
branch
from
a2b0cb3
to
3632424
Compare
Bug 1181602 - Display pin badge for Firefox Apps r=benfrancis
No description provided.