1041482 - [meta] Larger icon sizes for web content

Status: RESOLVED FIXED

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

Description

[Kevin Grandon :kgrandon]

There have been several bugs filed about this, here is yet another to drive the effort for Rocketbar.

Comment 1

[Kevin Grandon :kgrandon]

It appears there have been several patches/attempts in the past. I'm going to reference everything here for tracking/decision making.

See also:
Bug 897900 - HD: bookmark icon in homescreen is not sharp
Bug 921005 - [Browser] Enable to choose icon with proper image size by link tag
Bug 921014 - [BrowserAPI] Support link rel="apple-touch-icon" in mozbrowsericonchange event
Dev-gaia discussion - https://groups.google.com/forum/#!topic/mozilla.dev.gaia/Bi2-ufWd75A

Comment 2

[Dale Harvey (:daleharvey)]

I think it would be best to treat this as a meta bug (unassigned) and assign yourself to the sub tasks, there is at least one more bug under this and they wont need to all be one bug commit.

Sam, I remember you mentioning a bug to pick the correct size out of a favicon package, can you this bug with that?

Comment 3

[Dale Harvey (:daleharvey)]

Forgot to needinfo sam

Comment 4

[Dale Harvey (:daleharvey)]

Added https://bugzilla.mozilla.org/show_bug.cgi?id=968156 which was the first step to using higher quality icons for places results

So one issue is how to harmonise the icon code between bookmarks and places, as the ability to pick the best icon is going to change them both and I would like to have them both use similiar code

Right now the places implementation listens to mozbrowsericonchange events, it stores the resulting uri in the places database, the search application then uses this uri to fetch the icon and display it (it doesnt currently cache, but could / should)

I assume the bookmark code when invoked from the browser or the collections app will send along the icon as part of the activity, can that be updated? from collections we rely on the icon from e.me that part is fine. 

I think change here is that the browser (+system browser) should keep a list of the iconchange events, then we can have a shared function between places / browser / system browser that takes a list of the mozbrowsericonchange objects, a requested size and returns the most appropriate icon.

Some details to note, some applications use dynamic icons, that at least means we need to cull the list of icons we store in places, it may change the behaviour of how we pick the correct icon, and another note is that we may not have to care about the browser since thats going away, but if it helps to implement it then its there to be fixed

Comment 5

[Sam Foster [:sfoster] (he/him)]

https://bugzilla.mozilla.org/show_bug.cgi?id=962630 has some discussion on the state of things as they stand. Also for reference, see: https://bugzilla.mozilla.org/show_bug.cgi?id=828508, especially the lines in the patch: 

  let size = Math.round(16 * window.devicePixelRatio);
             sizedIconUrl += (sizedIconUrl.indexOf("#") > -1 ? "&" : "#") +
                             "-moz-resolution=" + size + "," + size;

..which use the fragment identifier to explicitly request a preferred size should the be a .ico bundle with multiple images.

I think there is another bug out there for storing icons in their own table in the places db where appropriate. All of which becomes quite a lot of boilerplate for every app that needs to show icons, so if we can get a sharable implementation into shared for at least some of the steps to fetch, select, decorate and display an icon that would be great.

Comment 6

[Francisco Jordano [:arcturus] [:francisco]]

Glad to come to the same results that Dale and Sam are pointing out (just after finishing my investigation came and see your comments).

As Dale says, we are not honoring the list of provided icons and we just keep the last one, we can change that to store an array of those icons:

https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/app_window.js#L891

Also realised what Sam's comment, we don't get all the icon sizes, an example of that can be found while visiting http://www.wordpress.com

Last thing that I tried was the gecko patch provided in bug 921014, which definitely improves a lot the icon fetching, but unfortunately it's a philosophical question.

Happy to make this a meta :), and unassigning myself.

Comment 7

[Francisco Jordano [:arcturus] [:francisco]]

Attachment: 2014-07-22-12-00-43.png

I'm attaching a screenshot of homescreen with 3 icons that we got using the touch icons from apple.

There you can see 'linkedin', 'amazon webservices', and a chinese nes portal ifeng.com with really nice icons

Comment 10

[Dale Harvey (:daleharvey)]

Dependancies have all landed :)