Closed Bug 1045053 Opened 10 years ago Closed 10 years ago

Determine default Accept-Language header from user elections, not displayed locale

Categories

(Firefox for Android Graveyard :: Locale switching and selection, defect)

All
Android
defect
Not set
normal

Tracking

(firefox35 fixed, firefox36 fixed, fennec+)

VERIFIED FIXED
Firefox 36
Tracking Status
firefox35 --- fixed
firefox36 --- fixed
fennec + ---

People

(Reporter: marek, Assigned: rnewman)

References

Details

Attachments

(4 files, 2 obsolete files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/36.0.1985.125 Safari/537.36

Steps to reproduce:

We have tested our website localization with Firefox for Android on devices with various mostly asian languages. Accept-language header was wrong for some of them.

Steps to reproduce:

1) Switch Android device into one of the following languages: Bahasa Indonesia, Filipino, Vietnamese, Arabic, Tamil, Hindi.
2) Either visit some website (http://request.urih.com/) which shows you your request headers or use developer tools to view them.
3) Check "accept-language" header.



Actual results:

"accept-language" header is "en-US  (en-US,en;q=0.5)"


Expected results:

"accept-language" header should ...

... for Bahasa Indonesia start with "id-ID"
... for Filipino start with either "tl-PH" or "fil-PH"
... for Vietnamese start with "vi-VN"
... for Arabic start with "ar-something"
... for Tamil start with "ta-something"
... for Hindi start with "hin-something"

We have tested also English, Bahasa Malaysia, Chinese Simplified, Chinese Traditional, Japanese, Thai and Korean. Accept-language header was correct for these languages.

This is really important problem because it's not possible to smoothly deliver localized website to the users speaking languages mentioned above.
tracking-fennec: --- → ?
We don't ship Arabic, Filipino (we don't support this in the Mozilla ecosystem at all), Vietnamese on Fennec (multi-locale build).

id: id, en-us, en
http://hg.mozilla.org/releases/l10n/mozilla-release/id/file/default/toolkit/chrome/global/intl.properties#l33

ta: intl.accept_languages=ta-IN, ta
http://hg.mozilla.org/releases/l10n/mozilla-release/ta/file/default/toolkit/chrome/global/intl.properties#l33

hi-IN: hi-in, hi, en-us, en
http://hg.mozilla.org/releases/l10n/mozilla-release/hi-IN/file/default/toolkit/chrome/global/intl.properties#l33

Can you give some details on what you tested exactly?
Marek, are there documents on where you got your expected behavior from?
"accept-language" header is used by all the standart CMS and frameworks to deliver localized website (user can later switch languages manually). There is no reason to return accept language "en" for indonesian speaking users. These language codes are ISO639 standart.

Desktop Firefox works correctly - I have tested the same set of languages. 

Here you can see Android Firefox VS Android default browser "accept-language" headers: https://dl.dropboxusercontent.com/u/4434838/langs.jpg As you can see default browser also works as expected.
We have tried to switch device into those languages and checked "accept-language" headers from Firefox request if it's correct for those languages. We tested this because of some bug we have found earlier in Android Chrome browser.

I understand that you don't support Arabic, Filipino and Vietnamese as interface language atc. but why don't you deliver that language in "accept-language" header for Arabic, Filipino and Vietnamese speeking users? Desktop Firefox delivers right headers for these languages.

For Indonesia, Tamil and Hindi we really got "accept-language"="en".

I also forgot to mention that we have tested this on Ninetology U9 Z1 Android 4.2.1 from Malaysian distribution. 

(In reply to Francesco Lodolo [:flod] from comment #1)
> We don't ship Arabic, Filipino (we don't support this in the Mozilla
> ecosystem at all), Vietnamese on Fennec (multi-locale build).
> 
> id: id, en-us, en
> http://hg.mozilla.org/releases/l10n/mozilla-release/id/file/default/toolkit/
> chrome/global/intl.properties#l33
> 
> ta: intl.accept_languages=ta-IN, ta
> http://hg.mozilla.org/releases/l10n/mozilla-release/ta/file/default/toolkit/
> chrome/global/intl.properties#l33
> 
> hi-IN: hi-in, hi, en-us, en
> http://hg.mozilla.org/releases/l10n/mozilla-release/hi-IN/file/default/
> toolkit/chrome/global/intl.properties#l33
> 
> Can you give some details on what you tested exactly?

(In reply to Francesco Lodolo [:flod] from comment #1)
> We don't ship Arabic, Filipino (we don't support this in the Mozilla
> ecosystem at all), Vietnamese on Fennec (multi-locale build).
> 
> id: id, en-us, en
> http://hg.mozilla.org/releases/l10n/mozilla-release/id/file/default/toolkit/
> chrome/global/intl.properties#l33
> 
> ta: intl.accept_languages=ta-IN, ta
> http://hg.mozilla.org/releases/l10n/mozilla-release/ta/file/default/toolkit/
> chrome/global/intl.properties#l33
> 
> hi-IN: hi-in, hi, en-us, en
> http://hg.mozilla.org/releases/l10n/mozilla-release/hi-IN/file/default/
> toolkit/chrome/global/intl.properties#l33
> 
> Can you give some details on what you tested exactly?
hi-IN and ta were added only recently (31), id was added before (30), that makes me think you didn't test the current release.

If you switch Android to "Vietnamese", the browser you get is in English, and so is the accept language it sends.

What you're asking is to change the behavior and send the system locale before the default values. 

I'm not sure that makes sense, especially considering that we're trying to decouple Firefox's locale from system's locale. In your case, a "Welsh" browser on an English Android (either as a single locale build, or multi-locale build with language switcher) would send out "en-US, cy, etc.".
Thank you for info. We will test hi and id once again. But I am quite sure that the version was the last one - 31.

You know on desktop Firefox you can choose  (settings -> content -> languages) language you prefere websites to be displayed in. But there is no such settings in Android Firefox.

And it really make sense to me that for example:
- I speak Vietnamese
- so I switch Android device to Vietnamese language
- Firefox doesn't support Vietnamese as interface language so it's displayed in default - English
- but still I prefere for example Google to be displayed in my Vietnamese language so I need Firefox to send my device locale as "accept-language" header

Your example is different. Because there is no language settings in Firefox for Android. Firefox for Android respects Android language as every other app/browser does. So the only case when "accept-language" header would be different than Firefox language is a case of languages not supported by Firefox as user interface language which is the example above (Vietnemese).

But I may be wrong. I just expect websites to be displayed in my language however that language is not supported (as interface language) by browser I use.


(In reply to Francesco Lodolo [:flod] from comment #5)
> hi-IN and ta were added only recently (31), id was added before (30), that
> makes me think you didn't test the current release.
> 
> If you switch Android to "Vietnamese", the browser you get is in English,
> and so is the accept language it sends.
> 
> What you're asking is to change the behavior and send the system locale
> before the default values. 
> 
> I'm not sure that makes sense, especially considering that we're trying to
> decouple Firefox's locale from system's locale. In your case, a "Welsh"
> browser on an English Android (either as a single locale build, or
> multi-locale build with language switcher) would send out "en-US, cy, etc.".
So, there's no intent to be compatible with stock browser on what we serve as accept-languages.

We do intend to offer an explicit choice for accept-lang to users, that's bug 881510.

I can see how adding the Android UI language to the accept-lang headers might be useful, iff the accept-lang doesn't have a user-set value, and Fennec's UI language isn't user-set.

That's not exactly trivial to do, though. We'd need to set a gecko pref, but not set a pref. No idea if we can, Richard?
Flags: needinfo?(rnewman)
Adding to comment 7

> Your example is different. Because there is no language settings in Firefox
> for Android. Firefox for Android respects Android language as every other
> app/browser does. So the only case when "accept-language" header would be
> different than Firefox language is a case of languages not supported by
> Firefox as user interface language which is the example above (Vietnemese).

There will be. See bug 917480 and 
https://blog.mozilla.org/l10n/2014/05/20/language-switching-in-fennec/

But unfortunately it still means no Vietnamese, because localization is not ready (ever thought of contributing to the localization? ;-))
Unfurtunately I don't speak any of languages mentioned above. I just found this problem so I reported it as a bug here. But it seems to be expected behaviour for you so you can probably close this issue.


(In reply to Francesco Lodolo [:flod] from comment #8)
> Adding to comment 7
> 
> > Your example is different. Because there is no language settings in Firefox
> > for Android. Firefox for Android respects Android language as every other
> > app/browser does. So the only case when "accept-language" header would be
> > different than Firefox language is a case of languages not supported by
> > Firefox as user interface language which is the example above (Vietnemese).
> 
> There will be. See bug 917480 and 
> https://blog.mozilla.org/l10n/2014/05/20/language-switching-in-fennec/
> 
> But unfortunately it still means no Vietnamese, because localization is not
> ready (ever thought of contributing to the localization? ;-))
> I can see how adding the Android UI language to the accept-lang headers
> might be useful, iff the accept-lang doesn't have a user-set value, and
> Fennec's UI language isn't user-set.

That behavior is actually what I expect Gecko to be doing now: the default Accept-Language header is built from general.useragent.locale.

But it's likely only built once.

Marek: did you relaunch the browser (kill the process to be sure, or use the Quit Now add-on) after switching locales? I suspect that Gecko doesn't regenerate Accept-Language on the fly, and that would affect your results.
Flags: needinfo?(rnewman) → needinfo?(marek)
OS: Mac OS X → Android
Hardware: x86 → All
Marek: I guess there's another question, too: are you switching the *device* language or the *app* language (using Settings > Language)?

My guess is that you're doing the former. Relaunching Firefox might still give you different results.
I think he's setting device language.

Say, you're using Android in Filipino, you get en-US Fennec, en-US, en accept-lang. We could make that tl-PH, tl, en-US, en or so.
If you're using Android in Filipino, choose to use 'fr' Fennec, then we should not touch accept-lang.

Once we're offering accept-lang UI, and you make any explicit choice, we'd use just that. If you reset, we'd go back to the above.
> Say, you're using Android in Filipino, you get en-US Fennec, en-US, en
> accept-lang. We could make that tl-PH, tl, en-US, en or so.

So I think we could summarize this as "use user-selected locale for default Accept-Language, not (only) displayed locale", right?


I'd posit that there are three ways to select a locale, in decreasing order of strength:

* Single-locale build. If you download and install a Maithili build, your default Accept-Language should be 'mai'.
* Multi-locale build with a user-selected locale. If you picked French, you get 'fr', regardless of OS.
* Multi-locale build using the system locale. If we can't display it, you get both the selected locale and the fallback that we're displaying.

Does that sound right to you, Axel?
That sounds right to me.
This could well end up in Necko, but let's leave it here for now.
Blocks: 935025
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(marek)
Summary: Incorrect accept-language header for some languages → Determine default Accept-Language header from user elections, not displayed locale
Version: Firefox 31 → Trunk
Kinda yes, though the gecko locale is actually a list of locales.

I kinda tried to wrap my head around how to do this, and I think the best way would be to have an additional pref of "additional_accept_lang" and have gecko prepend that, and maintain that pref from the Android side.

I've come up with a bunch of other ways, but all of them ended up being funky at the moment where some user launches about:config.
Richard - Can you do some digging and summarize how we can move forward?
Assignee: nobody → rnewman
tracking-fennec: ? → +
Right now we pull the default Accept-Language header from intl.properties:

general.useragent.locale=en-US
intl.accept_languages=en-US, en

Which intl.properties file we use is determined by the selected locale.

The issue here is that "selected locale" doesn't mean "the one the user picked", or "the one the OS is currently using": it's "the one Gecko is using".

So the solution doesn't involve intl.properties. We need to expose to Gecko two values -- the OS and app locales -- and complicate the header creation process to include them.


