Bug 1494046 - Improve how the folder icon is selected on the folder list in the bookmark popup window. r=mak
authorMark Banner <standard8@mozilla.com>
Tue, 23 Oct 2018 15:52:22 +0000
changeset 490966 7965ea047d6820f81670821b042654d3f41692c4
parent 490965 083e7905bc6505d7d35b6ce09f8ae412e7b6ea5a
child 490967 147e9c6a801ca68685ae0512bee2134976edeb29
child 490989 135b9518b925cfbd47e251a044c38d4304edf12b
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersmak
bugs1494046
milestone65.0a1
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/components/places/tests/browser/browser_bookmark_add_tags.js
browser/components/places/tests/browser/head.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
@@ -403,26 +403,39 @@ var gEditItemOverlay = {
       await this._appendFolderItemToMenupopup(menupopup,
                                               this._recentFolders[i].guid,
                                               this._recentFolders[i].title);
     }
 
     let title = (await PlacesUtils.bookmarks.fetch(aSelectedFolderGuid)).title;
     var defaultItem = this._getFolderMenuItem(aSelectedFolderGuid, title);
     this._folderMenuList.selectedItem = defaultItem;
+    // Ensure the selectedGuid attribute is set correctly (the above line wouldn't
+    // necessary trigger a select event, so handle it manually, then add the
+    // listener).
+    this._onFolderListSelected();
 
-    // Set a selectedIndex attribute to show special icons
-    this._folderMenuList.setAttribute("selectedIndex",
-                                      this._folderMenuList.selectedIndex);
+    this._folderMenuList.addEventListener("select", this);
+    this._folderMenuListListenerAdded = true;
 
     // 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) {
@@ -439,16 +452,21 @@ var gEditItemOverlay = {
     }
 
     if (this._observersAdded) {
       PlacesUtils.bookmarks.removeObserver(this);
       window.removeEventListener("unload", 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;
   },
 
@@ -677,19 +695,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
@@ -866,16 +881,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
@@ -32,16 +32,17 @@ support-files =
 [browser_bookmarkProperties_addFolderDefaultButton.js]
 [browser_bookmarkProperties_addKeywordForThisSearch.js]
 skip-if = (verify && debug)
 [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,91 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/
+ */
+
+const TEST_URL = "about:robots";
+let bookmarkPanel;
+let folders;
+let win;
+
+add_task(async function setup() {
+  await PlacesUtils.bookmarks.eraseEverything();
+
+  win = await BrowserTestUtils.openNewBrowserWindow();
+  await BrowserTestUtils.openNewForegroundTab({
+    gBrowser: win.gBrowser,
+    opening: TEST_URL,
+    waitForStateStop: true,
+  });
+
+  let oldTimeout = win.StarUI._autoCloseTimeout;
+  // Make the timeout something big, so it doesn't iteract badly with tests.
+  win.StarUI._autoCloseTimeout = 6000000;
+
+  bookmarkPanel = win.document.getElementById("editBookmarkPanel");
+  bookmarkPanel.setAttribute("animate", false);
+
+  registerCleanupFunction(async () => {
+    bookmarkPanel = null;
+    win.StarUI._autoCloseTimeout = oldTimeout;
+    // BrowserTestUtils.removeTab(tab);
+    await BrowserTestUtils.closeWindow(win);
+    win = null;
+    await PlacesUtils.bookmarks.eraseEverything();
+  });
+});
+
+add_task(async function test_selectChoose() {
+  await clickBookmarkStar(win);
+
+  // Open folder selector.
+  let menuList = win.document.getElementById("editBMPanel_folderMenuList");
+  let folderTreeRow = win.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, {}, win);
+  await promisePopup;
+
+  // Click the choose item.
+  EventUtils.synthesizeMouseAtCenter(win.document.getElementById("editBMPanel_chooseFolderMenuItem"), {}, win);
+
+  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(win);
+});
+
+
+add_task(async function test_selectBookmarksMenu() {
+  await clickBookmarkStar(win);
+
+  // Open folder selector.
+  let menuList = win.document.getElementById("editBMPanel_folderMenuList");
+
+  let promisePopup = BrowserTestUtils.waitForEvent(menuList.menupopup, "popupshown");
+  EventUtils.synthesizeMouseAtCenter(menuList, {}, win);
+  await promisePopup;
+
+  // Click the choose item.
+  EventUtils.synthesizeMouseAtCenter(win.document.getElementById("editBMPanel_bmRootItem"), {}, win);
+
+  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(win);
+});
--- a/browser/components/places/tests/browser/browser_bookmark_add_tags.js
+++ b/browser/components/places/tests/browser/browser_bookmark_add_tags.js
@@ -18,16 +18,30 @@ async function clickBookmarkStar() {
 }
 
 async function hideBookmarksPanel(callback) {
   let hiddenPromise = promisePopupHidden(bookmarkPanel);
   callback();
   await hiddenPromise;
 }
 
+add_task(function setup() {
+  let oldTimeout = StarUI._autoCloseTimeout;
+
+  bookmarkPanel.setAttribute("animate", false);
+
+  StarUI._autoCloseTimeout = 1000;
+
+  registerCleanupFunction(async () => {
+    StarUI._autoCloseTimeout = oldTimeout;
+    bookmarkPanel.removeAttribute("animate");
+    await PlacesUtils.bookmarks.eraseEverything();
+  });
+});
+
 add_task(async function test_add_bookmark_tags_from_bookmarkProperties() {
   const TEST_URL = "about:robots";
 
   let tab = await BrowserTestUtils.openNewForegroundTab({
     gBrowser,
     opening: TEST_URL,
     waitForStateStop: true,
   });
--- a/browser/components/places/tests/browser/head.js
+++ b/browser/components/places/tests/browser/head.js
@@ -399,21 +399,21 @@ function getToolbarNodeForItemGuid(itemG
     if (itemGuid === child._placesNode.bookmarkGuid) {
       return child;
     }
   }
   return null;
 }
 
 // Open the bookmarks Star UI by clicking the star button on the address bar.
-async function clickBookmarkStar() {
-  let shownPromise = promisePopupShown(document.getElementById("editBookmarkPanel"));
-  BookmarkingUI.star.click();
+async function clickBookmarkStar(win = window) {
+  let shownPromise = promisePopupShown(win.document.getElementById("editBookmarkPanel"));
+  win.BookmarkingUI.star.click();
   await shownPromise;
 }
 
 // Close the bookmarks Star UI by clicking the "Done" button.
-async function hideBookmarksPanel() {
-  let hiddenPromise = promisePopupHidden(document.getElementById("editBookmarkPanel"));
+async function hideBookmarksPanel(win = window) {
+  let hiddenPromise = promisePopupHidden(win.document.getElementById("editBookmarkPanel"));
   // Confirm and close the dialog.
-  document.getElementById("editBookmarkPanelDoneButton").click();
+  win.document.getElementById("editBookmarkPanelDoneButton").click();
   await hiddenPromise;
 }
--- 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;
 }