Closed Bug 796079 Opened 12 years ago Closed 12 years ago

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

Categories

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

defect

Tracking

(blocking-basecamp:+, firefox18 fixed, firefox19 fixed)

RESOLVED FIXED
blocking-basecamp +
Tracking Status
firefox18 --- fixed
firefox19 --- fixed

People

(Reporter: Pike, Assigned: vingtetun)

References

Details

Attachments

(1 file, 3 obsolete files)

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.
Blocks: 796424
Blocks: 768939
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.
Attachment #666946 - Flags: review?(kaze)
PS: No idea how one would write tests for this, too.
Assuming this blocks localization, marking as a blocker.
blocking-basecamp: ? → +
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.
Attachment #666946 - Flags: review?(kaze) → review-
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.
Priority: -- → P1
Unassigning as I'm on PTO.
Assignee: l10n → nobody
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?
Attached patch Patch (obsolete) (deleted) — Splinter Review
Pike let me know if that what you expect.
Attachment #673312 - Flags: review?(l10n)
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.
Attachment #673312 - Flags: review?(l10n) → review-
(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
Attached patch Patch v2 (obsolete) (deleted) — Splinter Review
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.
Attachment #666946 - Attachment is obsolete: true
Attachment #673312 - Attachment is obsolete: true
Attachment #673817 - Flags: review?(l10n)
(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.
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 on attachment 673817 [details] [diff] [review] Patch v2 Review of attachment 673817 [details] [diff] [review]: ----------------------------------------------------------------- r- based on my previous comments.
Attachment #673817 - Flags: review?(l10n) → review-
(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.
Attached patch Patch v3 (deleted) — Splinter Review
Let's try to implement your idea.
Attachment #673817 - Attachment is obsolete: true
Attachment #674180 - Flags: review?(l10n)
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?
Attachment #674180 - Flags: review?(l10n) → review+
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Oups I didn't meant to close it.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: [checking-needed-aurora]
Status: REOPENED → RESOLVED
Closed: 12 years ago12 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Whiteboard: [checking-needed-aurora]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: