635342 - The word "Loading" on startup is not translated into any languages

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Description

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Mozilla/5.0 (Android; Linux armv71; rv2.0b12pre) Gecko/20110217 Firefox/4.0b12pre Fennec/4.0b5 (Beta 5 candidate 3)
Device: Droid 2 
OS: Android 2.2

1. set OS language to Spanish
2. launch firefox
3. set the firefox language to French
4. force quit the app
5. relaunch the app

Expected: the startup screen should say Loading in either Spanish or French (not sure which)
Actual: the startup splash says Loading in English

Comment 1

[Brad Lassey [:blassey] (use needinfo?)]

this string is localized, I assume these locales have just copied the english string

https://mxr.mozilla.org/mozilla-central/source/embedding/android/locales/en-US/android_strings.dtd#1

Comment 2

[u60234]

No, the string is translated in both fr and es-ES. They are just not used.

http://hg.mozilla.org/l10n-central/fr/file/c56a5c97e334/embedding/android
http://hg.mozilla.org/l10n-central/es-ES/file/7ee59ed89404/embedding/android

Comment 3

[Brad Lassey [:blassey] (use needinfo?)]

in the multi-local builds the java strings are selected based on the device's selected locale. I just tested by switching my devices locale to french and I got the french splash screen messages.

single locale (if we had them) would only have one set of strings, so this wouldn't be an issue

Comment 4

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch

this patch listens for changes to the locale prefs in nsAppShell and calls into GeckoAppShell to reflect that to the java Locale system. It also saves the selected locale to a presistant pref so we can start up with the right locale even before we can read prefs.

Comment 5

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch

Comment 6

[Doug Turner (:dougt)]

Comment on attachment 513841 [details] [diff] [review]
patch


>+        SharedPreferences settings = getSharedPreferences("GeckoPrefs", 0);


getPreferences() instead?  Do we already have multiple android preference files?  If this is for future proofing or something, please change 0 to MODE_PRIVATE.


>+        String localeCode = settings.getString(getPackageName() + ".locale", "");

No need to pass that empty string since we aren't formating anything.

>+        if (localeCode != null && localeCode.length() > 0) {
>+            Locale locale = new Locale(localeCode); 
>+            Locale.setDefault(locale);

Don't we want to set both the language code and the country code?

Locale(String language, String country) -- Constructs a new Locale using the specified language and country codes.


>+            
>+            Configuration config = new Configuration();
>+            config.locale = locale;
>+            getBaseContext().getResources().updateConfiguration(config,
>+                                                getBaseContext().getResources().getDisplayMetrics());
>+        }

I don't understand the chain of calls to get to updateConfiguration.  can these throw/fail?  It would be nice if you can to the actual Configuration being used and call updateFrom().


