Closed Bug 1181185 Opened 9 years ago Closed 9 years ago

Site icon doesn't use nice homecreen icons

Categories

(Firefox OS Graveyard :: Gaia::System::Browser Chrome, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:2.5+)

RESOLVED DUPLICATE of bug 1184210
blocking-b2g 2.5+

People

(Reporter: mikehenrty, Assigned: apastor)

References

Details

(Whiteboard: [systemsfe])

Attachments

(1 file)

Example, Facebook has a nice homescreen icon which I think we should try to use for the site icon. But the site icon is a tiny favicon which doesn't look very good.

Also, when you pin maps.google.com you get a nice circular icon. But the site icon is a tiny 'G' favicon.

We should try to use the best icon available.
blocking-b2g: --- → 2.5?
blocking-b2g: 2.5? → 2.5+
Michael, how are you "pinning" maps.google.com? We didn't implement that feature yet :)

Can you provide STR so we can see the icons you're talking about and compare?

Thanks
Flags: needinfo?(mhenretty)
BTW, the Facebook icon in your screenshot looks black, which is not the same icon I'm seeing (which is blue).

I suspect the black icon is an SVG mask that Facebook are experimenting with due to Apple's recent "pinned sites" feature which breaks the web :/ https://lists.w3.org/Archives/Public/public-whatwg-archive/2015Jun/0011.html

They should hopefully go away soon as Apple agreed to use a new <link rel="mask-icon"> link relation instead.

The fact that we add a circle background does suggest that we read an SVG icon as having a 0 width and height, which is possibly something we can improve.
(In reply to Ben Francis [:benfrancis] from comment #2)
> Michael, how are you "pinning" maps.google.com? We didn't implement that
> feature yet :)

Sorry, I should say "bookmark". I bookmarked maps.google.com, and see a really nice icon (probably the iOS apple-touch meta tag one) on the homescreen. But when I open it, I get a crappy tiny 'g' in the URL bar. To me, we should always use apple touch icon over the favicon when available.

> Can you provide STR so we can see the icons you're talking about and compare?

