733515 - Use localized strings for special folder names

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Richard Newman [:rnewman]]

See <https://bugzilla.mozilla.org/show_bug.cgi?id=733481#c9>:

Fennec should also use a localized "magic" string for the title [of each special record], rather than looking in the database; if it doesn't, then folder names won't track localization switches on the device, rather keeping whichever title was in play when the DB was initialized.

Comment 1

[:Margaret Leibovic]

Attachment: patch

Luckily for us, the string are already there!

Comment 2

[Richard Newman [:rnewman]]

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

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

::: mobile/android/base/AwesomeBarTabs.java
@@ +271,5 @@
> +            String title;
> +            String guid = c.getString(c.getColumnIndexOrThrow(Bookmarks.GUID));
> +
> +            // Use localized strings for special folder names
> +            if (guid.equals(Bookmarks.MOBILE_FOLDER_GUID))

Early returns?

Also, teensy bit of performance and NPE-safety: skip all of these string checks if the GUID is null or has length 12.

Length on a string is fast, and none of our special GUIDs (and all of our non-special GUIDs!) have a "regular" GUID. E.g.,


// Can't be a special GUID.
if (guid == null || guid.length == 12)
    return c.getString(c.getColumnIndexOrThrow(Bookmarks.TITLE));

// Use localized strings for special folder names.
if (guid.equals(Bookmarks.MOBILE_FOLDER_GUID))
    return BOOKMARKS_FOLDER_MENU_TITLE;
if (guid.equals(Bookmarks.TOOLBAR_FOLDER_GUID))
    return BOOKMARKS_FOLDER_TOOLBAR_TITLE;
...

@@ +272,5 @@
> +            String guid = c.getString(c.getColumnIndexOrThrow(Bookmarks.GUID));
> +
> +            // Use localized strings for special folder names
> +            if (guid.equals(Bookmarks.MOBILE_FOLDER_GUID))
> +                title = mContext.getResources().getString(R.string.bookmarks_folder_mobile);

Is it worth caching these? (I don't know too much about the performance characteristics of Resources.)

Comment 3

[Lucas Rocha (:lucasr)]

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

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

::: mobile/android/base/AwesomeBarTabs.java
@@ +272,5 @@
> +            String guid = c.getString(c.getColumnIndexOrThrow(Bookmarks.GUID));
> +
> +            // Use localized strings for special folder names
> +            if (guid.equals(Bookmarks.MOBILE_FOLDER_GUID))
> +                title = mContext.getResources().getString(R.string.bookmarks_folder_mobile);

Caching resources in a private mResources member seems doable here, given that we use it quite often here.

Comment 4

[:Margaret Leibovic]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7536add66c43

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7536add66c43