Bug 1478735 Download with target=_blank switches to other tab target=_blank r=JanH,jchen
authorAndrei Lazar <andrei.a.lazar@softvision.ro>
Wed, 10 Oct 2018 09:49:07 +0000
changeset 488837 e9dfaf584e37774a7cfc24242ebb02c11539be97
parent 488836 baa2b88b3ed1cd04fbae4d7ec20124c4ee2b87e6
child 488838 8597671d8dbd7e63706957ef853f1d8b7cb50dfd
push id246
push userfmarier@mozilla.com
push dateSat, 13 Oct 2018 00:15:40 +0000
reviewersJanH, jchen
bugs1478735
milestone64.0a1
Bug 1478735 Download with target=_blank switches to other tab target=_blank r=JanH,jchen Download with target=_blank now switches to the parent tab. Differential Revision: https://phabricator.services.mozilla.com/D6928
mobile/android/base/java/org/mozilla/gecko/Tabs.java
mobile/android/tests/browser/chrome/test_session_parentid.html
--- a/mobile/android/base/java/org/mozilla/gecko/Tabs.java
+++ b/mobile/android/base/java/org/mozilla/gecko/Tabs.java
@@ -62,16 +62,17 @@ public class Tabs implements BundleEvent
     private volatile CopyOnWriteArrayList<Tab> mOrder = new CopyOnWriteArrayList<Tab>();
 
     // A cache that maps a tab ID to an mOrder tab position.  All access should be synchronized.
     private final TabPositionCache tabPositionCache = new TabPositionCache();
 
     // All writes to mSelectedTab must be synchronized on the Tabs instance.
     // In general, it's preferred to always use selectTab()).
     private volatile Tab mSelectedTab;
+    private volatile int mPreviouslySelectedTabId = INVALID_TAB_ID;
 
     // All accesses to mTabs must be synchronized on the Tabs instance.
     private final HashMap<Integer, Tab> mTabs = new HashMap<Integer, Tab>();
 
     private AccountManager mAccountManager;
     private OnAccountsUpdateListener mAccountListener;
 
     public static final int LOADURL_NONE         = 0;
@@ -332,16 +333,17 @@ public class Tabs implements BundleEvent
         }
 
         mSelectedTab = tab;
         mSelectedTab.updatePageAction();
 
         notifyListeners(tab, TabEvents.SELECTED);
 
         if (oldTab != null) {
+            mPreviouslySelectedTabId = oldTab.getId();
             notifyListeners(oldTab, TabEvents.UNSELECTED);
         }
 
         // Pass a message to Gecko to update tab state in BrowserApp.
         final GeckoBundle data = new GeckoBundle(1);
         data.putInt("id", tab.getId());
         mEventDispatcher.dispatch("Tab:Selected", data);
         EventDispatcher.getInstance().dispatch("Tab:Selected", data);
@@ -487,25 +489,22 @@ public class Tabs implements BundleEvent
             Tab lastTab = mOrder.get(mOrder.size() - 1);
             if (!lastTab.isPrivate()) {
                 nextTab = lastTab;
             } else {
                 nextTab = getPreviousTabFrom(lastTab, false);
             }
         }
 
-        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;
+        final Tab parentTab = getTab(tab.getParentId());
+        if (tab.getParentId() == mPreviouslySelectedTabId && tab.getParentId() != INVALID_TAB_ID && parentTab != null) {
+            return parentTab;
+        } else {
+            return nextTab;
         }
