796079 - language.current should map to general.useragent.locale instead of intl.accept_languages

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect, P1)

Description

[Axel Hecht [:Pike]]

Right now, the gecko settings observer syncs language.current to intl.accept_languages. That doesn't do the right thing, neither for requests sent to websites, nor for switching locale settings for gecko-based localized strings like neterror.

It should sync with general.useragent.locale instead.

I have a local build, but that does funky things on macos desktop, and I didn't extensively test the unpatched version, so I don't know who's at fault.

This is required for bootcamp to make the firefox OS browser work against the web.

Comment 1

[Axel Hecht [:Pike]]

Attachment: sync general.useragent.locale instead of accept-lang

I tried this on my osx desktop build, but that was all around funky. So funky that I'm not sure this patch had anything to do with it.

Kaze, can you have a look? I tested this with switching the language of the phone in the settings app, and you'd need to have both netError.dtd and appstrings.properties localized and registered on the gecko side. For now, I did that manually. And then I hit DNS errors in the browser app. That did the right thing, but I'm not sure if I totally regressed language switching in gaia, or if that wasn't working well to begin with.

Comment 2

[Axel Hecht [:Pike]]

PS: No idea how one would write tests for this, too.

Comment 3

[Andrew Overholt [:overholt]]

Assuming this blocks localization, marking as a blocker.

Comment 4

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 666946 [details] [diff] [review]
sync general.useragent.locale instead of accept-lang

Review of attachment 666946 [details] [diff] [review]:
-----------------------------------------------------------------

This will prevent web applications to access the user selected value from navigator.language. You may want to set up the 2 prefs when the setting is changed instead of only one.

Comment 5

[Axel Hecht [:Pike]]

You don't want to set accept-lang to a single value, that's breaking the web. Any multilingual site will get its language negotiation corrupted.

The way this should work in theory is that you set general.useragent.locale, and the navigator code picks that up through the generic value 
pref("intl.accept_languages",               "chrome://global/locale/intl.properties");
which then ends up that navigator.language is the first entry there.

This works as long as the first entry in intl.properties is actually the language we intend to select for the UI, but that should be true in general. And it is for the P1 languages we care about.

Comment 6

[Axel Hecht [:Pike]]

Unassigning as I'm on PTO.

Comment 7

[chris hofmann]

Vivien,  thoughts on axel's proposal in comment 5? could you work on a patch that does that while axel is on pto until the 29th, or do we need to wait?

Comment 8

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch

Pike let me know if that what you expect.

Comment 9

[Axel Hecht [:Pike]]

Comment on attachment 673312 [details] [diff] [review]
Patch

Review of attachment 673312 [details] [diff] [review]:
-----------------------------------------------------------------

We really shouldn't have to set accept_languages, the generic value should be fine. If that doesn't work, it'd be cached somewhere (which I didn't find).

PS: This will only start to work once we have the gecko localizations.

Maybe this:

Set the locale, get the selected locale for the 'global' provider, if that's the same language (region may differ, I think, for es vs. es-ES), reset the accept_languages pref, otherwise set it to the given language. That should hopefully be proof to mixed gecko/gaia locale setups, but it's not really how things should work, just how we can make gaia work on top of mis-localized geckos.

The right fix would be a create a new webapi to determine UI language independent of navigator.language, there are too many implications here, I'm afraid.

Or to put the accept-language headers into gaia itself, which, actually, might be a good thing to do for V2, too. It'd be nice if users could edit their languages preferences on the web in the device.

Comment 10

[Fabien Cazenave [:kaze]]