Sorry, I think I forgot to mention an important point here! I am using the Facebook installed app, not merely the website. When we have these nice big icons in the FxOS manifest, I think we should use those in the site pin box. Facebook is one example, but also for Gaia apps which now currently only display the "pin" icon itself. Since I don't think we will be able to pin gaia apps, we should just use the manifest one here as well.
Flags: needinfo?(mhenretty)
(In reply to Michael Henretty [:mhenretty] from comment #4) 
> Sorry, I should say "bookmark". I bookmarked maps.google.com, and see a
> really nice icon (probably the iOS apple-touch meta tag one) on the
> homescreen. But when I open it, I get a crappy tiny 'g' in the URL bar. To
> me, we should always use apple touch icon over the favicon when available.

That's strange, I'm always seeing the "crappy tiny 'g'". I can see a <link href="//maps.gstatic.com/favicon4.ico" rel="shortcut icon"> in the source of the page, but we seem to be falling back to http://www.google.com/favicon.ico

That is a bug. Are we maybe struggling with a URL starting with "//"?

The inconsistent behaviour may be to do with the fact that we're not using the same IconsHelper everywhere yet.

> Sorry, I think I forgot to mention an important point here! I am using the
> Facebook installed app, not merely the website. When we have these nice big
> icons in the FxOS manifest, I think we should use those in the site pin box.
> Facebook is one example, but also for Gaia apps which now currently only
> display the "pin" icon itself. Since I don't think we will be able to pin
> gaia apps, we should just use the manifest one here as well.

Using icons from the proprietary Firefox App manifest is bug 1181602, this is my bad because I missed it during code review, Alberto has volunteered to fix that. Gaia apps should be considered to be already pinned, Firefox Apps should be pinnable (though the exact details of that are still to be figured out, see the thread on dev-b2g). One problem is that they lack the link relation that W3C manifests have so the manifest isn't discoverable by the user agent.
> Facebook has a nice homescreen icon which I think we should try to use for the site icon. But the site icon is a tiny favicon which doesn't look very good.

Having debugged this it looks like touch.facebook.com serves a <link rel="apple-touch-icon-precomposed"> (we ignore the precomposed type), and an Apple style <link rel=icon mask> SVG icon, but no standard <link rel=icon>. Sometimes we show the SVG icon (which is black, because Apple suck), and sometimes we fall back to /favicon.ico (which is tiny) because the login screen at m.facebook.com omits the SVG icon.

When Alberto's patch from bug 1181602 lands we will use the high resolution icon from the Firefox App manifest, but this will only be used if you install the app from the marketplace using the current install flow. We have no way of detecting this when "pinning".

Looks like in the case of Facebook it's going to be an evangelism thing, maybe we can get them to serve a web manifest...
> Also, when you pin maps.google.com you get a nice circular icon. But the site icon is a tiny 'G' favicon.

I can't quite figure out what's going on with this one. When I step through the code in the debugger I can get it to use the a 32x32 Google Maps icon, but in real time it falls back to google.com/favicon.ico. Maybe there's some kind of timing issue.

Sam, could it be that the <link rel=icon> is getting added into the DOM after loadend and we're not listening for iconchange events in browser chrome so it never gets used?
Flags: needinfo?(sfoster)
There are probably 2 issues here. I noticed that we were getting back an icon object with isSmall: true, when actually the image was 32x32 and should not be treated as small. Tracing it back, the 'size' property originates from a measurement in IconsHelper.fetchIcon, where we assign 

   img.src = URL.createObjectURL(iconBlob).

As iconBlob is a .ico url, we just get the first/smallest image in that ico bundle which is 16px. We can fix that by passing in the targetIconSize and: 

   img.src = URL.createObjectURL(iconBlob) + '#-moz-resolution=' + iconTargetSize + ',' + iconTargetSize;;

Note: you'll need to clear the icons cache when testing this, e.g. on the webIDE console in the system app: 

   navigator.getDataStores('icons').then((stores) => { stores[0].clear() })
 
The second issue is that when we get a mozbrowserlocationchange, we reset/invalidate all our favicons. When you load a google maps page, you can see we get 3 mozbrowserlocationchange events, but the detail shows the URL has not actually changed. So we lose the nice https://maps.gstatic.com/favicon4.ico we collected initially. I'm not sure why we get these events, but we should probably check it represents an actual change before we throw away our favicons. I dont know what that heuristic is, I was kinda hoping the platform would be doing that for us!

Finally, you're right we're not handling iconchange at all in AppChrome. I'm still not sure if we need to, maybe revisit when we get the above fixed
Flags: needinfo?(sfoster) → needinfo?(bfrancis)
Alberto, bug 1184210 says that the Clock app doesn't show the Clock icon. I guess we thought that would be fixed by bug 1181602, any idea what's going on?
Flags: needinfo?(bfrancis) → needinfo?(apastor)
I just commented on the bug. It seems like a race condition, as sometimes is showing the right icon. I'll take look.
Flags: needinfo?(apastor)
Hi Alberto,
Do you have any update?
Thanks!
Flags: needinfo?(apastor)
I took a look (and resolved) bug 1184210, which I though was a duplicated of this. Michael, is this still happening? Is Facebook still an example? Thanks!
Assignee: nobody → apastor
Flags: needinfo?(apastor) → needinfo?(mhenretty)
Yup, looks like a dupe. Don't see it happening anymore.
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: needinfo?(mhenretty)
Resolution: --- → DUPLICATE
(In reply to Ben Francis [:benfrancis] (PTO until 1st October) from comment #5)
> That's strange, I'm always seeing the "**** tiny 'g'". I can see a <link
> href="//maps.gstatic.com/favicon4.ico" rel="shortcut icon"> in the source of
> the page, but we seem to be falling back to http://www.google.com/favicon.ico
> 
> That is a bug. Are we maybe struggling with a URL starting with "//"?

I've opened Bug 1203136 to follow up on that.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: