Bug 1462594 - Allow accessing all Settings menus on tablets; r?sdaswani draft
authorPetru Lingurar <petru.lingurar@softvision.ro>
Thu, 24 May 2018 14:09:22 +0300
changeset 799269 69274b1c8a29457f5368ffe8043eff7de46dd6fd
parent 799267 043e4ab6e72469ed8121f4da98dcdfef983a49d9
push id110984
push userplingurar@mozilla.com
push dateThu, 24 May 2018 11:12:13 +0000
reviewerssdaswani
bugs1462594
milestone62.0a1
Bug 1462594 - Allow accessing all Settings menus on tablets; r?sdaswani Bug details: The problem stemmed from the now called GeckoPreferences.trySwitchToHeader(int id) which could be called with an invalid id, constant with the same value as the id of the last available setting. (GeckoPreferenceFragment().getHeader() would return valid ids only for preference screens that are launched directly. Otherwise it would return: -1) By chance the id for the last available setting - vendor was not set and so Android saw it with an invalid header id: -1. GeckoPreferences.trySwitchToHeader(int id) would just switch to showing the vendor setting because that is what he has been instructed to whenever the user tried to access other settings than the ones which can be launched directly. Cleaned the code a bit: - renamed GeckoPreferences.switchToHeader(..) to trySwitchToHeader(..) as it won't always perform that action - removed the call to activity.showBreadCrumbs(..) as in my tests it didn't have any effect and the documentation says "This will normally be called for you". Tested on An Android 8 tablet, on an Android 8 phone, on an Android 5.0.1 phone and all works ok. MozReview-Commit-ID: 2sbfcuRHgZd
mobile/android/app/src/main/res/xml-v11/preference_headers.xml
mobile/android/base/java/org/mozilla/gecko/preferences/GeckoPreferenceFragment.java
mobile/android/base/java/org/mozilla/gecko/preferences/GeckoPreferences.java
--- a/mobile/android/app/src/main/res/xml-v11/preference_headers.xml
+++ b/mobile/android/app/src/main/res/xml-v11/preference_headers.xml
@@ -61,14 +61,15 @@
     <header android:fragment="org.mozilla.gecko.preferences.GeckoPreferenceFragment"
         android:title="@string/pref_default_browser"
         android:id="@+id/pref_header_default_browser">
         <extra android:name="resource"
             android:value="preferences_default_browser_tablet"/>
     </header>
 
     <header android:fragment="org.mozilla.gecko.preferences.GeckoPreferenceFragment"
-            android:title="@string/pref_header_vendor">
+            android:title="@string/pref_header_vendor"
+            android:id="@+id/pref_header_vendor">
         <extra android:name="resource"
                android:value="preferences_vendor"/>
     </header>
 
 </preference-headers>
