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 498884 e9dfaf584e37774a7cfc24242ebb02c11539be97
parent 498883 baa2b88b3ed1cd04fbae4d7ec20124c4ee2b87e6
child 498885 8597671d8dbd7e63706957ef853f1d8b7cb50dfd
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersJanH, jchen
bugs1478735
milestone64.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 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>