Bug 1346004 - Part 3 - getNextTab() should fall back to browsing-type tabs if no tabs of the same type are available. r?sebastian draft
authorJan Henning <jh+bugzilla@buttercookie.de>
Sat, 11 Mar 2017 20:57:54 +0100
changeset 502491 2ab97b16955c8f43b5d41582c25b22a81fc1112f
parent 502490 355a2c876b8affd1b09ac97a40905a325b04096b
child 502492 52e8d1a0f89ce96b553744c1203e3de5782038f4
push id50298
push usermozilla@buttercookie.de
push dateTue, 21 Mar 2017 21:08:11 +0000
reviewerssebastian
bugs1346004
milestone55.0a1
Bug 1346004 - Part 3 - getNextTab() should fall back to browsing-type tabs if no tabs of the same type are available. r?sebastian We always want at least one browsing type tab open to match current behaviour and assumptions built into the app, but the same doesn't hold true for other tab types. A CustomTabActivity instance e.g. only holds a single tab, so the lifetime of the tab is tied to the lifetime of the CustomTabActivity. If we're exiting the activity, we just want to close the corresponding tab without opening a new replacement for it. Since we have to select some other tab instead in that case (so the selected tab in the tab list and the selected tab in Gecko remain in sync and so we always have a selected tab), getNextTab() therefore has to fall back to browsing-type tabs as a default if no other tabs of the same type are available. MozReview-Commit-ID: IpvINlu5Qq9
mobile/android/base/java/org/mozilla/gecko/Tabs.java
--- a/mobile/android/base/java/org/mozilla/gecko/Tabs.java
+++ b/mobile/android/base/java/org/mozilla/gecko/Tabs.java
@@ -443,36 +443,56 @@ public class Tabs implements BundleEvent
             return selectedTab;
 
         boolean getPrivate = tab.isPrivate();
         TabType type = tab.getType();
         Tab nextTab = getNextTabFrom(tab, getPrivate, type);
         if (nextTab == null)
             nextTab = getPreviousTabFrom(tab, getPrivate, type);
         if (nextTab == null && getPrivate) {
-            // If there are no private tabs remaining, get the last normal tab
-            Tab lastTab = mOrder.get(mOrder.size() - 1);
-            if (!lastTab.isPrivate()) {
-                nextTab = lastTab;
-            } else {
-                nextTab = getPreviousTabFrom(lastTab, false, type);
-            }
+            // If there are no private tabs remaining, get the last normal tab.
+            nextTab = getFallbackNextTab(type);
+        }
+        if (nextTab == null && type != TabType.BROWSING) {
+            // If there are no non-private tabs of the same type remaining,
+            // fall back to TabType.BROWSING.
+            nextTab = getFallbackNextTab(TabType.BROWSING);
         }
 
         Tab parent = getTab(tab.getParentId());
         if (parent != null) {
             // If the next tab is a sibling, switch to it. Otherwise go back to the parent.
             if (nextTab != null && nextTab.getParentId() == tab.getParentId())
                 return nextTab;
             else
                 return parent;
         }
         return nextTab;
     }
 
+    /**
+     * Normally, {@link #getNextTab(Tab)} will attempt to find a tab of the same privacy mode and
+     * {@link TabType} as the currently selected tab. If no such tab exists, we will first fall back
+     * to non-private tabs if the current tab is a private tab. If we can't find any non-private
+     * tabs of the same type, we then start looking for any non-private {@link TabType#BROWSING} tabs.
+     *
+     * @param type The {@link TabType} of tab to be searched.
+     * @return A non-private tab of the type specified or null if none could be found.
+     */
+    private Tab getFallbackNextTab(TabType type) {
+        Tab nextTab;
+        Tab lastTab = mOrder.get(mOrder.size() - 1);
+        if (!lastTab.isPrivate() && lastTab.getType() == type) {
+            nextTab = lastTab;
+        } else {
+            nextTab = getPreviousTabFrom(lastTab, false, type);
+        }
+        return nextTab;
+    }
+
     public Iterable<Tab> getTabsInOrder() {
         return mOrder;
     }
 
     /**
      * @return the current GeckoApp instance, or throws if
      *         we aren't correctly initialized.
      */