Bug 1304777 - Part 2: Fix panel removal migration to not discard DEFAULT flag if set r=sebastian
authorAndrzej Hunt <ahunt@mozilla.com>
Mon, 26 Sep 2016 13:16:03 -0700
changeset 315358 aa465e8e37238688535692637be905d8e54aeb13
parent 315357 03843beb3dfdaafdda4cf6dc07afa3cf07097ec2
child 315359 a718fc141d0a3efa8db4d09969b9c606a1d10e5c
push id30748
push usercbook@mozilla.com
push dateWed, 28 Sep 2016 13:53:19 +0000
treeherdermozilla-central@8c84b7618840 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssebastian
bugs1304777
milestone52.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1304777 - Part 2: Fix panel removal migration to not discard DEFAULT flag if set r=sebastian If the replacement panel was DEFAULT, then the migration should retain the DEFAULT flag. By trying to ensure visibility of the default panel, we accidentally discarded the DEFAULT flag. This patch provides a minimal fix for not discarding the default flag. It might be better to rearchitect the whole method, but a minimal solution is preferred for now since this requires uplifting to beta. MozReview-Commit-ID: EkbDD7pipgJ
mobile/android/base/java/org/mozilla/gecko/home/HomeConfigPrefsBackend.java
--- a/mobile/android/base/java/org/mozilla/gecko/home/HomeConfigPrefsBackend.java
+++ b/mobile/android/base/java/org/mozilla/gecko/home/HomeConfigPrefsBackend.java
@@ -248,46 +248,63 @@ public class HomeConfigPrefsBackend impl
      *                     otherwise only if we turn it into the new default panel.
      * @return new array of updated JSON panels
      * @throws JSONException
      */
     private static JSONArray removePanel(Context context, JSONArray jsonPanels,
                                          PanelType panelToRemove, PanelType replacementPanel, boolean alwaysUnhide) throws JSONException {
         boolean wasDefault = false;
         int replacementPanelIndex = -1;
+        boolean replacementWasDefault = false;
 
         // JSONArrary doesn't provide remove() for API < 19, therefore we need to manually copy all
         // the items we don't want deleted into a new array.
         final JSONArray newJSONPanels = new JSONArray();
 
         for (int i = 0; i < jsonPanels.length(); i++) {
             final JSONObject panelJSON = jsonPanels.getJSONObject(i);
             final PanelConfig panelConfig = new PanelConfig(panelJSON);
 
             if (panelConfig.getType() == panelToRemove) {
                 // If this panel was the default we'll need to assign a new default:
                 wasDefault = panelConfig.isDefault();
             } else {
                 if (panelConfig.getType() == replacementPanel) {
                     replacementPanelIndex = newJSONPanels.length();
+                    if (panelConfig.isDefault()) {
+                        replacementWasDefault = true;
+                    }
                 }
 
                 newJSONPanels.put(panelJSON);
             }
         }
 
         // Unless alwaysUnhide is true, we make the replacement panel visible only if it is going
         // to be the new default panel, since a hidden default panel doesn't make sense.
         // This is to allow preserving the behaviour of the original reading list migration function.
         if (wasDefault || alwaysUnhide) {
             final JSONObject replacementPanelConfig;
             if (wasDefault) {
+                // If the removed panel was the default, the replacement has to be made the new default
                 replacementPanelConfig = createBuiltinPanelConfig(context, replacementPanel, EnumSet.of(PanelConfig.Flags.DEFAULT_PANEL)).toJSON();
             } else {
-                replacementPanelConfig = createBuiltinPanelConfig(context, replacementPanel).toJSON();
+                final EnumSet<HomeConfig.PanelConfig.Flags> flags;
+                if (replacementWasDefault) {
+                    // However if the replacement panel was already default, we need to preserve it's default status
+                    // (By rewriting the PanelConfig, we lose all existing flags, so we need to make sure desired
+                    // flags are retained - in this case there's only DEFAULT_PANEL, which is mutually
+                    // exclusive with the DISABLE_PANEL case).
+                    flags = EnumSet.of(PanelConfig.Flags.DEFAULT_PANEL);
+                } else {
+                    flags = EnumSet.noneOf(PanelConfig.Flags.class);
+                }
+
+                // The panel is visible since we don't set Flags.DISABLED_PANEL.
+                replacementPanelConfig = createBuiltinPanelConfig(context, replacementPanel, flags).toJSON();
             }
 
             if (replacementPanelIndex != -1) {
                 newJSONPanels.put(replacementPanelIndex, replacementPanelConfig);
             } else {
                 newJSONPanels.put(replacementPanelConfig);
             }
         }