After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 657755 - Try to be smarter when creating the fallback icon for web applications
Try to be smarter when creating the fallback icon for web applications
Status: RESOLVED FIXED
Product: epiphany
Classification: Core
Component: Web Applications
git master
Other Linux
: Normal normal
: ---
Assigned To: Epiphany Maintainers
Epiphany Maintainers
: 670899 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2011-08-30 19:05 UTC by Xan Lopez
Modified: 2012-12-12 19:24 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Use better application names for web apps (2.31 KB, patch)
2012-12-10 20:07 UTC, William Jon McCann
reviewed Details | Review
Use better icons for webapps (11.77 KB, patch)
2012-12-10 20:07 UTC, William Jon McCann
none Details | Review
Use better icons for webapps (12.08 KB, patch)
2012-12-10 20:23 UTC, William Jon McCann
none Details | Review
Use better application names for web apps (3.00 KB, patch)
2012-12-11 15:15 UTC, William Jon McCann
none Details | Review
Use better icons for webapps (12.05 KB, patch)
2012-12-11 15:15 UTC, William Jon McCann
reviewed Details | Review
Use better application names for web apps (2.99 KB, patch)
2012-12-12 12:04 UTC, William Jon McCann
committed Details | Review
Use better icons for webapps (15.42 KB, patch)
2012-12-12 18:52 UTC, William Jon McCann
committed Details | Review

Description Xan Lopez 2011-08-30 19:05:38 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).
Comment 1 Craig Barnes 2012-01-30 07:43:09 UTC
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
Comment 2 Xan Lopez 2012-02-28 09:18:51 UTC
*** Bug 670899 has been marked as a duplicate of this bug. ***
Comment 3 William Jon McCann 2012-12-09 23:16:38 UTC
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
Comment 4 William Jon McCann 2012-12-10 16:36:46 UTC
Another one similar in priority to og:image I think:
<meta itemprop="image" content="/images/google_favicon_128.png">
Comment 5 William Jon McCann 2012-12-10 20:07:55 UTC
Created attachment 231206 [details] [review]
Use better application names for web apps
Comment 6 William Jon McCann 2012-12-10 20:07:58 UTC
Created attachment 231207 [details] [review]
Use better icons for webapps
Comment 7 William Jon McCann 2012-12-10 20:23:00 UTC
Created attachment 231212 [details] [review]
Use better icons for webapps
Comment 8 Xan Lopez 2012-12-11 09:38:34 UTC
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?).
Comment 9 Claudio Saavedra 2012-12-11 09:40:44 UTC
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.
Comment 10 William Jon McCann 2012-12-11 15:15:28 UTC
Created attachment 231268 [details] [review]
Use better application names for web apps
Comment 11 William Jon McCann 2012-12-11 15:15:32 UTC
Created attachment 231269 [details] [review]
Use better icons for webapps
Comment 12 William Jon McCann 2012-12-12 12:04:43 UTC
Created attachment 231347 [details] [review]
Use better application names for web apps
Comment 13 Xan Lopez 2012-12-12 17:12:23 UTC
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!
Comment 14 Xan Lopez 2012-12-12 17:17:34 UTC
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.
Comment 15 William Jon McCann 2012-12-12 18:52:00 UTC
Created attachment 231406 [details] [review]
Use better icons for webapps
Comment 16 Xan Lopez 2012-12-12 19:21:03 UTC
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!
Comment 17 William Jon McCann 2012-12-12 19:24:14 UTC
Attachment 231347 [details] pushed as aa5172e - Use better application names for web apps
Attachment 231406 [details] pushed as 787d591 - Use better icons for webapps