>             public void run() {
>+                // At some point while loading the gecko libs our default locale gets set
>+                // so just save it to locale here and reset it as default after the join
>+                Locale locale = Locale.getDefault();
>                 GeckoAppShell.loadGeckoLibs(
>                     getApplication().getPackageResourcePath());
>+                                Locale.setDefault(locale);                        
>+                Configuration config = new Configuration();

Spacing looks off.

>diff --git a/embedding/android/GeckoAppShell.java b/embedding/android/GeckoAppShell.java
>--- a/embedding/android/GeckoAppShell.java
>+++ b/embedding/android/GeckoAppShell.java
>@@ -845,4 +845,18 @@ class GeckoAppShell
>             return false;
>         return true;
>     }
>+
>+    public static void setSelectedLocale(String localeCode) {
>+        SharedPreferences settings = GeckoApp.mAppContext.getSharedPreferences("GeckoPrefs", 0);

Same as above.

>+        SharedPreferences.Editor editor = settings.edit();
>+        editor.putString(GeckoApp.mAppContext.getPackageName() + ".locale", localeCode);
>+        editor.commit();

terse:

setting.edit().putString(GeckoApp.mAppContext.getPackageName() +
                         ".locale", localeCode).commit();


>+
>+    nsCOMPtr<nsIPrefBranch2> branch = do_GetService(NS_PREFSERVICE_CONTRACTID, &rv);
>+    if (branch) {

just |if (NS_FAILED(rv)) return rv;|

>+        branch->AddObserver("intl.locale.matchOS", this, PR_FALSE);
>+        branch->AddObserver("general.useragent.locale", this, PR_FALSE);
>+        if (bridge) {

Can't we just bail if bridge is null way before getting here?  If it is null, i don't know why we pretend that everything can/should keep going.

>+            PRBool match = PR_FALSE;
>+            nsCString locale;
>+            branch->GetBoolPref("intl.locale.matchOS", &match);
>+            if (!match ||
>+                NS_FAILED(branch->GetCharPref("general.useragent.locale", getter_Copies(locale))))
>+                bridge->SetSelectedLocale(EmptyCString());
>+            else
>+                bridge->SetSelectedLocale(locale);


>@@ -135,7 +155,24 @@ nsAppShell::Observe(nsISupports* aSubjec
>         // We need to ensure no observers stick around after XPCOM shuts down
>         // or we'll see crashes, as the app shell outlives XPConnect.
>         mObserversHash.Clear();
>+    } else if (!strcmp(aTopic, NS_PREFBRANCH_PREFCHANGE_TOPIC_ID) && (
>+                   !wcscmp((const wchar_t*)aData, L"intl.locale.matchOS") ||
>+                   !wcscmp((const wchar_t*)aData, L"general.useragent.locale"))) {
>+        AndroidBridge* bridge = AndroidBridge::Bridge();
>+        nsCOMPtr<nsIPrefBranch> prefs = do_QueryInterface(aSubject);
>+        if (prefs && bridge) {
>+            PRBool match = PR_FALSE;
>+            nsXPIDLCString locale;
>+            prefs->GetBoolPref("intl.locale.matchOS", &match);
>+            if (!match ||
>+                NS_FAILED(prefs->GetCharPref("general.useragent.locale", getter_Copies(locale))))
>+                bridge->SetSelectedLocale(EmptyCString());
>+            else
>+                bridge->SetSelectedLocale(locale);            
>+        }
>+        return NS_OK;


i think i would have spread this out a bit and done more early returns.

Comment 7

[Brad Lassey [:blassey] (use needinfo?)]

> >+        String localeCode = settings.getString(getPackageName() + ".locale", "");
> 
> No need to pass that empty string since we aren't formating anything.

That empty string isn't for formatting, its the default value (i.e. what to return if the pref isn't set). It needs to be passed in, unless you see another api to use.

Comment 8

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch

there is some unrelated whitespace clean up in this patch, hope you don't mind

Comment 9

[Doug Turner (:dougt)]

Comment on attachment 515209 [details] [diff] [review]
patch

what sort of testing have you done? what is the test plan?

Comment 10

[Brad Lassey [:blassey] (use needinfo?)]

pushed http://hg.mozilla.org/mozilla-central/rev/b66fe2e433c9

(In reply to comment #9)
> Comment on attachment 515209 [details] [diff] [review]
> patch
> 
> what sort of testing have you done? what is the test plan?

I've basically just switched between several of the locales repeatedly and made sure the strings were in the right language

Comment 11

[Jean-Marc Spaggiari]

Brad, is the fix on the nighly build so I can test it too?

Comment 12

[Ioana Chiorean]

So for me it work like this:
- set the devices  language to Russian
- set Fennec language to French

the application loading screen is translated in French. 

Is this the correct behavior? Or it should be translated in Russian?

@Brad the patch should be in the 03/01 build.

Comment 13

[Brad Lassey [:blassey] (use needinfo?)]

the patch should be in the 3/1 build and that is the expected behavior now

Comment 14

[Ioana Chiorean]

Mozilla /5.0 (Android;Linux armv7l;rv:2.0b13pre) Gecko/20110301 Firefox/4.0b13pre Fennec /4.0b6pre 

So  it work like this:
- set the devices  language to Russian
- set Fennec language to French

The application loading screen is translated in French (expected behavior -> verified fixed )