Bug 1400323 - Refactor code for openInTabClosesMenu pref to avoid needing setTimeout on Mac. (cleanup on popuphidden) draft
authorTawny Hoover <stayopenmenu@gmail.com>
Thu, 17 May 2018 22:32:47 -0400
changeset 800748 a53a54cdc15a8121af3b4164822423d214cb7496
parent 800319 4e9446f9e8f0a75c7ffe063f1dfb311cc90d56cf
push id111465
push userbmo:stayopenmenu@gmail.com
push dateTue, 29 May 2018 02:55:29 +0000
bugs1400323
milestone62.0a1
Bug 1400323 - Refactor code for openInTabClosesMenu pref to avoid needing setTimeout on Mac. (cleanup on popuphidden) MozReview-Commit-ID: FR6vZksnFEz
browser/base/content/browser-places.js
browser/components/places/tests/browser/browser.ini
--- a/browser/base/content/browser-places.js
+++ b/browser/base/content/browser-places.js
@@ -755,16 +755,24 @@ var BookmarksEventHandler = {
       return;
     let target = aEvent.originalTarget;
     if (target.tagName != "menuitem")
       return;
     let modifKey = AppConstants.platform === "macosx" ? aEvent.metaKey
                                                       : aEvent.ctrlKey;
     if (modifKey) {
       target.setAttribute("closemenu", "none");
+      var menupopup = target.parentNode;
+      menupopup.addEventListener("popuphidden", () => {
+        target.removeAttribute("closemenu");
+      }, {once: true});
+    } else {
+      // Handles edge case where same menuitem was opened previously
+      // while menu was kept open, but now menu should close.
+      target.removeAttribute("closemenu");
     }
   },
 
   onClick: function BEH_onClick(aEvent, aView) {
     // Only handle middle-click or left-click with modifiers.
     let modifKey;
     if (AppConstants.platform == "macosx") {
       modifKey = aEvent.metaKey || aEvent.shiftKey;
@@ -777,30 +785,20 @@ var BookmarksEventHandler = {
 
     var target = aEvent.originalTarget;
     // If this event bubbled up from a menu or menuitem,
     // close the menus if browser.bookmarks.openInTabClosesMenu.
     var tag = target.tagName;
     if (PlacesUIUtils.openInTabClosesMenu && (tag == "menuitem" || tag == "menu")) {
       closeMenus(aEvent.target);
     }
-    // Command already processed so remove any closemenu attr set in onMouseUp.
-    if (aEvent.button == 0 &&
-        target.tagName == "menuitem" &&
-        target.getAttribute("closemenu") == "none") {
-      // On Mac we need to extend when we remove the flag, to avoid any pre-close
-      // animations.
-      setTimeout(() => {
-        target.removeAttribute("closemenu");
-      }, 500);
-    }
 
     if (target._placesNode && PlacesUtils.nodeIsContainer(target._placesNode)) {
       // Don't open the root folder in tabs when the empty area on the toolbar
-      // is middle-clicked or when a non-bookmark item except for Open in Tabs)
+      // is middle-clicked or when a non-bookmark item (except for Open in Tabs)
       // in a bookmarks menupopup is middle-clicked.
       if (target.localName == "menu" || target.localName == "toolbarbutton")
         PlacesUIUtils.openContainerNodeInTabs(target._placesNode, aEvent, aView);
     } else if (aEvent.button == 1) {
       // left-clicks with modifier are already served by onCommand
       this.onCommand(aEvent);
     }
   },
--- a/browser/components/places/tests/browser/browser.ini
+++ b/browser/components/places/tests/browser/browser.ini
@@ -74,17 +74,16 @@ subsuite = clipboard
 subsuite = clipboard
 [browser_remove_bookmarks.js]
 subsuite = clipboard
 [browser_sidebar_open_bookmarks.js]
 [browser_sidebarpanels_click.js]
 skip-if = (os == "mac" && debug) # Bug 1423667
 [browser_sort_in_library.js]
 [browser_stayopenmenu.js]
-skip-if = os == "mac" && debug # bug 1400323
 [browser_toolbar_drop_text.js]
 [browser_toolbar_overflow.js]
 [browser_toolbarbutton_menu_context.js]
 [browser_views_iconsupdate.js]
 [browser_bug485100-change-case-loses-tag.js]
 [browser_editBookmark_tags_liveUpdate.js]
 [browser_bug427633_no_newfolder_if_noip.js]
 [browser_editBookmark_keywords.js]