Bug 1395410 - Page action notifications should be anchored on relevant urlbar button, not main button, if present. r?jaws draft
authorDrew Willcoxon <adw@mozilla.com>
Thu, 31 Aug 2017 15:48:19 -0700
changeset 657000 c5f7d95ea691b948d826306c4e398503af1682b6
parent 656959 a815cfd494999014b4d839437cc9fcf27f944719
child 729310 a816990856afa8864b5cb199576b2b3898584da0
push id77401
push userdwillcoxon@mozilla.com
push dateThu, 31 Aug 2017 23:48:01 +0000
reviewersjaws
bugs1395410
milestone57.0a1
Bug 1395410 - Page action notifications should be anchored on relevant urlbar button, not main button, if present. r?jaws MozReview-Commit-ID: iFM0cR7zFi
browser/base/content/browser-pageActions.js
browser/base/content/test/urlbar/browser_page_action_menu.js
--- a/browser/base/content/browser-pageActions.js
+++ b/browser/base/content/browser-pageActions.js
@@ -177,43 +177,19 @@ var BrowserPageActions = {
 
   _toggleActivatedActionPanelForAction(action) {
     let panelNode = this.activatedActionPanelNode;
     if (panelNode) {
       panelNode.hidePopup();
       return null;
     }
 
-    // Before creating the panel, find the best anchor node for it because we'll
-    // bail if there isn't one.  Try each of the following nodes in order, using
-    // the first that's visible.
-    let anchorNode = null;
-    let potentialAnchorNodeIDs = [
-      action.anchorIDOverride || null,
-      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) {
-            anchorNode = node;
-            break;
-          }
-        }
-      }
-    }
-    if (!anchorNode) {
-      throw new Error(`PageActions: No anchor node for '${action.id}'`);
-    }
+    // Before creating the panel, get the anchor node for it because it'll throw
+    // if there isn't one (which shouldn't happen, but still).
+    let anchorNode = this.panelAnchorNodeForAction(action);
 
     panelNode = document.createElement("panel");
     panelNode.id = this._activatedActionPanelID;
     panelNode.classList.add("cui-widget-panel");
     panelNode.setAttribute("actionID", action.id);
     panelNode.setAttribute("role", "group");
     panelNode.setAttribute("type", "arrow");
     panelNode.setAttribute("flip", "slide");
@@ -267,16 +243,40 @@ var BrowserPageActions = {
 
     if (iframeNode) {
       action.onIframeShown(iframeNode, panelNode);
     }
 
     return panelNode;
   },
 
+  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),
+      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}'`);
+  },
+
   get activatedActionPanelNode() {
     return document.getElementById(this._activatedActionPanelID);
   },
 
   get _activatedActionPanelID() {
     return "pageActionActivatedActionPanel";
   },
 
@@ -695,28 +695,20 @@ var BrowserPageActionFeedback = {
   },
 
   get feedbackLabel() {
     delete this.feedbackLabel;
     return this.feedbackLabel = document.getElementById("pageActionFeedbackMessage");
   },
 
   show(action, event) {
-    this.feedbackLabel.textContent = this.panelNode.getAttribute(action + "Feedback");
+    this.feedbackLabel.textContent = this.panelNode.getAttribute(action.id + "Feedback");
     this.panelNode.hidden = false;
 
-    let anchor = BrowserPageActions.mainButtonNode;
-    if (event.target.classList.contains("urlbar-icon")) {
-      let id = BrowserPageActions._urlbarButtonNodeIDForActionID(action);
-      let node = document.getElementById(id);
-      if (node) {
-        anchor = node;
-      }
-    }
-
+    let anchor = BrowserPageActions.panelAnchorNodeForAction(action);
     this.panelNode.openPopup(anchor, {
       position: "bottomcenter topright",
       triggerEvent: event,
     });
 
     this.panelNode.addEventListener("popupshown", () => {
       this.feedbackAnimationBox.setAttribute("animate", "true");
     }, {once: true});
@@ -760,17 +752,18 @@ BrowserPageActions.copyURL = {
     BrowserPageActions.takeNodeAttributeFromPanel(buttonNode, "title");
   },
 
   onCommand(event, buttonNode) {
     BrowserPageActions.panelNode.hidePopup();
     Cc["@mozilla.org/widget/clipboardhelper;1"]
       .getService(Ci.nsIClipboardHelper)
       .copyString(gURLBar.makeURIReadable(gBrowser.selectedBrowser.currentURI).displaySpec);
-    BrowserPageActionFeedback.show("copyURL", event);
+    let action = PageActions.actionForID("copyURL");
+    BrowserPageActionFeedback.show(action, event);
   },
 };
 
 // email link
 BrowserPageActions.emailLink = {
   onPlacedInPanel(buttonNode) {
     BrowserPageActions.takeNodeAttributeFromPanel(buttonNode, "title");
   },
@@ -827,17 +820,18 @@ BrowserPageActions.sendToDevice = {
       item.setAttribute("tooltiptext", name);
       item.addEventListener("command", event => {
         if (panelNode) {
           panelNode.hidePopup();
         }
         // There are items in the subview that don't represent devices: "Sign
         // in", "Learn about Sync", etc.  Device items will be .sendtab-target.
         if (event.target.classList.contains("sendtab-target")) {
-          BrowserPageActionFeedback.show("sendToDevice", event);
+          let action = PageActions.actionForID("sendToDevice");
+          BrowserPageActionFeedback.show(action, event);
         }
       });
       return item;
     });
 
     bodyNode.removeAttribute("state");
     // In the first ~10 sec after startup, Sync may not be loaded and the list
     // of devices will be empty.
--- a/browser/base/content/test/urlbar/browser_page_action_menu.js
+++ b/browser/base/content/test/urlbar/browser_page_action_menu.js
@@ -546,16 +546,20 @@ add_task(async function sendToDevice_inU
     EventUtils.synthesizeMouseAtCenter(deviceMenuItem, {});
     info("Waiting for Send to Device panel to close after clicking a device");
     await hiddenPromise;
 
     // And then the "Sent!" notification panel should open and close by itself
     // after a moment.
     info("Waiting for the Sent! notification panel to open");
     await promisePanelShown(BrowserPageActionFeedback.panelNode.id);
+    Assert.equal(
+      BrowserPageActionFeedback.panelNode.anchorNode.id,
+      urlbarButton.id
+    );
     info("Waiting for the Sent! notification panel to close");
     await promisePanelHidden(BrowserPageActionFeedback.panelNode.id);
 
     // Remove Send to Device from the urlbar.
     action.shownInUrlbar = false;
     BrowserPageActions._disableActivatedActionPanelAnimation = false;
 
     cleanUp();