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)
Firefox OS Graveyard
General
Tracking
(blocking-basecamp:+, firefox18 fixed, firefox19 fixed)
RESOLVED
FIXED
blocking-basecamp | + |
People
(Reporter: Pike, Assigned: vingtetun)
References
Details
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
Pike
:
review+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•12 years ago
|
||
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)
Reporter | ||
Comment 2•12 years ago
|
||
PS: No idea how one would write tests for this, too.
Comment 3•12 years ago
|
||
Assuming this blocks localization, marking as a blocker.
blocking-basecamp: ? → +
Assignee | ||
Comment 4•12 years ago
|
||
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-
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → l10n
Reporter | ||
Comment 5•12 years ago
|
||
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.
Updated•12 years ago
|
Priority: -- → P1
Comment 7•12 years ago
|
||
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?
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → 21
Assignee | ||
Comment 8•12 years ago
|
||
Pike let me know if that what you expect.
Attachment #673312 -
Flags: review?(l10n)
Reporter | ||
Comment 9•12 years ago
|
||
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-
Comment 10•12 years ago
|
||
(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
Assignee | ||
Comment 11•12 years ago
|
||
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)
Reporter | ||
Comment 12•12 years ago
|
||
(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.
Reporter | ||
Comment 13•12 years ago
|
||
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.
Reporter | ||
Comment 14•12 years ago
|
||
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-
Comment 15•12 years ago
|
||
(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.
Assignee | ||
Comment 16•12 years ago
|
||
Let's try to implement your idea.
Attachment #673817 -
Attachment is obsolete: true
Attachment #674180 -
Flags: review?(l10n)
Reporter | ||
Comment 17•12 years ago
|
||
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+
Assignee | ||
Updated•12 years ago
|
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 18•12 years ago
|
||
Oups I didn't meant to close it.
Assignee | ||
Updated•12 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: [checking-needed-aurora]
Comment 19•12 years ago
|
||
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Whiteboard: [checking-needed-aurora]
Comment 20•12 years ago
|
||
status-firefox18:
--- → fixed
status-firefox19:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•