Bug 1257008 - Don't collapse the tab list when right-clicking on a device in the Synced Tabs sidebar. r=markh draft
authorKit Cambridge <kcambridge@mozilla.com>
Tue, 15 Mar 2016 16:56:52 -0700
changeset 348303 5a5fc7770e168f4cbf6a9fd4c403f83be3cf6b58
parent 348302 c599e46dbaf470314c7902a0f0ffbd9cd35b23b1
child 348304 c08d656c3b8320b7ba3dad8179c1ace628b1deaf
push id14805
push userbmo:jacheng@mozilla.com
push dateThu, 07 Apr 2016 07:37:41 +0000
reviewersmarkh
bugs1257008
milestone48.0a1
Bug 1257008 - Don't collapse the tab list when right-clicking on a device in the Synced Tabs sidebar. r=markh MozReview-Commit-ID: JsaWLfMOYyC
browser/components/syncedtabs/TabListComponent.js
browser/components/syncedtabs/TabListView.js
browser/components/syncedtabs/test/browser/browser_sidebar_syncedtabslist.js
browser/components/syncedtabs/test/xpcshell/test_TabListComponent.js
--- a/browser/components/syncedtabs/TabListComponent.js
+++ b/browser/components/syncedtabs/TabListComponent.js
@@ -76,21 +76,18 @@ TabListComponent.prototype = {
   onFilterFocus() {
     this._store.focusInput();
   },
 
   onFilterBlur() {
     this._store.blurInput();
   },
 
-  onSelectRow(position, id) {
+  onSelectRow(position) {
     this._store.selectRow(position[0], position[1]);
-    if (id) {
-      this._store.toggleBranch(id);
-    }
   },
 
   onMoveSelectionDown() {
     this._store.moveSelectionDown();
   },
 
   onMoveSelectionUp() {
     this._store.moveSelectionUp();
--- a/browser/components/syncedtabs/TabListView.js
+++ b/browser/components/syncedtabs/TabListView.js
@@ -259,21 +259,18 @@ TabListView.prototype = {
       }
     }
 
     if (event.target.classList.contains("item-twisty-container")) {
       this.props.onToggleBranch(itemNode.dataset.id);
       return;
     }
 
-    this._selectRow(itemNode);
-  },
-
-  _selectRow(itemNode) {
-    this.props.onSelectRow(this._getSelectionPosition(itemNode), itemNode.dataset.id);
+    let position = this._getSelectionPosition(itemNode);
+    this.props.onSelectRow(position);
   },
 
   /**
    * Handle a keydown event on the list box.
    * @param {Event} event - Triggering event.
    */
   onKeyDown(event) {
     if (event.keyCode == this._window.KeyEvent.DOM_VK_DOWN) {
@@ -452,17 +449,18 @@ TabListView.prototype = {
   handleContextMenu(event) {
     let menu;
 
     if (event.target == this.tabsFilter) {
       menu = getTabsFilterContextMenu(this._window);
     } else {
       let itemNode = this._findParentItemNode(event.target);
       if (itemNode) {
-        this._selectRow(itemNode);
+        let position = this._getSelectionPosition(itemNode);
+        this.props.onSelectRow(position);
       }
       menu = getContextMenu(this._window);
       this.adjustContextMenu(menu);
     }
 
     menu.openPopupAtScreen(event.screenX, event.screenY, true, event);
   },
 
--- a/browser/components/syncedtabs/test/browser/browser_sidebar_syncedtabslist.js
+++ b/browser/components/syncedtabs/test/browser/browser_sidebar_syncedtabslist.js
@@ -359,16 +359,17 @@ function checkItem(node, item) {
     Assert.equal(node.dataset.id, item.id,
       "Node's ID should match item ID");
   }
 }
 
 function* testContextMenu(syncedTabsDeckComponent, contextSelector, triggerSelector, menuSelectors) {
   let contextMenu = document.querySelector(contextSelector);
   let triggerElement = syncedTabsDeckComponent._window.document.querySelector(triggerSelector);
+  let isClosed = triggerElement.classList.contains("closed");
 
   let promisePopupShown = BrowserTestUtils.waitForEvent(contextMenu, "popupshown");
 
   let chromeWindow = triggerElement.ownerDocument.defaultView.top;
   let rect = triggerElement.getBoundingClientRect();
   let contentRect = chromeWindow.SidebarUI.browser.getBoundingClientRect();
   // The offsets in `rect` are relative to the content window, but
   // `synthesizeMouseAtPoint` calls `nsIDOMWindowUtils.sendMouseEvent`,
@@ -378,16 +379,18 @@ function* testContextMenu(syncedTabsDeck
   let offsetX = contentRect.x + rect.x + (rect.width / 2);
   let offsetY = contentRect.y + rect.y + (rect.height / 4);
 
   yield EventUtils.synthesizeMouseAtPoint(offsetX, offsetY, {
     type: "contextmenu",
     button: 2,
   }, chromeWindow);
   yield promisePopupShown;
+  is(triggerElement.classList.contains("closed"), isClosed,
+    "Showing the context menu shouldn't toggle the tab list");
   checkChildren(contextMenu, menuSelectors);
 
   let promisePopupHidden = BrowserTestUtils.waitForEvent(contextMenu, "popuphidden");
   contextMenu.hidePopup();
   yield promisePopupHidden;
 }
 
 function checkChildren(node, selectors) {
--- a/browser/components/syncedtabs/test/xpcshell/test_TabListComponent.js
+++ b/browser/components/syncedtabs/test/xpcshell/test_TabListComponent.js
@@ -95,31 +95,30 @@ add_task(function* testActions() {
   component.onFilterFocus();
   Assert.ok(store.focusInput.called);
 
   sinon.stub(store, "blurInput");
   component.onFilterBlur();
   Assert.ok(store.blurInput.called);
 
   sinon.stub(store, "selectRow");
-  sinon.stub(store, "toggleBranch");
-  component.onSelectRow([-1, -1], "foo-id");
+  component.onSelectRow([-1, -1]);
   Assert.ok(store.selectRow.calledWith(-1, -1));
-  Assert.ok(store.toggleBranch.calledWith("foo-id"));
 
   sinon.stub(store, "moveSelectionDown");
   component.onMoveSelectionDown();
   Assert.ok(store.moveSelectionDown.called);
 
   sinon.stub(store, "moveSelectionUp");
   component.onMoveSelectionUp();
   Assert.ok(store.moveSelectionUp.called);
 
+  sinon.stub(store, "toggleBranch");
   component.onToggleBranch("foo-id");
-  Assert.ok(store.toggleBranch.secondCall.calledWith("foo-id"));
+  Assert.ok(store.toggleBranch.calledWith("foo-id"));
 
   sinon.spy(windowMock.top.PlacesCommandHook, "bookmarkLink");
   component.onBookmarkTab("uri", "title");
   Assert.equal(windowMock.top.PlacesCommandHook.bookmarkLink.args[0][1], "uri");
   Assert.equal(windowMock.top.PlacesCommandHook.bookmarkLink.args[0][2], "title");
 
   sinon.spy(windowMock, "openUILinkIn");
   component.onOpenTab("uri", "where", "params");