Bug 1462594 - Allow accessing all Settings menus on tablets; r=mcomella, a=RyanVM
authorPetru Lingurar <petru.lingurar@softvision.ro>
Thu, 24 May 2018 14:09:22 +0300
changeset 473550 55923d75814212a1e137e17fe90962bdc5bfcd99
parent 473549 3607d8d3561b254246d7a9f7298ee3fafd89c28f
child 473551 251c38d653d08ec7c7b13a60a70d57283f29c65e
push id1728
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:12:27 +0000
treeherdermozilla-release@c296fde26f5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmcomella, RyanVM
bugs1462594
milestone61.0
Bug 1462594 - Allow accessing all Settings menus on tablets; r=mcomella, a=RyanVM 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
@@ -457,19 +457,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;
             }