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 767123 - Include User-Agent component for installed web apps runtime
Include User-Agent component for installed web apps runtime
Status: RESOLVED FIXED
Product: epiphany
Classification: Core
Component: Web Applications
3.21.x
Other Linux
: Normal enhancement
: ---
Assigned To: Epiphany Maintainers
Epiphany Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-06-01 18:19 UTC by Daniel Aleksandersen
Modified: 2016-06-09 12:08 UTC
See Also:
GNOME target: ---
GNOME version: 3.19/3.20


Attachments
Suggested patch based on clear instructions. (1.71 KB, patch)
2016-06-03 03:03 UTC, Daniel Aleksandersen
none Details | Review
Patch, take three (1.75 KB, patch)
2016-06-03 22:25 UTC, Daniel Aleksandersen
committed Details | Review
embed-prefs: Tweak the web app user agent (934 bytes, patch)
2016-06-09 12:08 UTC, Michael Catanzaro
committed Details | Review

Description Daniel Aleksandersen 2016-06-01 18:19:48 UTC
Please consider including an extra component to Epiphany’s User-Agent when Epiphany is used as an installable web application runtime.

The motivation for including an extra component is letting websites owners get a little feedback about the popularity of their Epiphany web app. This can in turn help them help Epiphany by promoting installable web apps to their users. It also allows web apps to provide a more targeted experience to web app users (hide cookie disclaimers, slightly different stylesheet, and the like). 


Suggested User-Agent for installed apps:
Mozilla/5.0 […] Epiphany/3.20.0 (WebApp)

Normal User-Agent for reference:
Mozilla/5.0 […] Epiphany/3.20.0
Comment 1 Michael Catanzaro 2016-06-01 19:43:45 UTC
This would be a good first task for someone who wants to start contributing to Epiphany. The place to start would be webkit_pref_callback_user_agent() in embed/ephy-embed-prefs.c. To decide if we're in application mode, use ephy_embed_shell_get_default() to get the singleton instance of the EphyEmbedShell class, then ephy_embed_shell_get_mode().

We have to decide whether to append (WebApp) to the user agent even if the user has overridden the user agent with the gsetting. I think either way would be reasonable.
Comment 2 Daniel Aleksandersen 2016-06-03 03:03:32 UTC
Created attachment 329014 [details] [review]
Suggested patch based on clear instructions.
Comment 3 Michael Catanzaro 2016-06-03 14:07:32 UTC
Review of attachment 329014 [details] [review]:

Thanks. A couple string handling mistakes here, though:

::: embed/ephy-embed-prefs.c
@@ +204,3 @@
+  if (ephy_embed_shell_get_mode (ephy_embed_shell_get_default ()) ==
+      EPHY_EMBED_SHELL_MODE_APPLICATION)
+    user_agent = g_strdup_printf ("%s (WebappShell)", user_agent);

You're leaking the previous value of user_agent here; got to free it before assigning to it again. Unfortunately you'll need to add yet another variable to use for the left hand side of this assignment. :/

@@ +208,3 @@
+  webkit_settings_set_user_agent (webkit_settings, user_agent);
+  g_free (user_agent);
+  g_free (internal_user_agent);

You don't want to free internal_user_agent here. Notice that webkit_pref_get_internal_user_agent returns a const char *, indicating that you're not supposed to free it. You probably got away with it because webkit_pref_get_internal_user_agent is probably only called once, but the function is written to return the original result if called multiple times, so now it's going to return freed memory on subsequent uses.
Comment 4 Daniel Aleksandersen 2016-06-03 22:25:38 UTC
Created attachment 329100 [details] [review]
Patch, take three

Everything should be in order now. Sorry for having submitted 3:00 am-code. I should have waited till morning. :)
Comment 5 Michael Catanzaro 2016-06-03 23:41:53 UTC
OK, looks good. I pushed a follow-up to simplify the code a bit more.
Comment 6 Daniel Aleksandersen 2016-06-08 19:43:58 UTC
I forgot to mention this here in the bug: the final user-agent for web apps looks like this:

Mozilla/5.0 […] Epiphany/3.21.2 (WebappShell)
Comment 7 Michael Catanzaro 2016-06-08 20:33:44 UTC
I've been thinking of changing it to (WebAppShell) or (WebApp) or (Web App) as I don't really like webapp as one word.
Comment 8 Daniel Aleksandersen 2016-06-09 11:10:00 UTC
I have no good arguments for or against any particular component name.  I didn’t capitalize the A because I came across some other examples of “Webapp” being used in User-Agents as one word. Similar solutions have used “(WAC)”, referring to a Web Application Container specification that seem to have died out. It doesn’t really matter as long as it stays consistent over time and doesn’t change. Choose something you’re happy with now rather than keep one you dislike and decide to change it later.
Comment 9 Michael Catanzaro 2016-06-09 12:08:46 UTC
The following fix has been pushed:
8b18787 embed-prefs: Tweak the web app user agent
Comment 10 Michael Catanzaro 2016-06-09 12:08:49 UTC
Created attachment 329453 [details] [review]
embed-prefs: Tweak the web app user agent