bug 1248460 pin during SSTabRestoring r=kmag draft
authorAndy McKay <amckay@mozilla.com>
Mon, 29 Feb 2016 13:45:36 -0800
changeset 335576 93c659ad68e171e7843093f9308677eac66c024f
parent 335075 5e0140b6d11821e0c2a2de25bc5431783f03380a
child 336288 ebe5512e508966480a445231bc5728fb67313846
push id11819
push userbmo:amckay@mozilla.com
push dateMon, 29 Feb 2016 21:53:24 +0000
reviewerskmag
bugs1248460
milestone47.0a1
bug 1248460 pin during SSTabRestoring r=kmag MozReview-Commit-ID: A88UnreihK7
browser/components/extensions/ext-tabs.js
browser/components/extensions/test/browser/browser_ext_tabs_duplicate.js
--- a/browser/components/extensions/ext-tabs.js
+++ b/browser/components/extensions/ext-tabs.js
@@ -779,22 +779,36 @@ extensions.registerSchemaAPI("tabs", nul
       duplicate: function(tabId) {
         let tab = TabManager.getTab(tabId);
         if (!tab) {
           return Promise.reject({message: `Invalid tab ID: ${tabId}`});
         }
 
         let gBrowser = tab.ownerDocument.defaultView.gBrowser;
         let newTab = gBrowser.duplicateTab(tab);
-        gBrowser.moveTabTo(newTab, tab._tPos + 1);
-        gBrowser.selectTabAtIndex(newTab._tPos);
 
         return new Promise(resolve => {
+          // We need to use SSTabRestoring because any attributes set before
+          // are ignored. SSTabRestored is too late and results in a jump in
+          // the UI. See http://bit.ly/session-store-api for more information.
+          newTab.addEventListener("SSTabRestoring", function listener() {
+            // As the tab is restoring, move it to the correct position.
+            newTab.removeEventListener("SSTabRestoring", listener);
+            // Pinned tabs that are duplicated are inserted
+            // after the existing pinned tab and pinned.
+            if (tab.pinned) {
+              gBrowser.pinTab(newTab);
+            }
+            gBrowser.moveTabTo(newTab, tab._tPos + 1);
+          });
+
           newTab.addEventListener("SSTabRestored", function listener() {
+            // Once it has been restored, select it and return the promise.
             newTab.removeEventListener("SSTabRestored", listener);
+            gBrowser.selectedTab = newTab;
             return resolve(TabManager.convert(extension, newTab));
           });
         });
       },
     },
   };
   return self;
 });
--- a/browser/components/extensions/test/browser/browser_ext_tabs_duplicate.js
+++ b/browser/components/extensions/test/browser/browser_ext_tabs_duplicate.js
@@ -30,15 +30,47 @@ add_task(function* testDuplicateTab() {
       });
     },
   });
 
   yield extension.startup();
   yield extension.awaitFinish("tabs.duplicate");
   yield extension.unload();
 
-  while (window.gBrowser.tabs.length > 1) {
-    let tab = window.gBrowser.tabs[0];
-    if (tab.linkedBrowser.currentURI.spec === "http://example.net/") {
-      yield BrowserTestUtils.removeTab(tab);
-    }
+  while (gBrowser.tabs[0].linkedBrowser.currentURI.spec === "http://example.net/") {
+    yield BrowserTestUtils.removeTab(gBrowser.tabs[0]);
   }
 });
+
+add_task(function* testDuplicatePinnedTab() {
+  let tab = yield BrowserTestUtils.openNewForegroundTab(gBrowser, "http://example.net/");
+  gBrowser.pinTab(tab);
+
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      "permissions": ["tabs"],
+    },
+
+    background: function() {
+      browser.tabs.query({
+        lastFocusedWindow: true,
+      }, function(tabs) {
+        // Duplicate the pinned tab, example.net.
+        browser.tabs.duplicate(tabs[0].id, (tab) => {
+          browser.test.assertEq("http://example.net/", tab.url);
+          // Should be the second tab, next to the one duplicated.
+          browser.test.assertEq(1, tab.index);
+          // Should be pinned.
+          browser.test.assertTrue(tab.pinned);
+          browser.test.notifyPass("tabs.duplicate.pinned");
+        });
+      });
+    },
+  });
+
+  yield extension.startup();
+  yield extension.awaitFinish("tabs.duplicate.pinned");
+  yield extension.unload();
+
+  while (gBrowser.tabs[0].linkedBrowser.currentURI.spec === "http://example.net/") {
+    yield BrowserTestUtils.removeTab(gBrowser.tabs[0]);
+  }
+});