Bug 1402721 - Add/edit bookmark panel should open anchored on the page action (ellipsis) button instead of on the identity block (left side of the address bar) when the bookmarks star action is not pinned/visible. r=Gijs
authorDrew Willcoxon <adw@mozilla.com>
Mon, 25 Sep 2017 13:30:08 -0700
changeset 670167 86a27018d5bcc3d0ab523e2a5706aa01f8b83245
parent 670112 7a0ad8b39920cde8ec68d5d7cd8d727fbd64fa1b
child 670168 11a85c79e87100e876f3a9dfcd1159db2b782956
push id81544
push userfmarier@mozilla.com
push dateTue, 26 Sep 2017 00:09:57 +0000
reviewersGijs
bugs1402721
milestone58.0a1
Bug 1402721 - Add/edit bookmark panel should open anchored on the page action (ellipsis) button instead of on the identity block (left side of the address bar) when the bookmarks star action is not pinned/visible. r=Gijs MozReview-Commit-ID: JMviXx5ov7F
browser/base/content/browser-pageActions.js
browser/base/content/browser-places.js
--- a/browser/base/content/browser-pageActions.js
+++ b/browser/base/content/browser-pageActions.js
@@ -257,38 +257,48 @@ var BrowserPageActions = {
 
     if (iframeNode) {
       action.onIframeShown(iframeNode, panelNode);
     }
 
     return panelNode;
   },
 
+  /**
+   * Returns the node in the urlbar to which popups for the given action should
+   * be anchored.  If the action is null, a sensible anchor is returned.
+   *
+   * @param  action (PageActions.Action, optional)
+   *         The action you want to anchor.
+   * @return (DOM node, nonnull) The node to which the action should be
+   *         anchored.
+   */
   panelAnchorNodeForAction(action) {
     // Try each of the following nodes in order, using the first that's visible.
     let potentialAnchorNodeIDs = [
-      action.anchorIDOverride || null,
-      this._urlbarButtonNodeIDForActionID(action.id),
+      action && action.anchorIDOverride,
+      action && this._urlbarButtonNodeIDForActionID(action.id),
       this.mainButtonNode.id,
       "identity-icon",
     ];
     let dwu = window.QueryInterface(Ci.nsIInterfaceRequestor)
                     .getInterface(Ci.nsIDOMWindowUtils);
     for (let id of potentialAnchorNodeIDs) {
       if (id) {
         let node = document.getElementById(id);
         if (node && !node.hidden) {
           let bounds = dwu.getBoundsWithoutFlushing(node);
           if (bounds.height > 0 && bounds.width > 0) {
             return node;
           }
         }
       }
     }
-    throw new Error(`PageActions: No anchor node for '${action.id}'`);
+    let id = action ? action.id : "<no action>";
+    throw new Error(`PageActions: No anchor node for ${id}`);
   },
 
   get activatedActionPanelNode() {
     return document.getElementById(this._activatedActionPanelID);
   },
 
   get _activatedActionPanelID() {
     return "pageActionActivatedActionPanel";
--- a/browser/base/content/browser-places.js
+++ b/browser/base/content/browser-places.js
@@ -297,31 +297,23 @@ var StarUI = {
     let forms = gNavigatorBundle.getString("editBookmark.removeBookmarks.label");
     let bookmarksCount = this._itemGuids.length;
     let label = PluralForm.get(bookmarksCount, forms)
                           .replace("#1", bookmarksCount);
     this._element("editBookmarkPanelRemoveButton").label = label;
 
     this.beginBatch();
 
-    if (aAnchorElement) {
-      // Set the open=true attribute if the anchor is a
-      // descendent of a toolbarbutton.
-      let parent = aAnchorElement.parentNode;
-      while (parent) {
-        if (parent.localName == "toolbarbutton") {
-          break;
-        }
-        parent = parent.parentNode;
-      }
-      if (parent) {
-        this._anchorToolbarButton = parent;
-        parent.setAttribute("open", "true");
-      }
+    if (aAnchorElement && aAnchorElement.closest("#urlbar")) {
+      this._anchorToolbarButton = aAnchorElement;
+      aAnchorElement.setAttribute("open", "true");
+    } else {
+      this._anchorToolbarButton = null;
     }
+
     let onPanelReady = fn => {
       let target = this.panel;
       if (target.parentNode) {
         // By targeting the panel's parent and using a capturing listener, we
         // can have our listener called before others waiting for the panel to
         // be shown (which probably expect the panel to be fully initialized)
         target = target.parentNode;
       }
@@ -329,23 +321,16 @@ var StarUI = {
         fn();
       }, {"capture": true, "once": true});
     };
     gEditItemOverlay.initPanel({ node: aNode,
                                  onPanelReady,
                                  hiddenRows: ["description", "location",
                                               "loadInSidebar", "keyword"],
                                  focusedElement: "preferred"});
-
-    if (aAnchorElement && aAnchorElement.id == BookmarkingUI.STAR_BOX_ID) {
-      aAnchorElement.setAttribute("open", "true");
-      this.panel.addEventListener("popuphiding", () => {
-        aAnchorElement.removeAttribute("open");
-      });
-    }
     this.panel.openPopup(aAnchorElement, aPosition);
   },
 
   panelShown:
   function SU_panelShown(aEvent) {
     if (aEvent.target == this.panel) {
       if (this._element("editBookmarkPanelContent").hidden) {
         // Note this isn't actually used anymore, we should remove this
@@ -1366,28 +1351,18 @@ var BookmarkingUI = {
   },
 
   get starBox() {
     delete this.starBox;
     return this.starBox = document.getElementById(this.STAR_BOX_ID);
   },
 
   get anchor() {
-    // Try to anchor the panel to:
-    // 1. The bookmarks star box (using the star itself is trickier because it
-    //    can be hidden while the star animation is visible)
-    // 2. The identity icon
-    if (this.starBox && isVisible(this.starBox)) {
-      return this.starBox;
-    }
-    let identityIcon = document.getElementById("identity-icon");
-    if (identityIcon && isVisible(identityIcon)) {
-      return identityIcon;
-    }
-    return null;
+    let action = PageActions.actionForID(PageActions.ACTION_ID_BOOKMARK);
+    return BrowserPageActions.panelAnchorNodeForAction(action);
   },
 
   get notifier() {
     delete this.notifier;
     return this.notifier = document.getElementById("bookmarked-notification-anchor");
   },
 
   get dropmarkerNotifier() {