Closed Bug 1019981 Opened 6 years ago Closed 6 years ago

Allow for locale switching functionality to be disabled

Categories

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

32 Branch
All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 32
Tracking Status
fennec 32+ ---

People

(Reporter: rnewman, Assigned: rnewman)

References

Details

Attachments

(2 files, 1 obsolete file)

This entails:

* Hiding the pref item.
* Optionally: ignoring any selected value (just in case), behaving as if "System default" were selected.

The initial motivation for this is threefold:

* To disable the feature on multipane API 11 and 12 devices, allowing us time to fix and test Bug 1015209.
* To allow for shipping with a build flag, in case some serious bug comes up.
* To provide a trivial hook for disabling in a single-locale build (bug not yet filed).
Comment on attachment 8433820 [details] [diff] [review]
Part 1: remove Language section from preferences if locale switching is disabled.

Notes:

* This adds only one build flag: MOZ_LOCALE_SWITCHER. This anticipates adding the content language picker later (see notes in the patch). When this happens we'll stop hiding the whole section, and start hiding just the locale switching pref itself.

* We need to remove the pref section in two places: one for fragments (header, added ID to do this), one for non-fragments. The latter is kinda ugly (check for the resource in the extra), but it's the most correct way to do it.

* The use of "preferences_locale" and "resource" inline is idiomatic for this file.

* Gah this whole pattern is kinda horrible, but hey.
Attachment #8433820 - Flags: review?(nalexander)
This is a stopgap until we have a real fix for API 12 and 13. Simply treat it as disabled on Honeycomb tablets.
Attachment #8433829 - Flags: review?(nalexander)
Attachment #8433820 - Attachment is obsolete: true
Attachment #8433820 - Flags: review?(nalexander)
Comment on attachment 8433876 [details] [diff] [review]
Part 1: remove Language section from preferences if locale switching is disabled. v2

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

Looks good!

::: configure.in
@@ +4949,5 @@
>  
>  dnl ========================================================
> +dnl = Enable runtime locale switching on Android
> +dnl ========================================================
> +if test -n "$MOZ_LOCALE_SWITCHER"; then

Do you not need/want MOZ_LOCALE_SWITCHER= before this?

::: mobile/android/base/AppConstants.java.in
@@ +120,5 @@
>  #else
>      false;
>  #endif
>  
> +    public static final boolean MOZ_LOCALE_SWITCHER = 

nit: trailing ws.
Attachment #8433876 - Flags: review+
Comment on attachment 8433829 [details] [diff] [review]
Part 2: disable locale switching on API level 11 and 12. v1

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

::: mobile/android/base/preferences/GeckoPreferences.java
@@ +295,5 @@
> +
> +                if (Build.VERSION.SDK_INT < 13) {
> +                    // Affected by Bug 1015209 -- no detach/attach.
> +                    // We can't do fragment rejigging correctly, so don't enable
> +                    // locale switching at all if it'll crash.

nit: phrase this as a statement, not a possibility: ... so don't enable locale switching at all, since it will crash.
Attachment #8433829 - Flags: review?(nalexander) → review+
Blocks: 1015209
https://hg.mozilla.org/mozilla-central/rev/a5088ee3f921
https://hg.mozilla.org/mozilla-central/rev/d8174c0580e5
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 32
Depends on: 1065531
You need to log in before you can comment on or make changes to this bug.