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

--HG--
extra : moz-landing-system : lando

mobile/android/base/java/org/mozilla/gecko/Tabs.java

@@ -67,6 +67,7 @@ public class Tabs implements BundleEventListener {
     // 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>();
@@ -337,6 +338,7 @@ public class Tabs implements BundleEventListener {
         notifyListeners(tab, TabEvents.SELECTED);
 
         if (oldTab != null) {
+            mPreviouslySelectedTabId = oldTab.getId();
             notifyListeners(oldTab, TabEvents.UNSELECTED);
         }
 
@@ -492,15 +494,12 @@ public class Tabs implements BundleEventListener {
             }
         }
 
-        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() {

mobile/android/tests/browser/chrome/test_session_parentid.html

@@ -56,12 +56,17 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=1301160
       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");
@@ -69,25 +74,42 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=1301160
     // 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");
@@ -105,7 +127,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=1301160
     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();
   });