(In reply to Axel Hecht [:Pike] (back Oct 29) from comment #9)
> We really shouldn't have to set accept_languages, the generic value should
> be fine.

We must set `accept_languages' from Gaia, and this has to be reflected in `navigator.language': in Gaia there’s no difference between UI and content.

We could use the `language.current' setting to expose the `general.useragent.locale', in order to properly localize toolkit messages.

We could set a list of locales in a `language.accepted' setting and map it to the `accept_languages' pref — e.g. for `pt-BR' we’d set `accept_languages' to `pt-BR, pt, en-US, en', as we do already for Firefox [1].

And selecting a language in the Gaia Settings would set both values. Thoughts?

[1] http://hg.mozilla.org/l10n-central/pt-BR/file/533ebea92a48/toolkit/chrome/global/intl.properties#l33

Comment 11

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v2

This is fairly easy to set both prefs if needed. This will let Gaia set the UI locale as well as a list for accept_languages synced with the values in  intl.properties files.

This needs some Gaia changes as well that I would add to this bug if you agree with this patch.

Comment 12

[Axel Hecht [:Pike]]

(In reply to Fabien Cazenave [:kaze] from comment #10)
> (In reply to Axel Hecht [:Pike] (back Oct 29) from comment #9)
> > We really shouldn't have to set accept_languages, the generic value should
> > be fine.
> 
> We must set `accept_languages' from Gaia, and this has to be reflected in
> `navigator.language': in Gaia there’s no difference between UI and content.

You will pick up localized and thus correct values once we have a localized gecko underneath gaia.

> We could use the `language.current' setting to expose the
> `general.useragent.locale', in order to properly localize toolkit messages.
> 
> We could set a list of locales in a `language.accepted' setting and map it
> to the `accept_languages' pref — e.g. for `pt-BR' we’d set
> `accept_languages' to `pt-BR, pt, en-US, en', as we do already for Firefox
> [1].
> 
> And selecting a language in the Gaia Settings would set both values.
> Thoughts?
> 
> [1]
> http://hg.mozilla.org/l10n-central/pt-BR/file/533ebea92a48/toolkit/chrome/
> global/intl.properties#l33

We don't have any good UX for setting accept-lang headers, even on desktop. I don't think we should burden ourselves with trying to create one for gaia v1.

Comment 13

[Axel Hecht [:Pike]]

I've reconsidered the code path from comment 9, to work around the lack of gecko localization support for a gaia-locale:

set general.useragent.locale
check if accept-lang has a user-set value, reset if so
get the complex value for an nsIPrefLocalizedString for accept-lang
if the first chunk of that value doesn't match the set locale
  prefix the found value with the set value
  set the user string pref for accept-lang

This code should only be hit for development, and not be used on production devices, too.

Comment 14

[Axel Hecht [:Pike]]

Comment on attachment 673817 [details] [diff] [review]
Patch v2

Review of attachment 673817 [details] [diff] [review]:
-----------------------------------------------------------------

r- based on my previous comments.

Comment 15

[Fabien Cazenave [:kaze]]

(In reply to Axel Hecht [:Pike] (back Oct 29) from comment #12)
> (In reply to Fabien Cazenave [:kaze] from comment #10)
> > (In reply to Axel Hecht [:Pike] (back Oct 29) from comment #9)
> > > We really shouldn't have to set accept_languages, the generic value should
> > > be fine.
> > 
> > We must set `accept_languages' from Gaia, and this has to be reflected in
> > `navigator.language': in Gaia there’s no difference between UI and content.
> 
> You will pick up localized and thus correct values once we have a localized
> gecko underneath gaia.
> 

The result would be the same if we reuse the values defined in Gecko in the `settings.[locale].properties' files — and that’d be much easier for us.

> > We could use the `language.current' setting to expose the
> > `general.useragent.locale', in order to properly localize toolkit messages.
> > 
> > We could set a list of locales in a `language.accepted' setting and map it
> > to the `accept_languages' pref — e.g. for `pt-BR' we’d set
> > `accept_languages' to `pt-BR, pt, en-US, en', as we do already for Firefox
> > [1].
> > 
> > And selecting a language in the Gaia Settings would set both values.
> > Thoughts?
> > 
> > [1]
> > http://hg.mozilla.org/l10n-central/pt-BR/file/533ebea92a48/toolkit/chrome/
> > global/intl.properties#l33
> 
> We don't have any good UX for setting accept-lang headers, even on desktop.
> I don't think we should burden ourselves with trying to create one for gaia
> v1.

I’m not suggesting to design an accept-lang setting panel: I’m saying that if these `accept_languages' values are defined in the `settings.[locale].properties' files, selecting a locale will automatically apply the same `accept_languages' pref as Gecko would do, without bothering the user. And if advanced users want to tweak this `accept_languages' pref, it’ll still be possible to design a Gaia app for that.

Comment 16

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v3

Let's try to implement your idea.

Comment 17

[Axel Hecht [:Pike]]

Comment on attachment 674180 [details] [diff] [review]
Patch v3

Review of attachment 674180 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with a slightly more rigorous test, Asturian and Assamese are counter examples I can theoretically see.

::: b2g/chrome/content/settings.js
@@ +79,5 @@
> +    intl = Services.prefs.getComplexValue(prefName,
> +                                          Ci.nsIPrefLocalizedString).data;
> +  } catch(e) {}
> +
> +  if (intl.indexOf(value) != 0) {

I think we need to make this check more rigorous, right now, "ast" and "as" make this pass. Maybe a regexp that only allows non-ascii characters?

Comment 18

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Oups I didn't meant to close it.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f6293f153a6b

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/eae4cdc7f382