Bug 1494046 - Improve how the folder icon is selected on the folder list in the bookmark popup window. r=mak
☠☠ backed out by 3fad68f9222d ☠ ☠
authorMark Banner <standard8@mozilla.com>
Thu, 04 Oct 2018 08:54:25 +0000
changeset 495273 2f1229c179d303c2df8c4fa7b05d7a4662f73422
parent 495272 ae757626c524f74920547bc09afb4188752fcff4
child 495274 cc56daee035f30628eb6b56706ea7b5fb8ede0ff
push id9984
push userffxbld-merge
push dateMon, 15 Oct 2018 21:07:35 +0000
treeherdermozilla-beta@183d27ea8570 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1494046
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 1494046 - Improve how the folder icon is selected on the folder list in the bookmark popup window. r=mak Change to using an event listener to follow when the selection changes more accurately. Also switch to using GUIDs as they are better defined than indexes. Differential Revision: https://phabricator.services.mozilla.com/D7537
browser/components/places/content/editBookmark.js
browser/components/places/tests/browser/browser.ini
browser/components/places/tests/browser/browser_bookmarkProperties_folderSelection.js
browser/themes/linux/places/editBookmark.css
browser/themes/osx/places/editBookmark.css
browser/themes/windows/places/editBookmark.css
--- a/browser/components/places/content/editBookmark.js
+++ b/browser/components/places/content/editBookmark.js
@@ -400,29 +400,38 @@ var gEditItemOverlay = {
     var numberOfItems = Math.min(MAX_FOLDER_ITEM_IN_MENU_LIST,
                                  this._recentFolders.length);
     for (let i = 0; i < numberOfItems; i++) {
       await this._appendFolderItemToMenupopup(menupopup,
                                               this._recentFolders[i].guid,
                                               this._recentFolders[i].title);
     }
 
+    this._folderMenuList.addEventListener("select", this);
+    this._folderMenuListListenerAdded = true;
+
     let title = (await PlacesUtils.bookmarks.fetch(aSelectedFolderGuid)).title;
     var defaultItem = this._getFolderMenuItem(aSelectedFolderGuid, title);
     this._folderMenuList.selectedItem = defaultItem;
 
-    // Set a selectedIndex attribute to show special icons
-    this._folderMenuList.setAttribute("selectedIndex",
-                                      this._folderMenuList.selectedIndex);
-
     // Hide the folders-separator if no folder is annotated as recently-used
     this._element("foldersSeparator").hidden = (menupopup.children.length <= 6);
     this._folderMenuList.disabled = this.readOnly;
   },
 
+  _onFolderListSelected() {
+    // Set a selectedGuid attribute to show special icons
+    let folderGuid = this.selectedFolderGuid;
+    if (folderGuid) {
+      this._folderMenuList.setAttribute("selectedGuid", folderGuid);
+    } else {
+      this._folderMenuList.removeAttribute("selectedGuid");
+    }
+  },
+
   QueryInterface:
   ChromeUtils.generateQI([Ci.nsINavBookmarkObserver]),
 
   _element(aID) {
     return document.getElementById("editBMPanel_" + aID);
   },
 
   uninitPanel(aHideCollapsibleElements) {
@@ -438,16 +447,21 @@ var gEditItemOverlay = {
         this.toggleTagsSelector().catch(Cu.reportError);
     }
 
     if (this._observersAdded) {
       PlacesUtils.bookmarks.removeObserver(this);
       this._observersAdded = false;
     }
 
+    if (this._folderMenuListListenerAdded) {
+      this._folderMenuList.removeEventListener("select", this);
+      this._folderMenuListListenerAdded = false;
+    }
+
     this._setPaneInfo(null);
     this._firstEditedField = "";
   },
 
   get selectedFolderGuid() {
     return this._folderMenuList.selectedItem && this._folderMenuList.selectedItem.folderGuid;
   },
 
