Closed Bug 635342 Opened 12 years ago Closed 12 years ago

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


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




Tracking Status
fennec 2.0+ ---


(Reporter: nhirata, Assigned: blassey)


(Keywords: l12y)


(1 file, 2 obsolete files)

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
this string is localized, I assume these locales have just copied the english string
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
Priority: -- → P2
Priority: P2 → P1
Attached patch patch (obsolete) — Splinter Review
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.
Assignee: nobody → blassey.bugs
Attachment #513838 - Flags: review?(doug.turner)
tracking-fennec: --- → ?
Attached patch patch (obsolete) — Splinter Review
Attachment #513838 - Attachment is obsolete: true
Attachment #513838 - Flags: review?(doug.turner)
Attachment #513841 - Flags: review?(doug.turner)
tracking-fennec: ? → 2.0+
Keywords: l12y
Whiteboard: [has-patch][needs review]
Comment on attachment 513841 [details] [diff] [review]

>+        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/ b/embedding/android/
>--- a/embedding/android/
>+++ b/embedding/android/
>@@ -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();


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.
Attachment #513841 - Flags: review?(doug.turner) → review-
> >+        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.
Attached patch patchSplinter Review
there is some unrelated whitespace clean up in this patch, hope you don't mind
Attachment #513841 - Attachment is obsolete: true
Attachment #515209 - Flags: review?(doug.turner)
Comment on attachment 515209 [details] [diff] [review]

what sort of testing have you done? what is the test plan?
Attachment #515209 - Flags: review?(doug.turner) → review+

(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
Closed: 12 years ago
Resolution: --- → FIXED
Brad, is the fix on the nighly build so I can test it too?
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.
the patch should be in the 3/1 build and that is the expected behavior now
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 )
Whiteboard: [has-patch][needs review]
You need to log in before you can comment on or make changes to this bug.