The default bug view has changed. See this FAQ.

Allow for locale switching functionality to be disabled

RESOLVED FIXED in Firefox 32

Status

()

Firefox for Android
Locale switching and selection
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: rnewman, Assigned: rnewman)

Tracking

32 Branch
Firefox 32
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(fennec32+)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

3 years ago
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).
(Assignee)

Comment 1

3 years ago
Created attachment 8433820 [details] [diff] [review]
Part 1: remove Language section from preferences if locale switching is disabled.
(Assignee)

Comment 2

3 years ago
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)
(Assignee)

Comment 3

3 years ago
Created attachment 8433829 [details] [diff] [review]
Part 2: disable locale switching on API level 11 and 12. v1

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)
(Assignee)

Comment 4

3 years ago
Created attachment 8433876 [details] [diff] [review]
Part 1: remove Language section from preferences if locale switching is disabled. v2

Forgot a chunk.
(Assignee)

Updated

3 years ago
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+
(Assignee)

Comment 7

3 years ago
   https://hg.mozilla.org/integration/fx-team/rev/a5088ee3f921
   https://hg.mozilla.org/integration/fx-team/rev/d8174c0580e5
(Assignee)

Updated

3 years ago
Blocks: 1015209
https://hg.mozilla.org/mozilla-central/rev/a5088ee3f921
https://hg.mozilla.org/mozilla-central/rev/d8174c0580e5
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 32

Updated

3 years ago
Depends on: 1065531
You need to log in before you can comment on or make changes to this bug.