@@ -676,19 +690,16 @@ var gEditItemOverlay = {
   },
 
   async onFolderMenuListCommand(aEvent) {
     // Check for _paneInfo existing as the dialog may be closing but receiving
     // async updates from unresolved promises.
     if (!this._paneInfo) {
       return;
     }
-    // Set a selectedIndex attribute to show special icons
-    this._folderMenuList.setAttribute("selectedIndex",
-                                      this._folderMenuList.selectedIndex);
 
     if (aEvent.target.id == "editBMPanel_chooseFolderMenuItem") {
       // reset the selection back to where it was and expand the tree
       // (this menu-item is hidden when the tree is already visible
       let item = this._getFolderMenuItem(this._paneInfo.parentGuid,
                                          this._paneInfo.title);
       this._folderMenuList.selectedItem = item;
       // XXXmano HACK: setTimeout 100, otherwise focus goes back to the
@@ -865,16 +876,19 @@ var gEditItemOverlay = {
         if (item) {
           this.toggleItemCheckbox(item);
         }
       }
       break;
     case "unload":
       this.uninitPanel(false);
       break;
+    case "select":
+      this._onFolderListSelected();
+      break;
     }
   },
 
   toggleItemCheckbox(item) {
     // Update the tags field when items are checked/unchecked in the listbox
     let tags = this._getTagsArrayFromTagsInputField();
 
     let curTagIndex = tags.indexOf(item.label);
--- a/browser/components/places/tests/browser/browser.ini
+++ b/browser/components/places/tests/browser/browser.ini
@@ -34,16 +34,17 @@ support-files =
 skip-if = (verify && debug)
 [browser_bookmarkProperties_addLivemark.js]
 skip-if = (verify && debug && (os == 'linux' || os == 'win'))
 [browser_bookmarkProperties_bookmarkAllTabs.js]
 skip-if = (verify && debug && (os == 'win' || os == 'mac'))
 [browser_bookmarkProperties_cancel.js]
 [browser_bookmarkProperties_editFolder.js]
 [browser_bookmarkProperties_editTagContainer.js]
+[browser_bookmarkProperties_folderSelection.js]
 [browser_bookmarkProperties_no_user_actions.js]
 [browser_bookmarkProperties_newFolder.js]
 [browser_bookmarkProperties_readOnlyRoot.js]
 [browser_bookmarkProperties_remember_folders.js]
 [browser_bookmarksProperties.js]
 skip-if = (verify && debug && (os == 'win' || os == 'mac'))
 [browser_check_correct_controllers.js]
 [browser_click_bookmarks_on_toolbar.js]
new file mode 100644
--- /dev/null
+++ b/browser/components/places/tests/browser/browser_bookmarkProperties_folderSelection.js
@@ -0,0 +1,86 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/
+ */
+
+const TEST_URL = "about:robots";
+const bookmarkPanel = document.getElementById("editBookmarkPanel");
+let folders;
+
+add_task(async function setup() {
+  await PlacesUtils.bookmarks.eraseEverything();
+
+  bookmarkPanel.setAttribute("animate", false);
+
+  let oldTimeout = StarUI._autoCloseTimeout;
+  // Make the timeout something big, so it doesn't iteract badly with tests.
+  StarUI._autoCloseTimeout = 6000000;
+
+  let tab = await BrowserTestUtils.openNewForegroundTab({
+    gBrowser,
+    opening: TEST_URL,
+    waitForStateStop: true,
+  });
+
+  registerCleanupFunction(async () => {
+    StarUI._autoCloseTimeout = oldTimeout;
+    BrowserTestUtils.removeTab(tab);
+    bookmarkPanel.removeAttribute("animate");
+    await PlacesUtils.bookmarks.eraseEverything();
+  });
+});
+
+add_task(async function test_selectChoose() {
+  await clickBookmarkStar();
+
+  // Open folder selector.
+  let menuList = document.getElementById("editBMPanel_folderMenuList");
+  let folderTreeRow = document.getElementById("editBMPanel_folderTreeRow");
+
+  Assert.equal(menuList.label, PlacesUtils.getString("OtherBookmarksFolderTitle"),
+    "Should have the other bookmarks folder selected by default");
+  Assert.equal(menuList.getAttribute("selectedGuid"), PlacesUtils.bookmarks.unfiledGuid,
+    "Should have the correct default guid selected");
+  Assert.equal(folderTreeRow.collapsed, true,
+    "Should have the folder tree collapsed");
+
+  let promisePopup = BrowserTestUtils.waitForEvent(menuList.menupopup, "popupshown");
+  EventUtils.synthesizeMouseAtCenter(menuList, {}, window);
+  await promisePopup;
+
+  // Click the choose item.
+  EventUtils.synthesizeMouseAtCenter(document.getElementById("editBMPanel_chooseFolderMenuItem"), {}, window);
+
+  await TestUtils.waitForCondition(() => !folderTreeRow.collapsed,
+    "Should show the folder tree");
+
+  Assert.equal(menuList.getAttribute("selectedGuid"), PlacesUtils.bookmarks.unfiledGuid,
+    "Should still have the correct selected guid");
+  Assert.equal(menuList.label, PlacesUtils.getString("OtherBookmarksFolderTitle"),
+    "Should have kept the same menu label");
+
+  await hideBookmarksPanel();
+});
+
+
+add_task(async function test_selectBookmarksMenu() {
+  await clickBookmarkStar();
+
+  // Open folder selector.
+  let menuList = document.getElementById("editBMPanel_folderMenuList");
+
+  let promisePopup = BrowserTestUtils.waitForEvent(menuList.menupopup, "popupshown");
+  EventUtils.synthesizeMouseAtCenter(menuList, {}, window);
+  await promisePopup;
+
+  // Click the choose item.
+  EventUtils.synthesizeMouseAtCenter(document.getElementById("editBMPanel_bmRootItem"), {}, window);
+
+  await TestUtils.waitForCondition(
+    () => menuList.getAttribute("selectedGuid") == PlacesUtils.bookmarks.menuGuid,
+    "Should select the menu folder item");
+
+  Assert.equal(menuList.label, PlacesUtils.getString("BookmarksMenuFolderTitle"),
+    "Should have updated the menu label");
+
+  await hideBookmarksPanel();
+});
--- a/browser/themes/linux/places/editBookmark.css
+++ b/browser/themes/linux/places/editBookmark.css
@@ -59,22 +59,22 @@
   -moz-box-align: center;
   margin: 0 2px;
   min-width: 13px;
   min-height: 13px;
 }
 
 
 /* Bookmark panel dropdown menu items */