That could be via a pref, as Axel suggests. It could also be via a JNI 'pull'.


A complicating factor: similar to Bug 1018240, we need to re-init the computed header value. nsHttpHandler.cpp currently does this via a pref observer for intl.accept_languages; that would need to be extended if we used more than one pref.


Now, given that we want to address Bug 881510 very soon, and we can access default preferences, my proposed solution is slightly simpler:

* Pass the OS and app locales to JS when we know them.
* Get the default intl.accept_languages from nsIPrefService.
* Write a new user-set intl.accept_languages that combines the default and the environment values.
* Do this whenever necessary (e.g., locale changes, OS locale switch).

This will break any users who try to customize intl.accept_languages themselves, but (a) so would gluing prefs together, (b) we'd probably break them in Bug 881510 regardless.

We could work around that by watching intl.accept_languages ourselves; if someone other than us changes it, switch modes.

Opinions?
Status: NEW → ASSIGNED
Flags: needinfo?(l10n)
Flags: needinfo?(jbeatty)
I think this looks like a good proposal. Just for a sanity check, line item three in your proposal would give priority preference to the default, user selected value from the language picker, correct?
Flags: needinfo?(jbeatty)
Scenario: Ernie and Bert have an unsupported locale selected, and now

- Ernie doesn't set own language choice
- Bert does (via bug 881510)

Both Ernie and Bert change to another unsupported locale on the device.

