Or similar. As of bug 1201206 we extend Theme.AppCompat and want to use its features but because of this bug, we can't! In particular, we'd like to tint the check box preferences.

We should be sure to remove the android: prefixes where applicable (see bug 1215198).

Can I investigate this bug and try to come up with a patch?

(In reply to k.krish from comment #2) > Can I investigate this bug and try to come up with a patch? Yes, patches are always welcome! Feel free to post any questions around the build / patch here, I'd also reccomend joining the #mobile IRC channel (that tends to be busiest Mon-Fri)!

I discovered the Android SDK ships an example AppCompatPreferenceActivity, I wonder if it's worth just importing that into our codebase and switching GeckoPreferences to extend that? See: https://android.googlesource.com/platform/development/+/master/samples/Support7Demos/src/com/example/android/supportv7/app/AppCompatPreferenceActivity.java

From the docs you posted: > /** > * A {@link android.preference.PreferenceActivity} which implements and proxies the necessary calls > * to be used with AppCompat. > * > * This technique can be used with an {@link android.app.Activity} class, not just > * {@link android.preference.PreferenceActivity}. > */ Nice find! It sounds like it could work! I'm slightly concerned that inflation won't be entirely correct, however. k.krish, are you interested in following the approach Andrzej suggested in comment 4?

Fixing this seems to be the simplest correct way to solve Bug 1250900, therefore taking this for myself. My approach involved importing AppCompatActivity, modifying (un-prefixing) some ActionBar related theme attributes, and modifying the GeckoPreferences ActionBar code to use getSupportActionbar(). This results in the preferences looking identical across Android 4 and newer (whereas previously Android 4 had different theming). Outstanding issues: - Dialog has old theming - Need to verify that this runs on Android >= 5 (we seem to set the prefixed actionbar attributes for v21, which I had to remove in v11 as to not crash on Android 4)

Review commit: https://reviewboard.mozilla.org/r/42873/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/42873/

I've found one major issue: - The Android AlertDialog doesn't work with AppCompat theming (looks broken: additional empty borders, and otherwise no change in appearance - screenshot attached) - We can replace with the v7 support AlertDialog in some but not all cases: most of our Preferences extend DialogPreference, which assumes the standard AlertDialog, hence we show the broken dialog. I.e. we need to either: (A) Have a mix of dialog styles (probably the worst option) (B) Stick with the old dialog style (but still use AppCompat for the preferences activity) (C) Implement an AppCompatDialogPreference to replace DialogPreference, in order to have consistent new styling. I don't see any huge issues with (B).

Here's what the dialogs currently look like on Android 4 - this is probably what we have to stick with.

Comment on attachment 8735597 [details] MozReview Request: Bug 1205124 - use AppCompatActivity for GeckoPreferences r?sebastian Does this look like a sane approach to you? Would you be comfortable with uplifting something like this to fix Bug 1250900?

(In reply to Andrzej Hunt :ahunt from comment #10) > Does this look like a sane approach to you? Would you be comfortable with > uplifting something like this to fix Bug 1250900? Note that uplifting this means you will have to support Gingerbread.

https://reviewboard.mozilla.org/r/42873/#review39619 I'd be fine uplifting this approach, provided we do a pretty thorough manual comb-over to make sure nothing changed. Like Sebastian said, we should test on all major design changes (GB, 4.0+, 5.0+, 4.0+ tablet, 5.0+ tablet). ::: mobile/android/base/java/org/mozilla/gecko/preferences/AppCompatPreferenceActivity.java:2 (Diff revision 1) > +package org.mozilla.gecko.preferences;/* > + * Copyright (C) 2014 The Android Open Source Project nit: fmting ::: mobile/android/base/java/org/mozilla/gecko/preferences/AppCompatPreferenceActivity.java:30 (Diff revision 1) > +import android.support.v7.app.AppCompatDelegate; > +import android.support.v7.widget.Toolbar; > +import android.view.MenuInflater; > +import android.view.View; > +import android.view.ViewGroup; > +/** nit: ws above ::: mobile/android/base/java/org/mozilla/gecko/preferences/AppCompatPreferenceActivity.java:35 (Diff revision 1) > +/** > + * A {@link android.preference.PreferenceActivity} which implements and proxies the necessary calls > + * to be used with AppCompat. > + * > + * This technique can be used with an {@link android.app.Activity} class, not just > + * {@link android.preference.PreferenceActivity}. Add a comment explaining if we modified anything here.

Oh, something for the future (because not uplift-able): Now that we are on Android 4+ we can use our PreferenceFragment implementation exclusively and do not need a PreferenceActivity anymore. I'll file a bug for that.

I'm still going through the various permutations, perhaps the biggest issue is that we now change the checkboxes on Gingerbread, everything else looks the same as before. I'm not sure this is necessarily a bad thing (GB default checkboxes look terrible IMHO), and since this will only exist in one release (and we're killing GB anyways), I'm not sure it's worth expending much effort on making it perfect, as long as we can fix the crash. Here's a screen on GB.

(In reply to Andrzej Hunt :ahunt from comment #14) > Created attachment 8736760 [details] > gb.png > > I'm still going through the various permutations, perhaps the biggest issue > is that we now change the checkboxes on Gingerbread, everything else looks > the same as before. > > I'm not sure this is necessarily a bad thing (GB default checkboxes look > terrible IMHO), and since this will only exist in one release (and we're > killing GB anyways), I'm not sure it's worth expending much effort on making > it perfect, as long as we can fix the crash. > > Here's a screen on GB. I think this is perfectly fine. We are looking forward to unify the themes with AppCompat and having the new checkboxes for the last GB releases is actually an improvement. ;)

Comment on attachment 8735597 [details] MozReview Request: Bug 1205124 - use AppCompatActivity for GeckoPreferences r?sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42873/diff/1-2/

Comment on attachment 8735597 [details] MozReview Request: Bug 1205124 - use AppCompatActivity for GeckoPreferences r?sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42873/diff/2-3/

Comment on attachment 8735597 [details] MozReview Request: Bug 1205124 - use AppCompatActivity for GeckoPreferences r?sebastian https://reviewboard.mozilla.org/r/42873/#review40641 Nice! Some things to test: * Phone vs. Tablet layouts (For example Nexus 7 portrait is a special case and doesn't use split pane) * Jumping from notification to settings (For example data reporting notification) * Switching between portrait / landscape * Resume with "Do not keep activities" activated ::: mobile/android/base/java/org/mozilla/gecko/preferences/GeckoPreferences.java:323 (Diff revision 3) > + // with an NPE in android.support.v7.app.AppCompatDelegateImplV7.ensureSubDecor(), and it's > + // likely other strange devices (other Asus devices, some Samsungs) could do the same. > + super.onCreate(savedInstanceState); > + > + // Apply the current user-selected locale, if necessary. > + checkLocale(); Does locale switching still work correctly after this has been moved after onCreate()? ::: mobile/android/base/java/org/mozilla/gecko/preferences/GeckoPreferences.java:329 (Diff revision 3) > + > + // Track this so we can decide whether to show locale options. > + // See also the workaround below for Bug 1015209. > + localeSwitchingIsEnabled = BrowserLocaleManager.getInstance().isEnabled(); > + > + if (Versions.feature11Plus && onIsMultiPane()) { Why did you add feature11Plus here? With minSdkVersion set to 15 everything should be 11+.

https://hg.mozilla.org/integration/fx-team/rev/9fb7908df083b09946413585d48d02bba09de934 Bug 1205124 - use AppCompatActivity for GeckoPreferences r=sebastian

[Tracking Requested - why for this release]: This fixes Bug 1250900, which is tracking 47.

Comment on attachment 8735597 [details] MozReview Request: Bug 1205124 - use AppCompatActivity for GeckoPreferences r?sebastian Approval Request Comment [Feature/regressing bug #]: Bug 1227325 [User impact if declined]: Cannot set master password on 2.x and 4.x devices. Firefox crashes when selecting the master password preference. [Describe test coverage new/current, TreeHerder]: Manual testing on 2.3 phone, 4.4 phone and tablet, 5.1 phone, 5.0 tablet and 6.0 tablet. [Risks and why]: Low risk: theming on 4.x devices now matches 5.0 and newer. (Theming on 2.3 doesn't change with this patch.) [String/UUID change made/needed]: None. Note: this patch causes a regression in sync settings theming (old-style theming on 4.x devices, missing titles on >= 5.0 devices), this is fixed in Bug 1264138 which has just landed in fx-team, I'll request uplift on that once it's made it through a nightly or two.

I will approve the uplift on this one together with aurora uplift (when ready) for bug 1264138.

Comment on attachment 8735597 [details] MozReview Request: Bug 1205124 - use AppCompatActivity for GeckoPreferences r?sebastian Regression in setting master password on some devices, Aurora47+

Hi Teodora, could you please verify this issue is fixed as expected on a latest Nightly build? This caused regression 1264138 and since you verified that I thought I could your your help here as well. Thanks!

Using 11-04 Nightly build, Firefox crashes when selecting the master password preference using Alcatel One Touch (Android 4.1.2). On 21-04 Nightly build, "Create Master Password" dialog is displayed when choosing "user master password" on Moto X (Android 4.4) and Alcatel one touch (Android 4.1.2) Prompt & Dialog on latest Nightly: Alcatel One Touch (Android 4.1.2) http://i.imgur.com/MvvK2uM.png http://i.imgur.com/ee7ZBrM.png One plus one (Android 5.1.1) http://i.imgur.com/JpRzqWR.png http://i.imgur.com/YWzm2S3.png HTC Desire HD (Android 5.0.2) http://i.imgur.com/JpRzqWR.png http://i.imgur.com/YWzm2S3.png Samsung Tab S2 (Android 5.0.2) http://i.imgur.com/ukqjz9Y.png http://i.imgur.com/VvDhxon.png Moto X (Android 4.4) http://i.imgur.com/1oCAnsy.png http://i.imgur.com/4WMzFi9.png Nexus 9 (Android 6.0) http://i.imgur.com/BC0m3Mg.png http://imgur.com/OduKvjL

Verified based on comment 28