-#editBMPanel_folderMenuList[selectedIndex="0"],
+#editBMPanel_folderMenuList[selectedGuid="toolbar_____"],
 #editBMPanel_toolbarFolderItem {
   list-style-image: url("chrome://browser/skin/places/bookmarksToolbar.svg") !important;
 }
 
-#editBMPanel_folderMenuList[selectedIndex="1"],
+#editBMPanel_folderMenuList[selectedGuid="menu________"],
 #editBMPanel_bmRootItem {
   list-style-image: url("chrome://browser/skin/places/bookmarksMenu.svg") !important;
 }
 
-#editBMPanel_folderMenuList[selectedIndex="2"],
+#editBMPanel_folderMenuList[selectedGuid="unfiled_____"],
 #editBMPanel_unfiledRootItem {
   list-style-image: url("chrome://browser/skin/places/unfiledBookmarks.svg") !important;
 }
--- a/browser/themes/osx/places/editBookmark.css
+++ b/browser/themes/osx/places/editBookmark.css
@@ -64,22 +64,22 @@
 
 #editBMPanel_tagsSelector > richlistitem[checked="true"] > image {
   background-image: url("chrome://global/skin/checkbox/cbox-check.gif");
 }
 
 
 /* ----- BOOKMARK PANEL DROPDOWN MENU ITEMS ----- */
 
-#editBMPanel_folderMenuList[selectedIndex="0"],
+#editBMPanel_folderMenuList[selectedGuid="toolbar_____"],
 #editBMPanel_toolbarFolderItem {
   list-style-image: url("chrome://browser/skin/places/bookmarksToolbar.svg") !important;
 }
 
-#editBMPanel_folderMenuList[selectedIndex="1"],
+#editBMPanel_folderMenuList[selectedGuid="menu________"],
 #editBMPanel_bmRootItem {
   list-style-image: url("chrome://browser/skin/places/bookmarksMenu.svg") !important;
 }
 
-#editBMPanel_folderMenuList[selectedIndex="2"],
+#editBMPanel_folderMenuList[selectedGuid="unfiled_____"],
 #editBMPanel_unfiledRootItem {
   list-style-image: url("chrome://browser/skin/places/unfiledBookmarks.svg") !important;
 }
--- a/browser/themes/windows/places/editBookmark.css
+++ b/browser/themes/windows/places/editBookmark.css
@@ -67,22 +67,22 @@
   min-width: 13px;
   min-height: 13px;
   background: -moz-Field no-repeat 50% 50%;
 }
 
 
 /* ::::: bookmark panel dropdown icons ::::: */
 
-#editBMPanel_folderMenuList[selectedIndex="0"],
+#editBMPanel_folderMenuList[selectedGuid="toolbar_____"],
 #editBMPanel_toolbarFolderItem {
   list-style-image: url("chrome://browser/skin/places/bookmarksToolbar.svg") !important;
 }
 
-#editBMPanel_folderMenuList[selectedIndex="1"],
+#editBMPanel_folderMenuList[selectedGuid="menu________"],
 #editBMPanel_bmRootItem {
   list-style-image: url("chrome://browser/skin/places/bookmarksMenu.svg") !important;
 }
 
-#editBMPanel_folderMenuList[selectedIndex="2"],
+#editBMPanel_folderMenuList[selectedGuid="unfiled_____"],
 #editBMPanel_unfiledRootItem {
   list-style-image: url("chrome://browser/skin/places/unfiledBookmarks.svg") !important;
 }