What happens? Can we tell Ernie and Bert apart? Do we need to?
Flags: needinfo?(l10n)
(In reply to Jeff Beatty [:gueroJeff] from comment #19)
> I think this looks like a good proposal. Just for a sanity check, line item
> three in your proposal would give priority preference to the default, user
> selected value from the language picker, correct?

Yes. If your OS was in English, you selected to use Firefox in French, and hypothetically were forced to see the Firefox UI in Spanish (running a broken single-locale build?), you'd get:

  fr, en-US, es

with appropriate q codes. In the normal case you'd get:

  fr, en-US

and for the case of your OS matching your selected language:

  fr


(In reply to Axel Hecht [:Pike] from comment #20)

> Both Ernie and Bert change to another unsupported locale on the device.
> 
> What happens? Can we tell Ernie and Bert apart? Do we need to?

In both cases the language selection and the explicit Accept-Language election occur in Java-land, so yes, we can. The only case we'd be breaking is that of a user or add-on trying to set intl.a_l (e.g., in about:config).

The explicit A-L selection in the UI would overrule even a later browser locale change, just as setting a value for intl.a_l does right now.

We would probably consider saying "hey, you just switched from Spanish to Tagalog; do you want to change your preferred web content language, too?", to avoid Bert being trapped by a decision that he forgot he made, but I'll punt that to a follow-up, because it's getting into edge cases of edge cases.
(In reply to Richard Newman [:rnewman] from comment #21)
> (In reply to Jeff Beatty [:gueroJeff] from comment #19)
> > I think this looks like a good proposal. Just for a sanity check, line item
> > three in your proposal would give priority preference to the default, user
> > selected value from the language picker, correct?
> 
> Yes. If your OS was in English, you selected to use Firefox in French, and
> hypothetically were forced to see the Firefox UI in Spanish (running a
> broken single-locale build?), you'd get:
> 
>   fr, en-US, es
> 
> with appropriate q codes. In the normal case you'd get:
> 
>   fr, en-US
> 
> and for the case of your OS matching your selected language:
> 
>   fr
> 
> 
> (In reply to Axel Hecht [:Pike] from comment #20)
> 
> > Both Ernie and Bert change to another unsupported locale on the device.
> > 
> > What happens? Can we tell Ernie and Bert apart? Do we need to?
> 
> In both cases the language selection and the explicit Accept-Language
> election occur in Java-land, so yes, we can. The only case we'd be breaking
> is that of a user or add-on trying to set intl.a_l (e.g., in about:config).
> 
> The explicit A-L selection in the UI would overrule even a later browser
> locale change, just as setting a value for intl.a_l does right now.
> 
> We would probably consider saying "hey, you just switched from Spanish to
> Tagalog; do you want to change your preferred web content language, too?",
> to avoid Bert being trapped by a decision that he forgot he made, but I'll
> punt that to a follow-up, because it's getting into edge cases of edge cases.

Cool, I'm satisfied :-)
Blocks: 881510
While I was here, figured it was worth commenting about what kinds of locale strings these methods returned.
Saving my place (though this patch is likely complete).

Analysis:

Places that changes could occur:
* OS locale changed during runtime. (GeckoApp#onConfigurationChanged)
* OS locale changed outside of runtime. (GeckoApp#onCreate)
* User selected a different locale through the locale picker, but OS locale is unchanged. (Only happens when Gecko is running, already handled.)

We can't use JNI to get the OS locale: if the user picked one, we overwrote the OS locale before Gecko even runs. So we use messaging at the right points.

No need to send the selected locale: we already have that.

So, on startup, send the raw OS locale to Gecko. To avoid doing this unnecessarily, store it in SharedPreferences and emit a change to Gecko only when it differs.

Do the same thing in onConfigurationChanged.
Untested, but it looks like this should work...
N.B., we cannot use GeckoApp#onConfigurationChanged, because GeckoApplication fixes up the locale before it runs.
Running notes:

Recording the external locale during GeckoApplication#onConfigurationChanged appears to be correct, but I have little confidence that we will always get the OS locale in the provided configuration -- we have no way to tell.

As such, I think the safest approach is to detect the OS locale *only* during cold launch. If the user changes locales during Fennec's runtime, we won't alter the Accept-Language header until next cold launch.

This seems like a good tradeoff to me.
Attachment #8490487 - Attachment is obsolete: true
Attachment #8490522 - Attachment is obsolete: true
Note that this test won't work correctly if run multiple times unless you have the fix from Bug 1069687.

Note that the test is written to work correctly in both single- and multi-locale builds; their properties differ.
Comment on attachment 8490486 [details] [diff] [review]
Part 0: add descriptive comments for locale handling. v1

r=me, trivial.
Attachment #8490486 - Flags: review+
Attachment #8491958 - Flags: review?(bnicholson)
Comment on attachment 8491959 [details] [diff] [review]
Part 2: set intl.accept_languages from OS/app locale. v2

We don't seem to remove observers anywhere in BrowserApp (JS), so I followed that pattern.
Attachment #8491959 - Flags: review?(bnicholson)
Comment on attachment 8491960 [details] [diff] [review]
Part 3: tests for OS locale handling. v1

Note that:

* This test introduces a coupling between SharedPreferences and Gecko prefs. In a world where test profiles are partially reused (see comments earlier in this bug), this will cause the test to fail. It shouldn't happen on infra, where the package is uninstalled and reinstalled.

* The results of the test depends on whether the app is multilocale or not. I made the test adapt at runtime.

* The results of the test depends on the OS locale. I left a comment about this.
Attachment #8491960 - Flags: review?(bnicholson)
Comment on attachment 8491958 [details] [diff] [review]
Part 1: send OS locale from Android to Gecko. v2

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

::: mobile/android/base/BrowserLocaleManager.java
@@ +256,5 @@
> +     * @param prefs the SharedPreferences instance to use. Cannot be null.
> +     * @param osLocale the new locale instance. Safe if null.
> +     */
> +    public static void storeAndNotifyOSLocale(final SharedPreferences prefs,
> +                                               final Locale osLocale) {

Nit: shift indentation 1 space left

::: mobile/android/base/GeckoApp.java
@@ +1300,1 @@
>                  String appLocale = localeManager.getAndApplyPersistedLocale(GeckoApp.this);

appLocaleString for consistency?
Attachment #8491958 - Flags: review?(bnicholson) → review+
Comment on attachment 8491959 [details] [diff] [review]
Part 2: set intl.accept_languages from OS/app locale. v2

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

::: mobile/android/chrome/content/browser.js
@@ +338,5 @@
>      ViewportHandler.init();
>  
>      Services.androidBridge.browserApp = this;
>  
> +    Services.obs.addObserver(this, "Locale:OS", false);

This should be moved to the first patch (or fold patches together).

@@ +1799,5 @@
>          // correct locale.
>          Services.strings.flushBundles();
> +
> +        // Make sure we use the right Accept-Language header.
> + 

Nit: ws

@@ +1838,5 @@
> +    if (defaultAccept && defaultAccept.startsWith("chrome://")) {
> +      defaultAccept = null;
> +    } else {
> +      // Ensure lowercase everywhere so we can
> +      // compare.

Nit: no line break

@@ +1861,5 @@
> +                            .filter((x) => (x != appLocale && x != osLocale));
> +    }
> +
> +    if (osLocale) {
> +      chosen.unshift(osLocale);

Since you're guarding against situations where defaultAccept is null, chosen can be undefined here. Initialize chosen as an empty array?

@@ +1868,5 @@
> +    if (appLocale && appLocale != osLocale) {
> +      chosen.unshift(appLocale);
> +    }
> +
> +    let result = chosen.join(", ");

Remove the space here to save a few bits in the header. :D
Attachment #8491959 - Flags: review?(bnicholson) → review+
Comment on attachment 8491960 [details] [diff] [review]
Part 3: tests for OS locale handling. v1

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

::: mobile/android/base/tests/testOSLocale.java
@@ +23,5 @@
> +                }
> +            }
> +        };
> +
> +        ThreadUtils.getBackgroundHandler().postDelayed(r, 2000);

This is pretty gross. Are there no events we can listen for in the locale switching instead of relying on the background queue? If not, is there at least some startup event where we know the locale processing has been queued so we don't need this arbitrary 2 second timeout?

::: mobile/android/base/tests/testOSLocale.js
@@ +35,5 @@
> +    do_check_eq(getOSLocale(), "en-US");
> +    run_next_test();
> +  };
> +
> +  Services.prefs.addObserver("intl.locale.os", osObserver, false);

Since JS is single-threaded, you can move this after the if statement (and remove the removeObserver below).
Attachment #8491960 - Flags: review?(bnicholson) → review+
Thanks for the thorough review!


> This is pretty gross.

Yeah, I know!


> Are there no events we can listen for in the locale
> switching instead of relying on the background queue?

Not really. Locale switching is one-way, and OS locale sending is the last thing we do in GeckoApp's background work.


> If not, is there at
> least some startup event where we know the locale processing has been queued
> so we don't need this arbitrary 2 second timeout?

It should work to listen for Gecko:DelayedStartup and post to the background thread from that listener, so let me try that. It'll be more gross in that we'll register a listener to fire a runnable, but less gross in that we don't hard-code a wait.
When running locally, bear in mind Bug 1069687.
Depends on: 1069687
That test failure:

 00:30:25 WARNING - TEST-UNEXPECTED-FAIL | testOSLocale | GeckoEventExpecter - blockForEvent timeout: Robocop:JS

is congruent with the test machines running these tests more than once between package uninstalls.

But of course what's really happening is they're *launching the activity* more than once, so part of the logic that this test is exercising is happening during an earlier test.

Which isn't what we want. So we need to modify this test to have a blank slate before starting GeckoApp.
Status update: I am *still* trying to get these tests to pass reliably on infra. Might have to fix profile stuff in order to get there. *sigh*
The JavascriptTest approach was consistently passing locally and failing on infra (*after* the test passed, it waited for another Robocop:JS message that never came).

After wasting an insane amount of time trying to get that to work, and reading spidery logs, I just rewrote the test to use PrefsHelper and be pure Java, got it to pass locally and on Try, and landed the damn thing.

Lesson learned.
Flagging me for uplift after bake.
Flags: needinfo?(rnewman)
Set in-app locale to es-ES, system locale to en-US. Watched HTTP headers.

es-es,en-us;q=0.8,es;q=0.5,en;q=0.3

Set OS locale to fr-FR, relaunched Firefox.

es-es,fr-fr;q=0.8,es;q=0.6,en-us;q=0.4,en;q=0.2
Status: RESOLVED → VERIFIED
Flags: needinfo?(rnewman)
Something to think about.

Let's say the user has an OS locale we don't support -- call it xx-YY. They launch Firefox for the first time.

We'll store that locale on both sides (to avoid doing all this work every time).

On the Gecko side, we'll compute an Accept-Language header, which will be partly based on en-US's values -- after all, they're effectively using Firefox in English, and that includes headers.

We'll save

  xx-yy,en-US,en

as their Accept-Language header.

If we later add support for xx-YY, along with a localized value for intl.accept_languages, existing xx-YY users will be 'stranded' with an incorrect intl.accept_languages user-set override.

Rather than having

  xx-yy,xx,zz,en

they'll still have

  xx-yy,en-US,en


You can only fix this by:

1. Switching app locales (which would include going from "System default" to "xx-YY").
2. Switching your OS locale, relaunching Firefox, switching again, relaunching Firefox again.

tl;dr: these changes ossify the value of intl.accept_languages at the moment Firefox determines your OS locale.

We can fix this by providing some hook to recompute the Accept-Languages header in some circumstances, or we can leave this as a support issue.

Any thoughts before I request uplift?
Flags: needinfo?(jbeatty)
Thanks for your thorough description of the issue. I think we should document this as a support issue. Users may find it a bother to see their Accept-langauges header settings reverted without provocation.
Flags: needinfo?(jbeatty)
It's not really about reverting; it's that changes to the underlying intl.properties will be ignored.

Though it's worth calling out: with this change we no longer support user editing of intl.accept_languages in Fennec, if we ever did.

(User-set values will only be overwritten if they change OS or app locale, but that's enough to make that statement, I think.)
Comment on attachment 8491959 [details] [diff] [review]
Part 2: set intl.accept_languages from OS/app locale. v2

Approval Request Comment

Requesting uplift for all four parts.

[Feature/regressing bug #]:
  New work to make OS and app locale selections affect HTTP Accept-Language header.

[User impact if declined]:
  Users will need to use about:config to adjust their preferred web content language -- i.e., they can't.

[Describe test coverage new/current, TBPL]:
  Robocop test added. Tested manually.

[Risks and why]: 
  Fairly low risk -- it pipes values from Java to JS, then very defensively computes a new value for intl.accept_languages.

  This does slightly change the 'character' of intl.accept_languages. It'll be lowercased and explicitly set, so we'll be relying on this mechanism to correctly convey the OS locale from Android to Gecko.

[String/UUID change made/needed]:
  None.
Attachment #8491959 - Flags: approval-mozilla-aurora?
Attachment #8491959 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
ni me for uplift.
Flags: needinfo?(rnewman)
Flags: needinfo?(rnewman)
(In reply to Richard Newman [:rnewman] from comment #56)
> It's not really about reverting; it's that changes to the underlying
> intl.properties will be ignored.
> 
> Though it's worth calling out: with this change we no longer support user
> editing of intl.accept_languages in Fennec, if we ever did.
> 
> (User-set values will only be overwritten if they change OS or app locale,
> but that's enough to make that statement, I think.)

ok, good to know. How do we initiate a support page for this to instruct users on it?
Depends on: 1091803
Blocks: 1092423
Too risky for 34 right?
Yup.
Blocks: 839881
Is this legal javascript?

http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js?rev=8e516e029c8e#1923

When I build fennec locally I run browser.js through spidermonkey to check for syntax errors and it is complaining about this:

/home/kats/tmp/check-this.js:1893:24 SyntaxError: missing : after property id:
/home/kats/tmp/check-this.js:1893:24   computeAcceptLanguages(osLocale, appLocale) {
/home/kats/tmp/check-this.js:1893:24 ........................^
Flags: needinfo?(rnewman)
Never mind, updating my spidermonkey build made this go away so I assume this syntax is new in JS. TIL.
Flags: needinfo?(rnewman)
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.