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 528893 - epiphany-webkit don't respect language settings for pages
epiphany-webkit don't respect language settings for pages
Status: RESOLVED FIXED
Product: epiphany
Classification: Core
Component: Backend
git master
Other Linux
: Normal major
: ---
Assigned To: Xan Lopez
Epiphany Maintainers
Depends on:
Blocks:
 
 
Reported: 2008-04-19 11:49 UTC by Luca Ferretti
Modified: 2009-12-18 18:51 UTC
See Also:
GNOME target: ---
GNOME version: 2.23/2.24


Attachments
Properly manage the list of languages for the "Accept-Language" header (3.50 KB, patch)
2009-12-16 17:24 UTC, Mario Sánchez Prada
none Details | Review
Properly manage the list of languages for the "Accept-Language" header (3.46 KB, patch)
2009-12-18 12:09 UTC, Mario Sánchez Prada
none Details | Review

Description Luca Ferretti 2008-04-19 11:49:55 UTC
Epiphany with WebKit backend don't load the web page using the selected language.

## Steps to Reproduce ##
 * Go to Edit -> Preferences -> Language -> Languages
 * Add to language list, for example, "es"
 * Go to http://www.debian.org/intro/cn or http://l10n.gnome.org

## Current Results ##
Both suggested pages are showed using English.

## Expected Results
Suggested pages should be showed using Spanish language

## Additional Notes ##
It seems there is nothing related to this on http://bugs.webkit.org/
Comment 1 Javier Jardón (IRC: jjardon) 2009-08-12 22:25:40 UTC
You reported this bug a while ago and there hasn't been any activity in it recently. We were wondering if this is still an issue for you.

Can you please check again if the issue you reported here still happens in a recent version and update this report by adding a comment and adjusting the 'Version' field?

Again thank you for reporting this and sorry that it could not be fixed for the version you originally used here.

Without feedback this report will be closed as INCOMPLETE after 6 weeks.
Comment 2 Mario Sánchez Prada 2009-09-30 15:57:15 UTC
Right one week after the 6-week limit, I'm now confirming this bug is still present in last webkit (1.1.15) + epiphany (2.29.0) code from git repositories.

Looks to me like a bug in WebKit but can't assure that atm. I'll take a deeper look at some point if I find time for it, but here you have this tiny contribution in the meanwhile.
Comment 3 Mario Sánchez Prada 2009-10-02 06:33:57 UTC
Filed a bug in libsoup which could help with this issue:

Bug 597004: Provide API to set a list of languages for the accept-language header
Comment 4 Reinout van Schouwen 2009-12-16 15:13:46 UTC
Confirming.
Comment 5 Mario Sánchez Prada 2009-12-16 17:24:17 UTC
Created attachment 149857 [details] [review]
Properly manage the list of languages for the "Accept-Language" header

This patch adds a new handler to watch for changes in the GConf key with the list of languages for the "Accept-Language" HTTP header, and update the (incoming) SoupSession's "accept-language" property.

Such an "accept-language" property for SoupSession is something still pending to be accepted (see bug 528893), in the best case in libsoup 2.29.4, so that's why I changed the configure.ac also to that value.
Comment 6 Diego Escalante Urrelo (not reading bugmail) 2009-12-17 01:39:42 UTC
Review of attachment 149857 [details] [review]:

Thanks Mario! I added some minor comments.
Convince Xan while he's "a tiro de piedra" to give an ok for this ;)

::: embed/ephy-embed-prefs.c
@@ +212,3 @@
+  char *webkit_pref;
+
+  webkit_pref = data;

Don't you need a (char *) here?

@@ +226,3 @@
+      const char *lang = gconf_value_get_string ((GConfValue *) l->data);
+
+      if ((lang != NULL) && g_str_equal (lang, "system")) {

Why not g_strcmp0?

@@ +237,3 @@
+
+  langs = (char **) g_array_free (array, FALSE);
+  langs_str = g_strjoinv (", ", langs);

You miss the sentinel there, the NULL after langs.

@@ +241,3 @@
+  /* Update Soup session */
+  session = webkit_get_default_session ();
+  langs_str = g_strjoinv (", ", langs);

Same as above
Comment 7 Mario Sánchez Prada 2009-12-18 12:09:28 UTC
Created attachment 149985 [details] [review]
Properly manage the list of languages for the "Accept-Language" header

(In reply to comment #6)
> Review of attachment 149857 [details] [review]:
> 
> Thanks Mario! I added some minor comments.
> Convince Xan while he's "a tiro de piedra" to give an ok for this ;)
> 
> ::: embed/ephy-embed-prefs.c
> @@ +212,3 @@
> +  char *webkit_pref;
> +
> +  webkit_pref = data;
> 
> Don't you need a (char *) here?

In this case is not needed, as it's an implicit conversion coming from a gpointer.

> @@ +226,3 @@
> +      const char *lang = gconf_value_get_string ((GConfValue *) l->data);
> +
> +      if ((lang != NULL) && g_str_equal (lang, "system")) {
> 
> Why not g_strcmp0?

Not big reason here :-), it's just that the code came from gecko version that way. But I'd also prefer g_strcmp0 there. I'll change it in the patch I'm attaching right now.

> @@ +237,3 @@
> +
> +  langs = (char **) g_array_free (array, FALSE);
> +  langs_str = g_strjoinv (", ", langs);
> 
> You miss the sentinel there, the NULL after langs.

Actually not, as the GArray was created so the internal array was NULL terminated. :-)

> @@ +241,3 @@
> +  /* Update Soup session */
> +  session = webkit_get_default_session ();
> +  langs_str = g_strjoinv (", ", langs);
> 
> Same as above

Same answer than above :-), although with this you made me realize of an even worse problem: a leak because of calling to 'langs_str = g_strjoinv (", ", langs)' twice with no good reason for that, making it so the second call leaks the memory allocated with the first one. I'm including that change in my new patch as well.

Thanks for your comments!
Comment 8 Xan Lopez 2009-12-18 18:51:50 UTC
Fixed in master, thank you!