Bug 1417036 - "Save to Pocket" drop down menu glitches. r?Gijs draft
authorDrew Willcoxon <adw@mozilla.com>
Wed, 15 Nov 2017 10:44:47 -0800
changeset 698436 2bfb965913939aeadaf85aa44192c307c8aac210
parent 697757 b0a6b4678b2f7dfb499328946b95366775f71edd
child 740377 408f8914369ea8f43d101f0e317d970b863b11ce
push id89287
push userdwillcoxon@mozilla.com
push dateWed, 15 Nov 2017 18:46:04 +0000
reviewersGijs
bugs1417036
milestone59.0a1
Bug 1417036 - "Save to Pocket" drop down menu glitches. r?Gijs MozReview-Commit-ID: 1JPOmQNLn26
browser/base/content/browser-pageActions.js
browser/extensions/pocket/bootstrap.js
browser/modules/PageActions.jsm
browser/modules/test/browser/browser_PageActions.js
--- a/browser/base/content/browser-pageActions.js
+++ b/browser/base/content/browser-pageActions.js
@@ -242,34 +242,36 @@ var BrowserPageActions = {
       iframeNode = document.createElement("iframe");
       iframeNode.setAttribute("type", "content");
       panelNode.appendChild(iframeNode);
     }
 
     let popupSet = document.getElementById("mainPopupSet");
     popupSet.appendChild(panelNode);
     panelNode.addEventListener("popuphidden", () => {
-      if (iframeNode) {
-        action.onIframeHidden(iframeNode, panelNode);
-      }
       panelNode.remove();
     }, { once: true });
 
     if (iframeNode) {
-      panelNode.addEventListener("popupshown", () => {
-        action.onIframeShown(iframeNode, panelNode);
+      panelNode.addEventListener("popupshowing", () => {
+        action.onIframeShowing(iframeNode, panelNode);
       }, { once: true });
       panelNode.addEventListener("popuphiding", () => {
         action.onIframeHiding(iframeNode, panelNode);
       }, { once: true });
+      panelNode.addEventListener("popuphidden", () => {
+        action.onIframeHidden(iframeNode, panelNode);
+      }, { once: true });
     }
 
     if (panelViewNode) {
       action.subview.onPlaced(panelViewNode);
-      action.subview.onShowing(panelViewNode);
+      panelNode.addEventListener("popupshowing", () => {
+        action.subview.onShowing(panelViewNode);
+      }, { once: true });
     }
 
     return panelNode;
   },
 
   // For tests.
   get _disablePanelAnimations() {
     return this.__disablePanelAnimations || false;
--- a/browser/extensions/pocket/bootstrap.js
+++ b/browser/extensions/pocket/bootstrap.js
@@ -134,17 +134,17 @@ var PocketPageAction = {
           wrapper.addEventListener("click", event => {
             let {BrowserPageActions} = wrapper.ownerGlobal;
             BrowserPageActions.doCommandForAction(this, event, wrapper);
           });
         },
         onPlacedInPanel(panelNode, urlbarNode) {
           PocketOverlay.onWindowOpened(panelNode.ownerGlobal);
         },
-        onIframeShown(iframe, panel) {
+        onIframeShowing(iframe, panel) {
           Pocket.onShownInPhotonPageActionPanel(panel, iframe);
 
           let doc = panel.ownerDocument;
           let urlbarNode = doc.getElementById("pocket-button-box");
           if (!urlbarNode || urlbarNode.hidden) {
             return;
           }
 
--- a/browser/modules/PageActions.jsm
+++ b/browser/modules/PageActions.jsm
@@ -520,19 +520,19 @@ this.PageActions = {
  *        onIframeHiding(iframeNode, parentPanelNode)
  *        * iframeNode: The iframe.
  *        * parentPanelNode: The panel node in which the iframe is shown.
  * @param onIframeHidden (function, optional)
  *        Called when the action's iframe is hidden:
  *        onIframeHidden(iframeNode, parentPanelNode)
  *        * iframeNode: The iframe.
  *        * parentPanelNode: The panel node in which the iframe is shown.
- * @param onIframeShown (function, optional)
- *        Called when the action's iframe is shown to the user:
- *        onIframeShown(iframeNode, parentPanelNode)
+ * @param onIframeShowing (function, optional)
+ *        Called when the action's iframe is showing to the user:
+ *        onIframeShowing(iframeNode, parentPanelNode)
  *        * iframeNode: The iframe.
  *        * parentPanelNode: The panel node in which the iframe is shown.
  * @param onLocationChange (function, optional)
  *        Called after tab switch or when the current <browser>'s location
  *        changes:
  *        onLocationChange(browserWindow)
  *        * browserWindow: The browser window containing the tab switch or
  *          changed <browser>.
@@ -579,17 +579,17 @@ function Action(options) {
     extensionID: false,
     iconURL: false,
     labelForHistogram: false,
     nodeAttributes: false,
     onBeforePlacedInWindow: false,
     onCommand: false,
     onIframeHiding: false,
     onIframeHidden: false,
-    onIframeShown: false,
+    onIframeShowing: false,
     onLocationChange: false,
     onPlacedInPanel: false,
     onPlacedInUrlbar: false,
     onRemovedFromWindow: false,
     onShowingInPanel: false,
     pinnedToUrlbar: false,
     subview: false,
     tooltip: false,
@@ -896,26 +896,26 @@ Action.prototype = {
    */
   onIframeHidden(iframeNode, parentPanelNode) {
     if (this._onIframeHidden) {
       this._onIframeHidden(iframeNode, parentPanelNode);
     }
   },
 
   /**
-   * Call this when the action's iframe is shown.
+   * Call this when the action's iframe is showing.
    *
    * @param  iframeNode (DOM node, required)
    *         The iframe that's being shown.
    * @param  parentPanelNode (DOM node, required)
    *         The panel in which the iframe is shown.
    */
-  onIframeShown(iframeNode, parentPanelNode) {
-    if (this._onIframeShown) {
-      this._onIframeShown(iframeNode, parentPanelNode);
+  onIframeShowing(iframeNode, parentPanelNode) {
+    if (this._onIframeShowing) {
+      this._onIframeShowing(iframeNode, parentPanelNode);
     }
   },
 
   /**
    * Call this on tab switch or when the current <browser>'s location changes.
    *
    * @param  browserWindow (DOM window, required)
    *         The browser window containing the tab switch or changed <browser>.
--- a/browser/modules/test/browser/browser_PageActions.js
+++ b/browser/modules/test/browser/browser_PageActions.js
@@ -450,32 +450,32 @@ add_task(async function withSubview() {
 
 // Tests a non-built-in action with an iframe.
 add_task(async function withIframe() {
   let id = "test-iframe";
 
   let onCommandCallCount = 0;
   let onPlacedInPanelCallCount = 0;
   let onPlacedInUrlbarCallCount = 0;
-  let onIframeShownCount = 0;
+  let onIframeShowingCount = 0;
 
   let panelButtonID = BrowserPageActions.panelButtonNodeIDForActionID(id);
   let urlbarButtonID = BrowserPageActions.urlbarButtonNodeIDForActionID(id);
 
   let action = PageActions.addAction(new PageActions.Action({
     iconURL: "chrome://browser/skin/mail.svg",
     id,
     pinnedToUrlbar: true,
     title: "Test iframe",
     wantsIframe: true,
     onCommand(event, buttonNode) {
       onCommandCallCount++;
     },
-    onIframeShown(iframeNode, panelNode) {
-      onIframeShownCount++;
+    onIframeShowing(iframeNode, panelNode) {
+      onIframeShowingCount++;
       Assert.ok(iframeNode, "iframeNode should be non-null: " + iframeNode);
       Assert.equal(iframeNode.localName, "iframe", "iframe localName");
       Assert.ok(panelNode, "panelNode should be non-null: " + panelNode);
       Assert.equal(panelNode.id, BrowserPageActions._activatedActionPanelID,
                    "panelNode.id");
     },
     onPlacedInPanel(buttonNode) {
       onPlacedInPanelCallCount++;
@@ -491,18 +491,18 @@ add_task(async function withIframe() {
 
   Assert.equal(action.id, id, "id");
   Assert.equal(action.wantsIframe, true, "wantsIframe");
 
   Assert.equal(onPlacedInPanelCallCount, 1,
                "onPlacedInPanelCallCount should be inc'ed");
   Assert.equal(onPlacedInUrlbarCallCount, 1,
                "onPlacedInUrlbarCallCount should be inc'ed");
-  Assert.equal(onIframeShownCount, 0,
-               "onIframeShownCount should remain 0");
+  Assert.equal(onIframeShowingCount, 0,
+               "onIframeShowingCount should remain 0");
   Assert.equal(onCommandCallCount, 0,
                "onCommandCallCount should remain 0");
 
   // The action's panel button should have been created.
   let panelButtonNode = document.getElementById(panelButtonID);
   Assert.notEqual(panelButtonNode, null, "panelButtonNode");
 
   // The action's urlbar button should have been created.
@@ -514,36 +514,36 @@ add_task(async function withIframe() {
   Assert.equal(
     urlbarButtonNode.nextSibling.id,
     PageActions.actionForID(PageActions.ACTION_ID_BOOKMARK).urlbarIDOverride,
     "Next node should be the bookmark star"
   );
 
   // Open the panel, click the action's button.
   await promisePageActionPanelOpen();
-  Assert.equal(onIframeShownCount, 0, "onIframeShownCount should remain 0");
+  Assert.equal(onIframeShowingCount, 0, "onIframeShowingCount should remain 0");
   EventUtils.synthesizeMouseAtCenter(panelButtonNode, {});
   await promisePanelShown(BrowserPageActions._activatedActionPanelID);
   Assert.equal(onCommandCallCount, 0, "onCommandCallCount should remain 0");
-  Assert.equal(onIframeShownCount, 1, "onIframeShownCount should be inc'ed");
+  Assert.equal(onIframeShowingCount, 1, "onIframeShowingCount should be inc'ed");
 
   // The activated-action panel should have opened, anchored to the action's
   // urlbar button.
   let aaPanel =
     document.getElementById(BrowserPageActions._activatedActionPanelID);
   Assert.notEqual(aaPanel, null, "activated-action panel");
   Assert.equal(aaPanel.anchorNode.id, urlbarButtonID, "aaPanel.anchorNode.id");
   EventUtils.synthesizeMouseAtCenter(urlbarButtonNode, {});
   await promisePanelHidden(BrowserPageActions._activatedActionPanelID);
 
   // Click the action's urlbar button.
   EventUtils.synthesizeMouseAtCenter(urlbarButtonNode, {});
   await promisePanelShown(BrowserPageActions._activatedActionPanelID);
   Assert.equal(onCommandCallCount, 0, "onCommandCallCount should remain 0");
-  Assert.equal(onIframeShownCount, 2, "onIframeShownCount should be inc'ed");
+  Assert.equal(onIframeShowingCount, 2, "onIframeShowingCount should be inc'ed");
 
   // The activated-action panel should have opened, again anchored to the
   // action's urlbar button.
   aaPanel = document.getElementById(BrowserPageActions._activatedActionPanelID);
   Assert.notEqual(aaPanel, null, "aaPanel");
   Assert.equal(aaPanel.anchorNode.id, urlbarButtonID, "aaPanel.anchorNode.id");
   EventUtils.synthesizeMouseAtCenter(urlbarButtonNode, {});
   await promisePanelHidden(BrowserPageActions._activatedActionPanelID);
@@ -553,17 +553,17 @@ add_task(async function withIframe() {
   urlbarButtonNode = document.getElementById(urlbarButtonID);
   Assert.equal(urlbarButtonNode, null, "urlbarButtonNode");
 
   // Open the panel, click the action's button.
   await promisePageActionPanelOpen();
   EventUtils.synthesizeMouseAtCenter(panelButtonNode, {});
   await promisePanelShown(BrowserPageActions._activatedActionPanelID);
   Assert.equal(onCommandCallCount, 0, "onCommandCallCount should remain 0");
-  Assert.equal(onIframeShownCount, 3, "onIframeShownCount should be inc'ed");
+  Assert.equal(onIframeShowingCount, 3, "onIframeShowingCount should be inc'ed");
 
   // The activated-action panel should have opened, this time anchored to the
   // main page action button in the urlbar.
   aaPanel = document.getElementById(BrowserPageActions._activatedActionPanelID);
   Assert.notEqual(aaPanel, null, "aaPanel");
   Assert.equal(aaPanel.anchorNode.id, BrowserPageActions.mainButtonNode.id,
                "aaPanel.anchorNode.id");
   EventUtils.synthesizeMouseAtCenter(BrowserPageActions.mainButtonNode, {});