GNOME Bugzilla – Bug 657755
Try to be smarter when creating the fallback icon for web applications
Last modified: 2012-12-12 19:24:21 UTC
Right now we just take a snapshot at 0:0 of size 128x128. One thing we could is look for any div or whatever with id "logo", or for a "logo.png" or something like that, and take a snapshot centered on that. Probably much more likely to be meaningful than what we are doing now (although, I must say, what we do now works surprisingly well in a lot of cases).
You could also check for apple-touch-icon references in the markup and likely default locations (e.g. /apple-touch-icon.png). They're supported by quite a range of other browsers and devices, despite what the name suggests and are sometimes available at 114x114px. Summary: http://en.wikipedia.org/wiki/Favicon#Device_support Quirks: http://mathiasbynens.be/notes/touch-icons Example: http://facebook.com/apple-touch-icon.png
*** Bug 670899 has been marked as a duplicate of this bug. ***
Facebook: "og:image" (http://ogp.me/) Windows 8: <meta name="application-name" content="CNN.com International"/> <meta name="msapplication-TileColor" content="#CA0002"/> <meta name="msapplication-TileImage" content="http://i.cdn.turner.com/cnn/2012/images/10/15/cnn_logo_144_144.png"/> http://blogs.msdn.com/b/ie/archive/2012/06/08/high-quality-visuals-for-pinned-sites-in-windows-8.aspx Maybe we should use the following priority: 1. image: msapplication-TileImage + msapplication-TileColor name: application-name 2. image: og:image name: og:site_name 3. image: favicon + background chosen from icon or site if icon is tranparent name: page title or derived from canonical link if title is too long 4. image: apple-touch-icon.png name: page title or derived from canonical link if title is too long
Another one similar in priority to og:image I think: <meta itemprop="image" content="/images/google_favicon_128.png">
Created attachment 231206 [details] [review] Use better application names for web apps
Created attachment 231207 [details] [review] Use better icons for webapps
Created attachment 231212 [details] [review] Use better icons for webapps
Review of attachment 231206 [details] [review]: Looks reasonable but you need to #ifdef this for WK1 only, WK2 does not have DOM bindings. ::: src/window-commands.c @@ +562,3 @@ + if (strcmp (host, "www.facebook.com") == 0) { + title = g_strdup ("Facebook"); + } else if (g_str_has_prefix (host, "www.")) If we are going to hardcode Facebook we might as well have a simple struct before this method with: "www.facebook.com" -> "Facebook" and use that. Then we can properly extend it with other common pages with shitty titles (GMail?).
Review of attachment 231206 [details] [review]: The metadata search is great, but I am not so keen on hardcoding special values for a few apps; at least not in the code itself. Perhaps we should ship a list of those as application data or fetch them from an online service, etc. Anything but hardcoding it! :) ::: src/window-commands.c @@ +543,3 @@ + property = webkit_dom_element_get_attribute (WEBKIT_DOM_ELEMENT (node), "property"); + if (g_strcmp0 (name, "application-name") == 0 + Picky Xan woud say that || needs to go up? @@ +550,3 @@ + g_free (name); + } + char *property; All this is awesome ... @@ +561,3 @@ + g_free (title); + if (strcmp (host, "www.facebook.com") == 0) { + if (g_strcmp0 (name, "application-name") == 0 ..but this not so much.
Created attachment 231268 [details] [review] Use better application names for web apps
Created attachment 231269 [details] [review] Use better icons for webapps
Created attachment 231347 [details] [review] Use better application names for web apps
Review of attachment 231347 [details] [review]: The opening braces are all wrong (they go in the next line). If you fix that and the static thing you can commit. ::: src/window-commands.c @@ +537,3 @@ + { "plus.google.com", "Google+" }, + { "youtube.com", "YouTube" }, + }; I think 'static' in a function does not really do what we want here? Should be outside of the function (otherwise it means the variable will survive across calls to the function, which makes no sense for this). Also, why no www. in the others? @@ +540,3 @@ + int i; + + g_assert (host != NULL); No asserts please!
Review of attachment 231269 [details] [review]: This is OK, but I think most of the code should go in lib/ephy-web-app-utils.[ch]. Mind doing the quick copy&paste? Otherwise we are bloating window-commands.[ch] for no reason. ::: src/window-commands.c @@ +514,3 @@ g_object_unref (snapshot); + gtk_image_set_from_pixbuf (GTK_IMAGE (data->image), framed); + g_object_unref (snapshot); I think you meant 'framed' there. @@ +634,3 @@ + char *ret; + + base = soup_uri_new (webkit_web_view_get_uri (WEBKIT_WEB_VIEW (data->view))); webkit_web_view_get_uri can return NULL surprisingly often, check for that.
Created attachment 231406 [details] [review] Use better icons for webapps
Review of attachment 231406 [details] [review]: Looks good. ::: lib/ephy-web-app-utils.c @@ +231,3 @@ +ephy_web_view_get_best_icon (WebKitWebView *view, + char **uri, + GdkRGBA *rgba) get_best_icon woo!
Attachment 231347 [details] pushed as aa5172e - Use better application names for web apps Attachment 231406 [details] pushed as 787d591 - Use better icons for webapps