Bug 1238313: Part 1 - Implement tabs.onMoved event. r?billm draft
authorKris Maglione <maglione.k@gmail.com>
Thu, 11 Feb 2016 14:32:58 -0800
changeset 330488 380fc3cb37d349806de2f43a6a48e9562b6836ef
parent 330477 27fac3f179d57d469053a3dc65c7749f7be95989
child 330489 5f84d1778a83899cc04b37db839153dee8dfb86e
push id10771
push usermaglione.k@gmail.com
push dateThu, 11 Feb 2016 23:39:49 +0000
reviewersbillm
bugs1238313
milestone47.0a1
Bug 1238313: Part 1 - Implement tabs.onMoved event. r?billm MozReview-Commit-ID: F2GoU1fzj4s
browser/components/extensions/ext-tabs.js
browser/components/extensions/test/browser/browser_ext_tabs_events.js
--- a/browser/components/extensions/ext-tabs.js
+++ b/browser/components/extensions/ext-tabs.js
@@ -245,16 +245,59 @@ extensions.registerSchemaAPI("tabs", nul
         return () => {
           WindowListManager.removeCloseListener(windowListener);
           AllWindowEvents.removeListener("TabClose", tabListener);
         };
       }).api(),
 
       onReplaced: ignoreEvent(context, "tabs.onReplaced"),
 
+      onMoved: new EventManager(context, "tabs.onMoved", fire => {
+        // There are certain circumstances where we need to ignore a move event.
+        //
+        // Namely, the first time the tab is moved after it's created, we need
+        // to report the final position as the initial position in the tab's
+        // onAttached or onCreated event. This is because most tabs are inserted
+        // in a temporary location and then moved after the TabOpen event fires,
+        // which generates a TabOpen event followed by a TabMove event, which
+        // does not match the contract of our API.
+        let ignoreNextMove = new WeakSet();
+
+        let openListener = event => {
+          ignoreNextMove.add(event.target);
+          // Remove the tab from the set on the next tick, since it will already
+          // have been moved by then.
+          Promise.resolve().then(() => {
+            ignoreNextMove.delete(event.target);
+          });
+        };
+
+        let moveListener = event => {
+          let tab = event.originalTarget;
+
+          if (ignoreNextMove.has(tab)) {
+            ignoreNextMove.delete(tab);
+            return;
+          }
+
+          fire(TabManager.getId(tab), {
+            windowId: WindowManager.getId(tab.ownerDocument.defaultView),
+            fromIndex: event.detail,
+            toIndex: tab._tPos,
+          });
+        };
+
+        AllWindowEvents.addListener("TabMove", moveListener);
+        AllWindowEvents.addListener("TabOpen", openListener);
+        return () => {
+          AllWindowEvents.removeListener("TabMove", moveListener);
+          AllWindowEvents.removeListener("TabOpen", openListener);
+        };
+      }).api(),
+
       onUpdated: new EventManager(context, "tabs.onUpdated", fire => {
         function sanitize(extension, changeInfo) {
           let result = {};
           let nonempty = false;
           for (let prop in changeInfo) {
             if ((prop != "favIconUrl" && prop != "url") || extension.hasPermission("tabs")) {
               nonempty = true;
               result[prop] = changeInfo[prop];
--- a/browser/components/extensions/test/browser/browser_ext_tabs_events.js
+++ b/browser/components/extensions/test/browser/browser_ext_tabs_events.js
@@ -16,16 +16,20 @@ add_task(function* testTabEvents() {
     browser.tabs.onDetached.addListener((tabId, info) => {
       events.push(Object.assign({ type: "onDetached", tabId }, info));
     });
 
     browser.tabs.onRemoved.addListener((tabId, info) => {
       events.push(Object.assign({ type: "onRemoved", tabId }, info));
     });
 
+    browser.tabs.onMoved.addListener((tabId, info) => {
+      events.push(Object.assign({ type: "onMoved", tabId }, info));
+    });
+
     function expectEvents(names) {
       browser.test.log(`Expecting events: ${names.join(", ")}`);
 
       return new Promise(resolve => {
         setTimeout(resolve, 0);
       }).then(() => {
         browser.test.assertEq(names.length, events.length, "Got expected number of events");
         for (let [i, name] of names.entries()) {
@@ -44,34 +48,48 @@ add_task(function* testTabEvents() {
       let windowId = windows[0].id;
       let otherWindowId = windows[1].id;
       let initialTab;
 
       return expectEvents(["onCreated"]).then(([created]) => {
         initialTab = created.tab;
 
         browser.test.log("Create tab in window 1");
-        return browser.tabs.create({ windowId });
+        return browser.tabs.create({ windowId, index: 0 });
       }).then(tab => {
         let oldIndex = tab.index;
+        browser.test.assertEq(0, oldIndex, "Tab has the expected index");
 
         return expectEvents(["onCreated"]).then(([created]) => {
           browser.test.assertEq(tab.id, created.tab.id, "Got expected tab ID");
+          browser.test.assertEq(oldIndex, created.tab.index, "Got expected tab index");
 
           browser.test.log("Move tab to window 2");
           return browser.tabs.move([tab.id], { windowId: otherWindowId, index: 0 });
         }).then(() => {
           return expectEvents(["onDetached", "onAttached"]);
         }).then(([detached, attached]) => {
           browser.test.assertEq(oldIndex, detached.oldPosition, "Expected old index");
           browser.test.assertEq(windowId, detached.oldWindowId, "Expected old window ID");
 
           browser.test.assertEq(0, attached.newPosition, "Expected new index");
           browser.test.assertEq(otherWindowId, attached.newWindowId, "Expected new window ID");
 
+          browser.test.log("Move tab within the same window");
+          return browser.tabs.move([tab.id], { index: 1 });
+        }).then(([moved]) => {
+          browser.test.assertEq(1, moved.index, "Expected new index");
+
+          return expectEvents(["onMoved"]);
+        }).then(([moved]) => {
+          browser.test.assertEq(tab.id, moved.tabId, "Expected tab ID");
+          browser.test.assertEq(0, moved.fromIndex, "Expected old index");
+          browser.test.assertEq(1, moved.toIndex, "Expected new index");
+          browser.test.assertEq(otherWindowId, moved.windowId, "Expected window ID");
+
           browser.test.log("Remove tab");
           return browser.tabs.remove(tab.id);
         }).then(() => {
           return expectEvents(["onRemoved"]);
         }).then(([removed]) => {
           browser.test.assertEq(tab.id, removed.tabId, "Expected removed tab ID");
           browser.test.assertEq(otherWindowId, removed.windowId, "Expected removed tab window ID");
           // Note: We want to test for the actual boolean value false here.