--- a/mobile/android/base/java/org/mozilla/gecko/preferences/GeckoPreferenceFragment.java
+++ b/mobile/android/base/java/org/mozilla/gecko/preferences/GeckoPreferenceFragment.java
@@ -35,16 +35,17 @@ import android.view.MenuInflater;
 
 /* A simple implementation of PreferenceFragment for large screen devices
  * This will strip category headers (so that they aren't shown to the user twice)
  * as well as initializing Gecko prefs when a fragment is shown.
 */
 public class GeckoPreferenceFragment extends PreferenceFragment {
 
     public static final int ACCOUNT_LOADER_ID = 1;
+    public static final int HEADER_ID_UNDEFINED = -1;
     private AccountLoaderCallbacks accountLoaderCallbacks;
     private SyncPreference syncPreference;
 
     @Override
     public void onConfigurationChanged(Configuration newConfig) {
         super.onConfigurationChanged(newConfig);
         Log.d(LOGTAG, "onConfigurationChanged: " + newConfig.locale);
 
@@ -144,35 +145,31 @@ public class GeckoPreferenceFragment ext
         if (res == R.xml.preferences_search) {
             return R.id.pref_header_search;
         }
 
         if (res == R.xml.preferences_notifications) {
             return R.id.pref_header_notifications;
         }
 
-        return -1;
+        return HEADER_ID_UNDEFINED;
     }
 
     private void updateParentTitle() {
         final String newTitle = getTitle();
         if (newTitle == null) {
             Log.d(LOGTAG, "No new title to show.");
             return;
         }
 
         final GeckoPreferences activity = (GeckoPreferences) getActivity();
         if (activity.isMultiPane()) {
-            // In a multi-pane activity, the title is "Settings", and the action
-            // bar is along the top of the screen. We don't want to change those.
-            activity.showBreadCrumbs(newTitle, newTitle);
-            activity.switchToHeader(getHeader());
+            activity.trySwitchToHeader(getHeader());
             return;
         }
-
         Log.v(LOGTAG, "Setting activity title to " + newTitle);
         activity.setTitle(newTitle);
     }
 
     @Override
     public void onActivityCreated(Bundle savedInstanceState) {
         super.onActivityCreated(savedInstanceState);
         accountLoaderCallbacks = new AccountLoaderCallbacks();
--- a/mobile/android/base/java/org/mozilla/gecko/preferences/GeckoPreferences.java
+++ b/mobile/android/base/java/org/mozilla/gecko/preferences/GeckoPreferences.java
@@ -52,34 +52,32 @@ import org.json.JSONArray;
 import org.mozilla.gecko.AboutPages;
 import org.mozilla.gecko.AdjustConstants;
 import org.mozilla.gecko.AppConstants;
 import org.mozilla.gecko.BrowserApp;
 import org.mozilla.gecko.BrowserLocaleManager;
 import org.mozilla.gecko.DataReportingNotification;
 import org.mozilla.gecko.DynamicToolbar;
 import org.mozilla.gecko.EventDispatcher;
-import org.mozilla.gecko.Experiments;
 import org.mozilla.gecko.GeckoApplication;
 import org.mozilla.gecko.GeckoProfile;
 import org.mozilla.gecko.GeckoSharedPrefs;
 import org.mozilla.gecko.LocaleManager;
 import org.mozilla.gecko.Locales;
 import org.mozilla.gecko.PrefsHelper;
 import org.mozilla.gecko.R;
 import org.mozilla.gecko.SnackbarBuilder;
 import org.mozilla.gecko.Telemetry;
 import org.mozilla.gecko.TelemetryContract;
 import org.mozilla.gecko.TelemetryContract.Method;
 import org.mozilla.gecko.db.BrowserContract.SuggestedSites;
 import org.mozilla.gecko.mma.MmaDelegate;
 import org.mozilla.gecko.permissions.Permissions;
 import org.mozilla.gecko.restrictions.Restrictable;
 import org.mozilla.gecko.restrictions.Restrictions;
-import org.mozilla.gecko.switchboard.SwitchBoard;
 import org.mozilla.gecko.tabqueue.TabQueueHelper;
 import org.mozilla.gecko.tabqueue.TabQueuePrompt;
 import org.mozilla.gecko.updater.UpdateService;
 import org.mozilla.gecko.updater.UpdateServiceHelper;
 import org.mozilla.gecko.util.BundleEventListener;
 import org.mozilla.gecko.util.ContextUtils;
 import org.mozilla.gecko.util.EventCallback;
 import org.mozilla.gecko.util.GeckoBundle;
@@ -461,19 +459,27 @@ public class GeckoPreferences
                 }
             }
 
             mHeaders = target;
         }
     }
 
     @TargetApi(11)
-    public void switchToHeader(int id) {
+    public void trySwitchToHeader(int id) {
+        /**
+         * Can't switch to an unknown header.
+         * See {@link GeckoPreferenceFragment#getHeader()}
+         */
+        if (id == GeckoPreferenceFragment.HEADER_ID_UNDEFINED) {
+            return;
+        }
+
+        // Can't switch to a header if there are no headers!
         if (mHeaders == null) {
-            // Can't switch to a header if there are no headers!
             return;
         }
 
         for (Header header : mHeaders) {
             if (header.id == id) {
                 switchToHeader(header);
                 return;
             }