-        return nextTab;
     }
 
     public Iterable<Tab> getTabsInOrder() {
         return mOrder;
     }
 
     /**
      * @return the current GeckoApp instance, or throws if
--- a/mobile/android/tests/browser/chrome/test_session_parentid.html
+++ b/mobile/android/tests/browser/chrome/test_session_parentid.html
@@ -51,48 +51,70 @@ https://bugzilla.mozilla.org/show_bug.cg
 
   const url = "data:text/html;charset=utf-8,It%20was%20a%20dark%20and%20stormy%20night.";
 
   add_task(async function test_sessionStoreParentId() {
     SimpleTest.registerCleanupFunction(function() {
       cleanupTabs();
     });
 
-    // First, check that passing a parent tab ID works as expected
+    // Initialize parent tab
     tabParent = BrowserApp.addTab(url, { selected: true });
-    await promiseBrowserEvent(tabParent.browser, "DOMContentLoaded");
+    await promiseTabEvent(tabParent.browser, "DOMContentLoaded");
+    is(BrowserApp.selectedTab, tabParent, "tabParent is selected");
 
+    // test case #1
     // Open tabs without passing a parent tab ID
     tabChild1 = BrowserApp.addTab(url, { selected: false });
+    await promiseTabEvent(tabChild1.browser, "DOMContentLoaded");
+    is(BrowserApp.selectedTab, tabParent, "tabParent is selected");
+
     tabChild2 = BrowserApp.addTab(url, { selected: true });
     await promiseTabEvent(BrowserApp.deck, "TabSelect");
     is(BrowserApp.selectedTab, tabChild2, "2nd child tab is selected");
 
     // After closing that tab, its neighbour should be selected
     BrowserApp.closeTab(tabChild2);
     tabChild2 = null;
-    await promiseTabEvent(BrowserApp.deck, "TabSelect");
+    await promiseTabEvent(BrowserApp.deck, "TabClose");
     is(BrowserApp.selectedTab, tabChild1, "1st child tab is selected");
 
-    // Add a new tab and pass a parent tab ID this time
-    tabChild2 = BrowserApp.addTab(url, { selected: true, parentId: tabParent.id });
+    // test case #2
+    // Let's open a new tab, this time with a parent id and let's check if after closing it,
+    // the selected tab will be the neighbour since the parent was not selected
+    tabChild2 = BrowserApp.addTab(url, { selected: false, parentId: tabParent.id });
+    await promiseTabEvent(tabChild2.browser, "DOMContentLoaded");
+    is(BrowserApp.selectedTab, tabChild1, "1st child tab is still selected");
+
+    BrowserApp.selectTab(tabChild2);
     await promiseTabEvent(BrowserApp.deck, "TabSelect");
     is(BrowserApp.selectedTab, tabChild2, "2nd child tab is selected");
 
-    // After closing that tab, its parent should be selected
     BrowserApp.closeTab(tabChild2);
     tabChild2 = null;
-    await promiseTabEvent(BrowserApp.deck, "TabSelect");
-    is(BrowserApp.selectedTab, tabParent, "parent tab is selected");
-
-    // Reset selection and switch to the other child tab
-    BrowserApp.selectTab(tabChild1);
-    await promiseTabEvent(BrowserApp.deck, "TabSelect");
+    await promiseTabEvent(BrowserApp.deck, "TabClose");
     is(BrowserApp.selectedTab, tabChild1, "1st child tab is selected");
 
+    // test case #3
+    // This time we open a new tab with a parent id but this time the parent should be selected
+    // after closing since the parent was the previously selected tab
+    BrowserApp.selectTab(tabParent);
+    await promiseTabEvent(BrowserApp.deck, "TabSelect");
+    is(BrowserApp.selectedTab, tabParent, "tabParent is selected");
+
+    tabChild2 = BrowserApp.addTab(url, { selected: true, parentId: tabParent.id });
+    await promiseTabEvent(tabChild2.browser, "DOMContentLoaded");
+    is(BrowserApp.selectedTab, tabChild2, "2d child tab is selected");
+
+    BrowserApp.closeTab(tabChild2);
+    tabChild2 = null;
+    await promiseTabEvent(BrowserApp.deck, "TabClose");
+    is(BrowserApp.selectedTab, tabParent, "tabParent is selected");
+
+    // test case #4
     // Now check that this works even if the child tab is closed and subsequently restored
     tabChild2 = BrowserApp.addTab(url, { selected: false, parentId: tabParent.id });
     await promiseTabEvent(tabChild2.browser, "SSTabDataUpdated");
     BrowserApp.closeTab(tabChild2);
     await promiseTabEvent(tabChild2.browser, "SSTabCloseProcessed");
 
     // Restore the tab
     let closedTabData = ss.getClosedTabs(chromeWin)[0];
@@ -100,17 +122,17 @@ https://bugzilla.mozilla.org/show_bug.cg
     await promiseTabEvent(BrowserApp.deck, "TabSelect");
     tabChild2 = BrowserApp.getTabForBrowser(browser);
     is(BrowserApp.selectedTab, tabChild2, "restored 2nd child tab is selected");
 
     // After closing that tab, its parent should be selected
     BrowserApp.closeTab(tabChild2);
     tabChild2 = null;
     await promiseTabEvent(BrowserApp.deck, "TabSelect");
-    is(BrowserApp.selectedTab, tabParent, "parent tab is selected after restoring");
+    is(BrowserApp.selectedTab, tabParent, "tabParent is selected after restoring");
 
     cleanupTabs();
   });
 
   </script>
 </head>
 <body>
 <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1301160">Mozilla Bug 1301160</a>